From 6c211f5c112e3ffa1e3e0cda969194b3284cbf47 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 10:43:43 +1000 Subject: [PATCH 1/3] fix(stack): consume protect-ffi 0.31.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps the exact pin from 0.30.0 to 0.31.0 across `@cipherstash/stack` and the two adapters that carry it as a devDependency. 0.31.0 is a release with a `Breaking` heading, and four incompatibilities land with it. **1. The `ProtectError` class is gone**, replaced by an `isProtectErrorCode` guard. Both bindings now throw an ordinary `Error` with `code` set by Rust, so there is no class to match — and `instanceof` was unreliable regardless, being false across duplicate copies of a package. Every type-only import is unaffected; the two value sites move to a value check. That check is on the code's VALUE, not the presence of a `code` property, which fixes a pre-existing bug in `dynamodb/helpers.ts`. Its fallback branch accepted any string-valued `code` and asserted it into `ProtectErrorCode`, so a Node error — `ECONNRESET` from the DynamoDB client, say — was reported as an encryption error code. The two branches collapse into one correct one. **2. The wasm `newClient` moved credentials into `clientOpts`** and renamed `strategy` to `authStrategy`. Credentials left at the top level are now rejected outright, so that half fails loudly; a `keyset` left there would be silently ignored and bind the client to the DEFAULT keyset, encrypting under the wrong keys. This config forwards no keyset, and the test now asserts `clientOpts` as a whole so one landing elsewhere is caught. The `as never` is deleted. 0.30 typed the wasm options as `any`, so the cast was load-bearing; 0.31 types them properly. Removing it immediately surfaced `encryptConfig`, below — which is the argument for removing it. **3. `encryptConfig` no longer needs normalising.** 0.30's wasm binding accepted EQL-native `cast_as` only, so the factory ran `normalizeCastAs` first. 0.31 normalizes at the Rust deserialization boundary on both bindings and types the result as `CanonicalEncryptConfig`, documented as a shape nothing asks you to build — so it is not assignable to the `EncryptConfig` `newClient` declares, and keeping the call would need an assertion that misdescribes the value. `normalizeCastAs` is deprecated rather than deleted — it is the only exhaustive consumer of `toEqlCastAs`, and removing both is a deliberate cleanup, not a side effect of a dependency bump. **4. Unknown payload keys now reach Rust and are rejected.** Stack attaches a correlation `id` to every bulk encrypt/decrypt payload, and protect-ffi's `EncryptPayload` / `BulkDecryptPayload` have never declared one — 0.30 dropped it silently, 0.31 fails the whole call with ``unknown field `id` ``. Nothing was using it: results are correlated positionally, by `keyMap` index in the model helpers and against the original array in `mapEncryptedDataToResult` / `mapDecryptedDataToResult`. The id is stripped at the FFI boundary and stays on stack's own side of it. This is the first of four stacked PRs splitting the protect-ffi monorepo absorption. It consumes the PUBLISHED 0.31.0 from npm and is independent of the vendoring that follows: `packages/stack` uses nothing added to protect-ffi after the 0.31.0 tag, and `eql-v3.ts`'s export surface is identical between the published release and the later in-tree copy. --- .changeset/olive-pugs-invite.md | 44 ++++++++++++ packages/stack-drizzle/package.json | 2 +- packages/stack-supabase/package.json | 2 +- .../encrypt-lock-context-guards.test.ts | 10 ++- .../encrypt-query-match-preflight.test.ts | 6 +- packages/stack/__tests__/error-codes.test.ts | 32 ++++++--- .../__tests__/wasm-inline-new-client.test.ts | 37 +++++++--- .../stack/__tests__/wasm-inline-v3.test.ts | 11 +-- packages/stack/package.json | 2 +- packages/stack/src/dynamodb/helpers.ts | 27 ++++---- .../src/encryption/helpers/error-code.ts | 14 +++- .../src/encryption/helpers/model-helpers.ts | 36 +++++++--- .../src/encryption/operations/bulk-decrypt.ts | 9 ++- .../src/encryption/operations/bulk-encrypt.ts | 10 ++- packages/stack/src/wasm-inline.ts | 68 +++++++++++++------ pnpm-lock.yaml | 66 +++++++++--------- 16 files changed, 268 insertions(+), 108 deletions(-) create mode 100644 .changeset/olive-pugs-invite.md diff --git a/.changeset/olive-pugs-invite.md b/.changeset/olive-pugs-invite.md new file mode 100644 index 000000000..d2c865be0 --- /dev/null +++ b/.changeset/olive-pugs-invite.md @@ -0,0 +1,44 @@ +--- +'@cipherstash/stack': major +--- + +Adopt protect-ffi 0.31.0. + +`major`, not `minor`, because of the first item below: a credential encoding +that worked on 1.x stops working at client construction, and `@cipherstash/stack` +pins `@cipherstash/protect-ffi` exactly — so upgrading stack forces the new FFI +and there is no version of this a caller opts into separately. That hex was +always the documented encoding describes intent, not the behaviour anyone was +running against. The fixed group takes `stash`, `wizard` and the three adapters +to 2.0.0 with it; that is a release-management cost, not an argument about what +the version number means. + +**`clientKey` must now be hex-encoded.** This is the change to check before +upgrading. The client key used to be decoded by a function that accepted both +hex and standard padded base64 — the encoding `~/.cipherstash/secretkey.json` +stores on disk — so a base64 value in `config.clientKey` or `CS_CLIENT_KEY` +worked even though the documented encoding is hex. It is now rejected at client +construction with `invalid clientKey: expected a hex-encoded key`. + +The message deliberately says nothing more, because the underlying decode error +names the offending character and its offset and would put part of a live key +into your logs. So if every operation starts failing at construction after this +upgrade, check the encoding of your key first. Re-encode it as hex, or drop the +explicit key and let the client read it from the profile store. + +Reading the key from `~/.cipherstash/secretkey.json` is unaffected — that path +still uses base64, and only an explicitly supplied key is now hex-only. + +**DynamoDB errors no longer report foreign error codes as encryption codes.** +`handleError` accepted any string-valued `code` on a caught error and passed it +through as a `ProtectErrorCode`, so a Node or AWS SDK failure — `ECONNRESET`, +say — surfaced as though it were an encryption error code. Codes are now checked +against the set the encryption layer actually emits, and anything else becomes +`DYNAMODB_ENCRYPTION_ERROR`. If you branch on `error.code` for DynamoDB +operations, a branch that was matching transport errors will stop. + +Also in this release, with no action needed: the WASM entry passes credentials +under the option shape 0.31 expects and no longer pre-normalises `cast_as` +(the native layer does it on both bindings now), and bulk operations no longer +forward their internal correlation id across the FFI boundary, which 0.31 +rejects rather than ignores. diff --git a/packages/stack-drizzle/package.json b/packages/stack-drizzle/package.json index d7dfcd2cc..dc9713602 100644 --- a/packages/stack-drizzle/package.json +++ b/packages/stack-drizzle/package.json @@ -62,7 +62,7 @@ "drizzle-orm": ">=0.33" }, "devDependencies": { - "@cipherstash/protect-ffi": "0.30.0", + "@cipherstash/protect-ffi": "0.31.0", "@cipherstash/test-kit": "workspace:*", "fta-cli": "3.0.0", "dotenv": "17.4.2", diff --git a/packages/stack-supabase/package.json b/packages/stack-supabase/package.json index 3a1837853..0efbfe0b6 100644 --- a/packages/stack-supabase/package.json +++ b/packages/stack-supabase/package.json @@ -69,7 +69,7 @@ } }, "devDependencies": { - "@cipherstash/protect-ffi": "0.30.0", + "@cipherstash/protect-ffi": "0.31.0", "@cipherstash/test-kit": "workspace:*", "fta-cli": "3.0.0", "@supabase/postgrest-js": "2.110.2", diff --git a/packages/stack/__tests__/encrypt-lock-context-guards.test.ts b/packages/stack/__tests__/encrypt-lock-context-guards.test.ts index feaa31a13..ec8dd92df 100644 --- a/packages/stack/__tests__/encrypt-lock-context-guards.test.ts +++ b/packages/stack/__tests__/encrypt-lock-context-guards.test.ts @@ -25,9 +25,13 @@ import { LockContext } from '@/identity' import { Encryption } from '@/index' vi.mock('@cipherstash/protect-ffi', () => ({ - // `getErrorCode` does `error instanceof ProtectError` on the failure path, - // so the mock must export the class even though the guards throw plain Errors. - ProtectError: class ProtectError extends Error {}, + // `getErrorCode` calls `isProtectErrorCode` on the failure path, so the mock + // must export it even though these guards throw plain Errors with no `code`. + // Mirrors the real predicate rather than stubbing `false`: a stub would pass + // whether or not the guards short-circuit before the FFI, which is the whole + // property under test. + isProtectErrorCode: (value: unknown) => + typeof value === 'string' && value === 'UNKNOWN_COLUMN', newClient: vi.fn(async () => ({ __mock: 'client' })), encrypt: vi.fn(async () => ({ v: 2, c: 'ciphertext' })), // The model / bulk-model path funnels through `encryptBulk`. Return one diff --git a/packages/stack/__tests__/encrypt-query-match-preflight.test.ts b/packages/stack/__tests__/encrypt-query-match-preflight.test.ts index 892761be6..717764d26 100644 --- a/packages/stack/__tests__/encrypt-query-match-preflight.test.ts +++ b/packages/stack/__tests__/encrypt-query-match-preflight.test.ts @@ -3,7 +3,11 @@ import { encryptedTable, types } from '@/eql/v3' import { Encryption } from '@/index' vi.mock('@cipherstash/protect-ffi', () => ({ - ProtectError: class ProtectError extends Error {}, + // 0.31 replaced the `ProtectError` class with this guard; `getErrorCode` + // reaches it on the failure path. The preflight rejects before the FFI, so + // the errors here carry no `code` and the predicate is never satisfied. + isProtectErrorCode: (value: unknown) => + typeof value === 'string' && value === 'UNKNOWN_COLUMN', newClient: vi.fn(async () => ({ __mock: 'client' })), encryptQuery: vi.fn(async () => ({ v: 3, bf: [1] })), encryptQueryBulk: vi.fn(async () => [{ v: 3, bf: [1] }]), diff --git a/packages/stack/__tests__/error-codes.test.ts b/packages/stack/__tests__/error-codes.test.ts index 09c3cf011..b99c4d2a9 100644 --- a/packages/stack/__tests__/error-codes.test.ts +++ b/packages/stack/__tests__/error-codes.test.ts @@ -1,5 +1,5 @@ import 'dotenv/config' -import { ProtectError as FfiProtectError } from '@cipherstash/protect-ffi' +import { isProtectErrorCode } from '@cipherstash/protect-ffi' import { beforeAll, describe, expect, it } from 'vitest' import type { EncryptionClient } from '@/encryption' import { encryptedTable, types } from '@/eql/v3' @@ -39,14 +39,28 @@ describe('FFI Error Code Preservation', () => { protectClient = await Encryption({ schemas: [testSchema, noIndexSchema] }) }) - describe('FfiProtectError class', () => { - it('constructs with code and message', () => { - const error = new FfiProtectError({ - code: 'UNKNOWN_COLUMN', - message: 'Test error', - }) - expect(error.code).toBe('UNKNOWN_COLUMN') - expect(error.message).toBe('Test error') + describe('isProtectErrorCode', () => { + // protect-ffi 0.31.0 removed the `ProtectError` class this block used to + // construct. Both bindings now throw an ordinary `Error` with `code` set by + // Rust, so there is no class to match — `instanceof` cost a rewritten stack + // trace, made the two bindings throw different things, and was false across + // duplicate copies of the package anyway. + it('recognises a code the FFI actually emits', () => { + expect(isProtectErrorCode('UNKNOWN_COLUMN')).toBe(true) + }) + + it('rejects a Node error code', () => { + // The reason `getErrorCode` checks the code's VALUE rather than the + // presence of a `code` property: Node sets `code` on its own errors, so a + // presence check would report `ECONNRESET` as an encryption error code. + expect(isProtectErrorCode('ECONNRESET')).toBe(false) + expect(isProtectErrorCode('MODULE_NOT_FOUND')).toBe(false) + }) + + it('rejects non-string values', () => { + expect(isProtectErrorCode(undefined)).toBe(false) + expect(isProtectErrorCode(null)).toBe(false) + expect(isProtectErrorCode(42)).toBe(false) }) }) diff --git a/packages/stack/__tests__/wasm-inline-new-client.test.ts b/packages/stack/__tests__/wasm-inline-new-client.test.ts index 4d2140fa1..b99aa0caa 100644 --- a/packages/stack/__tests__/wasm-inline-new-client.test.ts +++ b/packages/stack/__tests__/wasm-inline-new-client.test.ts @@ -65,7 +65,7 @@ describe('wasm-inline Encryption → newClient (protect-ffi 0.25 single-object f expect(call).toHaveLength(1) }) - it('nests the resolved strategy and forwards clientId / clientKey', async () => { + it('nests the resolved strategy and credentials under their 0.31 keys', async () => { await Encryption({ schemas: [users], config: { @@ -78,15 +78,28 @@ describe('wasm-inline Encryption → newClient (protect-ffi 0.25 single-object f // biome-ignore lint/suspicious/noExplicitAny: reading the recorded single options object const arg = vi.mocked(wasmNewClient).mock.calls[0][0] as any - expect(arg.strategy).toEqual({ __mock: 'access-key-strategy' }) - expect(arg.clientId).toBe('cid') - expect(arg.clientKey).toBe('ckey') + + // protect-ffi 0.31 moved the credentials into `clientOpts`, where the Neon + // entry has always had them, and renamed `strategy` to `authStrategy`. + // Credentials left at the top level are now REJECTED, so that half fails + // loudly — but a `keyset` left there is silently ignored and the client + // binds to the default keyset, encrypting under the wrong keys. This + // config forwards no keyset; if one is added it goes inside `clientOpts`, + // and this test is what should catch it landing anywhere else. + expect(arg.authStrategy).toEqual({ __mock: 'access-key-strategy' }) + expect(arg.clientOpts).toEqual({ clientId: 'cid', clientKey: 'ckey' }) + expect(arg.clientId).toBeUndefined() + expect(arg.clientKey).toBeUndefined() + expect(arg.strategy).toBeUndefined() }) - it('passes a cast_as-normalised encryptConfig (SDK "string" → EQL "text")', async () => { - // `types.TextSearch('email')` carries `cast_as: 'string'`; the WASM client - // only accepts EQL-native variants, so the factory must run the config - // through `normalizeCastAs` before handing it to `newClient`. + it('forwards encryptConfig unnormalised, letting the FFI canonicalise', async () => { + // `types.TextSearch('email')` carries `cast_as: 'string'`. Under 0.30 the + // WASM binding accepted EQL-native variants only, so the factory ran the + // config through `normalizeCastAs` first. 0.31 normalizes at the Rust + // deserialization boundary on both bindings — verified against the 0.31 + // wasm build, where `'string'` and `'text'` both get past config parsing to + // authentication — so the SDK spelling now goes through untouched. await Encryption({ schemas: [users], config: { @@ -100,7 +113,7 @@ describe('wasm-inline Encryption → newClient (protect-ffi 0.25 single-object f // biome-ignore lint/suspicious/noExplicitAny: navigating the recorded encryptConfig const arg = vi.mocked(wasmNewClient).mock.calls[0][0] as any expect(arg.encryptConfig).toBeDefined() - expect(arg.encryptConfig.tables.users.email.cast_as).toBe('text') + expect(arg.encryptConfig.tables.users.email.cast_as).toBe('string') }) it('uses an explicit config.authStrategy verbatim on the strategy path', async () => { @@ -117,6 +130,10 @@ describe('wasm-inline Encryption → newClient (protect-ffi 0.25 single-object f // biome-ignore lint/suspicious/noExplicitAny: reading the recorded single options object const arg = vi.mocked(wasmNewClient).mock.calls[0][0] as any - expect(arg.strategy).toBe(explicit) + // `authStrategy` since 0.31. `strategy` still works there as a deprecated + // alias, but this passes the resolved strategy under the current name so + // the call does not depend on a field slated for removal. + expect(arg.authStrategy).toBe(explicit) + expect(arg.strategy).toBeUndefined() }) }) diff --git a/packages/stack/__tests__/wasm-inline-v3.test.ts b/packages/stack/__tests__/wasm-inline-v3.test.ts index 917f1f180..4e63b561f 100644 --- a/packages/stack/__tests__/wasm-inline-v3.test.ts +++ b/packages/stack/__tests__/wasm-inline-v3.test.ts @@ -65,12 +65,15 @@ describe('wasm-inline is EQL v3 only (#614)', () => { expect(newClientOpts().eqlVersion).toBe(3) }) - it('normalises cast_as on the v3 path (SDK "string" → EQL "text")', async () => { + it('forwards cast_as unnormalised on the v3 path', async () => { await Encryption({ schemas: [users], config }) - // `types.TextSearch` carries `cast_as: 'string'`; the WASM client only - // accepts EQL-native variants, so the factory must map it to `'text'`. + // `types.TextSearch` carries `cast_as: 'string'`. Under protect-ffi 0.30 + // the WASM binding accepted EQL-native variants only, so the factory + // mapped it to `'text'` first. 0.31 normalizes at the Rust deserialization + // boundary on both bindings and documents `CanonicalEncryptConfig` as a + // shape callers do not build, so the SDK spelling goes straight through. expect(newClientOpts().encryptConfig.tables.users.email.cast_as).toBe( - 'text', + 'string', ) }) diff --git a/packages/stack/package.json b/packages/stack/package.json index 3e60aeb46..d538e6f79 100644 --- a/packages/stack/package.json +++ b/packages/stack/package.json @@ -216,7 +216,7 @@ "dependencies": { "@byteslice/result": "0.2.0", "@cipherstash/auth": "catalog:repo", - "@cipherstash/protect-ffi": "0.30.0", + "@cipherstash/protect-ffi": "0.31.0", "evlog": "1.11.0", "uuid": "14.0.1", "zod": "3.25.76" diff --git a/packages/stack/src/dynamodb/helpers.ts b/packages/stack/src/dynamodb/helpers.ts index a7d06085a..76f94f115 100644 --- a/packages/stack/src/dynamodb/helpers.ts +++ b/packages/stack/src/dynamodb/helpers.ts @@ -1,5 +1,5 @@ import type { ProtectErrorCode } from '@cipherstash/protect-ffi' -import { ProtectError as FfiProtectError } from '@cipherstash/protect-ffi' +import { isProtectErrorCode } from '@cipherstash/protect-ffi' import { resolveEncryptColumnMap } from '@/encryption/helpers/model-helpers' import { DATE_LIKE_CASTS } from '@/eql/v3/columns' import type { EncryptedValue } from '@/types' @@ -41,18 +41,21 @@ export function handleError( errorHandler?: (error: EncryptedDynamoDBError) => void }, ): EncryptedDynamoDBError { - // Preserve FFI error code if available, otherwise use generic DynamoDB error code - // Check for FfiProtectError instance or plain error objects with code property + // Preserve the FFI error code if this is an FFI error, otherwise use the + // generic DynamoDB one. + // + // protect-ffi 0.31.0 removed the `ProtectError` class the first branch used + // to match; both bindings now throw an ordinary `Error` carrying `code`. The + // two branches therefore collapse into one, and the collapse fixes a bug: + // the old fallback accepted *any* string-valued `code` and asserted it into + // `ProtectErrorCode`, so a Node error — `ECONNRESET` from the DynamoDB + // client, say — was reported as an encryption error code. `isProtectErrorCode` + // checks the value against the known set. const errorObj = error as Record - const errorCode = - error instanceof FfiProtectError - ? error.code - : errorObj && - typeof errorObj === 'object' && - 'code' in errorObj && - typeof errorObj.code === 'string' - ? (errorObj.code as ProtectErrorCode) - : 'DYNAMODB_ENCRYPTION_ERROR' + const errorCode: ProtectErrorCode | 'DYNAMODB_ENCRYPTION_ERROR' = + isProtectErrorCode(errorObj?.code) + ? errorObj.code + : 'DYNAMODB_ENCRYPTION_ERROR' const errorMessage = error instanceof Error diff --git a/packages/stack/src/encryption/helpers/error-code.ts b/packages/stack/src/encryption/helpers/error-code.ts index 0552e84a1..9e5bae556 100644 --- a/packages/stack/src/encryption/helpers/error-code.ts +++ b/packages/stack/src/encryption/helpers/error-code.ts @@ -1,12 +1,22 @@ import { - ProtectError as FfiProtectError, + isProtectErrorCode, type ProtectErrorCode, } from '@cipherstash/protect-ffi' /** * Extracts FFI error code from an error if it's an FFI error, otherwise returns undefined. * Used to preserve specific error codes in ProtectError responses. + * + * protect-ffi 0.31.0 removed the `ProtectError` class this used to match with + * `instanceof`. Both bindings now throw an ordinary `Error` with `code` set by + * Rust, so there is no class left to match — and `instanceof` was unreliable + * regardless, since it is false across duplicate copies of a package. + * + * The check is on the code's *value*, not the presence of a `code` property: + * Node sets `code` on its own errors, so `ECONNRESET` or `MODULE_NOT_FOUND` + * would otherwise be reported as an encryption error code. */ export function getErrorCode(error: unknown): ProtectErrorCode | undefined { - return error instanceof FfiProtectError ? error.code : undefined + const code = (error as { code?: unknown } | null | undefined)?.code + return isProtectErrorCode(code) ? code : undefined } diff --git a/packages/stack/src/encryption/helpers/model-helpers.ts b/packages/stack/src/encryption/helpers/model-helpers.ts index b321ecd4a..21cec9251 100644 --- a/packages/stack/src/encryption/helpers/model-helpers.ts +++ b/packages/stack/src/encryption/helpers/model-helpers.ts @@ -75,6 +75,26 @@ interface BulkOperationPayload { [key: string]: unknown } +/** + * Strip the stack-side correlation `id` before a payload crosses into + * protect-ffi. + * + * `EncryptPayload` and `BulkDecryptPayload` have never declared an `id`. Up to + * 0.30 the Neon entry dropped unrecognised top-level keys, so passing one was + * invisible; 0.31 forwards them to Rust, which rejects the whole payload with + * ``unknown field `id` `` — every model and bulk operation at once. + * + * Nothing downstream needs it: `handleSingleModelBulkOperation` and + * `handleMultiModelBulkOperation` both correlate results to keys by ARRAY + * INDEX through `keyMap`, never by reading an id back off the payload. So this + * removes a field that was already inert, rather than moving work elsewhere. + */ +function withoutId( + items: T[], +): Omit[] { + return items.map(({ id: _id, ...rest }) => rest) +} + /** * Interface for bulk operation key mapping */ @@ -159,7 +179,7 @@ export async function decryptModelFields>( bulkDecryptPayload, (items) => decryptBulk(client, { - ciphertexts: items, + ciphertexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -213,7 +233,7 @@ export async function encryptModelFields( bulkEncryptPayload, (items) => encryptBulk(client, { - plaintexts: items, + plaintexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -271,7 +291,7 @@ export async function decryptModelFieldsWithLockContext< bulkDecryptPayload, (items) => decryptBulk(client, { - ciphertexts: items, + ciphertexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -331,7 +351,7 @@ export async function encryptModelFieldsWithLockContext( bulkEncryptPayload, (items) => encryptBulk(client, { - plaintexts: items, + plaintexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -389,7 +409,7 @@ export async function bulkEncryptModels( bulkEncryptPayload, (items) => encryptBulk(client, { - plaintexts: items, + plaintexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -446,7 +466,7 @@ export async function bulkDecryptModels>( bulkDecryptPayload, (items) => decryptBulk(client, { - ciphertexts: items, + ciphertexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -507,7 +527,7 @@ export async function bulkDecryptModelsWithLockContext< bulkDecryptPayload, (items) => decryptBulk(client, { - ciphertexts: items, + ciphertexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, @@ -571,7 +591,7 @@ export async function bulkEncryptModelsWithLockContext( bulkEncryptPayload, (items) => encryptBulk(client, { - plaintexts: items, + plaintexts: withoutId(items), unverifiedContext: auditData?.metadata, }), keyMap, diff --git a/packages/stack/src/encryption/operations/bulk-decrypt.ts b/packages/stack/src/encryption/operations/bulk-decrypt.ts index 0be8c86aa..def9de4da 100644 --- a/packages/stack/src/encryption/operations/bulk-decrypt.ts +++ b/packages/stack/src/encryption/operations/bulk-decrypt.ts @@ -18,14 +18,19 @@ import { EncryptionOperation } from './base-operation' // Drops nulls so they don't reach protect-ffi's bulk decrypt. The // dropped positions are re-inserted as null in `mapDecryptedDataToResult`. +// +// The caller's `id` is deliberately NOT forwarded — protect-ffi's +// `BulkDecryptPayload` is `{ ciphertext, lockContext? }`, and since 0.31 +// unrecognised keys reach Rust and are rejected rather than dropped. Results +// are correlated positionally by `mapDecryptedDataToResult` against the +// ORIGINAL array, so the id was never doing work here. const createDecryptPayloads = ( encryptedPayloads: BulkDecryptPayload, lockContext?: Context, ) => { return encryptedPayloads .filter(({ data }) => data !== null) - .map(({ id, data }) => ({ - id, + .map(({ data }) => ({ ciphertext: data as CipherStashEncrypted, ...(lockContext && { lockContext }), })) diff --git a/packages/stack/src/encryption/operations/bulk-encrypt.ts b/packages/stack/src/encryption/operations/bulk-encrypt.ts index 3970fea0e..92b659885 100644 --- a/packages/stack/src/encryption/operations/bulk-encrypt.ts +++ b/packages/stack/src/encryption/operations/bulk-encrypt.ts @@ -30,6 +30,13 @@ import { EncryptionOperation } from './base-operation' // client-side, because protect-ffi's behaviour on such a value is unobservable. // Callers that batch instead of looping (the v3 Drizzle `inArray`, for one) // must not lose that guard by choosing the bulk path. +// +// The caller's `id` is deliberately NOT forwarded. protect-ffi's +// `EncryptPayload` is `{ plaintext, column, table, lockContext? }` and has never +// had an `id`; 0.30 silently dropped unrecognised keys, and 0.31 forwards them +// to Rust, which rejects the payload with ``unknown field `id` ``. Nothing was +// lost by the drop either way — results are correlated back to ids positionally +// by `mapEncryptedDataToResult`, reading the ORIGINAL array, not this one. const createEncryptPayloads = ( plaintexts: BulkEncryptPayload, column: BuildableColumn, @@ -38,10 +45,9 @@ const createEncryptPayloads = ( ) => { return plaintexts .filter(({ plaintext }) => plaintext !== null) - .map(({ id, plaintext }) => { + .map(({ plaintext }) => { assertValidNumericValue(plaintext) return { - id, plaintext: plaintext as JsPlaintext, column: column.getName(), table: table.tableName, diff --git a/packages/stack/src/wasm-inline.ts b/packages/stack/src/wasm-inline.ts index 48a8cd8ca..99a225933 100644 --- a/packages/stack/src/wasm-inline.ts +++ b/packages/stack/src/wasm-inline.ts @@ -1523,17 +1523,42 @@ export async function Encryption( const strategy = resolveStrategy(clientConfig) // protect-ffi 0.25 takes a single options object with the strategy nested - // under `strategy` (0.24 passed the strategy as a separate first argument). + // under `strategy` (0.24 passed the strategy as a separate first argument); + // 0.31 renamed that field to `authStrategy` and moved the credentials into + // `clientOpts`, where the Neon entry has always had them — both entries now + // deserialize the same `NewClientOptions`. + // + // The credential fields fail loudly if they are left at the top level (0.31 + // rejects unrecognised keys there), but `keyset` does NOT: it is silently + // ignored and the client binds to the default keyset, encrypting under the + // wrong keys. This config forwards no keyset today; if one is ever added, it + // goes inside `clientOpts` with the rest. + // // `eqlVersion: 3` pins the EQL v3 wire format — this entry is v3 only, so // every encrypt/query emits v3 (a v2-mode client cannot resolve the concrete - // `eql_v3_*` domains and would fail every encrypt). + // `eql_v3_*` domains and would fail every encrypt). It stays top-level. + // + // No `as never`: 0.30's wasm declarations typed this as `(client, opts: any)`, + // so the cast was load-bearing. 0.31 types the options properly, and letting + // the compiler check the shape is the point — it is what would have caught + // the misplaced credentials above. + // + // `encryptConfig` goes through unnormalised. Under 0.30 the wasm entry only + // accepted EQL-native `cast_as` variants, so this ran `normalizeCastAs` + // first; 0.31 normalizes at the Rust deserialization boundary on both + // bindings, and its `CanonicalEncryptConfig` is documented as a shape nothing + // asks you to build. Verified against the 0.31 wasm build: `cast_as: 'string'` + // and `cast_as: 'text'` both get past config parsing to authentication, where + // 0.30 rejected the former with ``unknown variant `string` ``. const client = await wasmNewClient({ - strategy, - encryptConfig: normalizeCastAs(encryptConfig), - clientId: clientConfig.clientId, - clientKey: clientConfig.clientKey, + authStrategy: strategy, + encryptConfig, + clientOpts: { + clientId: clientConfig.clientId, + clientKey: clientConfig.clientKey, + }, eqlVersion: 3, - } as never) + }) // `INTERNAL_CONSTRUCT` is module-scoped, so this factory is the only // code that can build a `WasmEncryptionClient` — external callers hit @@ -1544,20 +1569,25 @@ export async function Encryption( /** * Convert SDK-facing `cast_as` values (`'string'`, `'number'`, …) to the - * EQL-native variants (`'text'`, `'double'`, …) that the WASM - * `newClient` accepts. + * EQL-native variants (`'text'`, `'double'`, …). * - * The Node entry of protect-ffi performs this normalization internally - * via `normalizeEncryptConfig.js`; the WASM bindings do not. Without - * this, the WASM client rejects a column config whose SDK-facing cast is - * `string` with - * `unknown variant `string`, expected one of `big_int`, …`. + * @deprecated No longer on the client-construction path, and nothing else + * calls it. Under protect-ffi 0.30 the WASM binding accepted only EQL-native + * variants — the Node entry normalized internally via + * `normalizeEncryptConfig.js` and the WASM one did not — so a config whose + * cast was `string` was rejected with + * ``unknown variant `string`, expected one of `big_int`, … ``. 0.31 normalizes + * at the Rust deserialization boundary on *both* bindings, and types the + * result as `CanonicalEncryptConfig` with the note that nothing asks a caller + * to build one. Producing that shape here is now both redundant and + * untypeable: `CanonicalEncryptConfig` is not assignable to the + * `EncryptConfig` that `newClient` declares, so the only way to keep the call + * was an assertion that misdescribes the value. * - * `toEqlCastAs` is exhaustive over the current `CastAs` union; if a new - * SDK-facing variant is added without updating that switch, this - * function throws synchronously at startup with a clear message rather - * than handing `undefined` to the WASM serde (which surfaces as an - * opaque `unknown variant 'null'` error). + * Kept, rather than deleted with its tests, because it is the only exhaustive + * consumer of `toEqlCastAs` and still throws a clear synchronous error on an + * unmapped variant. Removing both is a deliberate cleanup, not a side effect + * of a dependency upgrade. * * @internal exported for unit-test coverage of the drift-guard branch. */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5e3b3783a..6b0ad0807 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -383,8 +383,8 @@ importers: specifier: catalog:repo version: 0.42.0(@cipherstash/auth-darwin-arm64@0.42.0)(@cipherstash/auth-darwin-x64@0.42.0)(@cipherstash/auth-linux-arm64-gnu@0.42.0)(@cipherstash/auth-linux-x64-gnu@0.42.0)(@cipherstash/auth-linux-x64-musl@0.42.0)(@cipherstash/auth-win32-x64-msvc@0.42.0) '@cipherstash/protect-ffi': - specifier: 0.30.0 - version: 0.30.0 + specifier: 0.31.0 + version: 0.31.0 evlog: specifier: 1.11.0 version: 1.11.0(next@15.5.21(react-dom@19.2.3(react@19.2.3))(react@19.2.3))(react@19.2.3) @@ -482,8 +482,8 @@ importers: version: link:../stack devDependencies: '@cipherstash/protect-ffi': - specifier: 0.30.0 - version: 0.30.0 + specifier: 0.31.0 + version: 0.31.0 '@cipherstash/test-kit': specifier: workspace:* version: link:../test-kit @@ -610,8 +610,8 @@ importers: version: link:../stack devDependencies: '@cipherstash/protect-ffi': - specifier: 0.30.0 - version: 0.30.0 + specifier: 0.31.0 + version: 0.31.0 '@cipherstash/test-kit': specifier: workspace:* version: link:../test-kit @@ -988,38 +988,38 @@ packages: '@cipherstash/eql@3.0.4': resolution: {integrity: sha512-h+/1bMCuglE9pTCZLpVfPTQY2nic5JRoNl+rMQX2B/5cHCWaN5p6Q3OjEOa4Wqf/a7XWn43S/vcJLvWYYvGJuQ==} - '@cipherstash/protect-ffi-darwin-arm64@0.30.0': - resolution: {integrity: sha512-MbUfH0em2ysQxEV8+ed6442Q/EPQbAh5p17p1p4+JLUgbQJwPvxP8gm0yjeVrYBuGnZodsMqZwgOgEGt7360Ig==} + '@cipherstash/protect-ffi-darwin-arm64@0.31.0': + resolution: {integrity: sha512-ZXMh+hsgddyKxbDSPN3JbpMOoUJK8LXgX3xL+wqb2IZo8RVFuHKfV3D0WSMFb1Avl/vDVjtzXLzCAM811gy14g==} cpu: [arm64] os: [darwin] - '@cipherstash/protect-ffi-darwin-x64@0.30.0': - resolution: {integrity: sha512-GD2RXtjLvQaxWOPq2kbZEoNKTYWpDNTPQJ/tb9djpJF4RaR15g/jYhvk7Nqe2v3o6gre5RJBUpsaTQqC+G+L9A==} + '@cipherstash/protect-ffi-darwin-x64@0.31.0': + resolution: {integrity: sha512-O4KsoIgQchylk6ha3/gnetgjpBOO8catgV9WnNNHJ9gfqWW813ytBrCyKHv8zLx79pl7EkaA8UnE/U6YiOF17A==} cpu: [x64] os: [darwin] - '@cipherstash/protect-ffi-linux-arm64-gnu@0.30.0': - resolution: {integrity: sha512-R/DWCDQDDx/hRksRueJLvqxyQCSpGOZIummKqcVvBNkIhF/6Aos1SZ+zXTHAZ4gvBlfu3ovnRfMMI6eAstZDog==} + '@cipherstash/protect-ffi-linux-arm64-gnu@0.31.0': + resolution: {integrity: sha512-gYlkx5Ol7iv/cEwS/jy6wFuTI5NaO+BNJnXZnwuu4HrPsXSdcFRd+eY3Z7px9mnb+s6lED72pv6hMcNiwUFZJQ==} cpu: [arm64] os: [linux] - '@cipherstash/protect-ffi-linux-x64-gnu@0.30.0': - resolution: {integrity: sha512-uLi9kiKkJ9jUki75hgvLnDsDrJOj0RtZ8G0fVaKl4nnhZjCIVqpkn0EMlf0GOq2Bj8WiGSnca7kTGrhVkvgY9w==} + '@cipherstash/protect-ffi-linux-x64-gnu@0.31.0': + resolution: {integrity: sha512-2zhNkN+T1+FN2hYAMOiUwk4r/oun+w0pwfeV2xKJhNtLzDvAfHO30ta/qyPdNtnGm/9+LBljfl4KGvz/0eVIuw==} cpu: [x64] os: [linux] - '@cipherstash/protect-ffi-linux-x64-musl@0.30.0': - resolution: {integrity: sha512-ZrPyxc+qi9u9LsyGYbdqoi6oMPeBBx+E+/uZuAzVtp115k8CsI8xlcrmNswQ6Z74EgvAUfLb1ykTn5/P9Ge0rw==} + '@cipherstash/protect-ffi-linux-x64-musl@0.31.0': + resolution: {integrity: sha512-Ll8T6feLmChCmRXAWnsXNH+GzwWAICD+it824BhJgEma8DF8aNSar+HsazkrHNfDvVWGVcsmBARfzxoP+6Wy4A==} cpu: [x64] os: [linux] - '@cipherstash/protect-ffi-win32-x64-msvc@0.30.0': - resolution: {integrity: sha512-xew3mH+jf9RlfuIjXQ7KFWTs44Wjur//Ed1U48yFK3zmsyzkZGYVwfN8zieKpKgKqvj844s5x/0kvFXtrJDB0A==} + '@cipherstash/protect-ffi-win32-x64-msvc@0.31.0': + resolution: {integrity: sha512-fx+61P4z0o+nyPkw7bc8QxC7+wV13I5m1llpvR/dO4juYOeCkrs4q7DdxnUyTIi1GOlQNGwGAV91PMKu70j9RQ==} cpu: [x64] os: [win32] - '@cipherstash/protect-ffi@0.30.0': - resolution: {integrity: sha512-Xh8X/71ZOW6B6iEKPQlG4KrDgsKYZBw29ohsFnmMKOaTWVR7EodCE6MpSRDJ4uc+zYftp3QWsvUDEFz9gDpg6w==} + '@cipherstash/protect-ffi@0.31.0': + resolution: {integrity: sha512-qzTJZE0agyWxlukQbA5CG3//gvmohJhM8a1ehYr8tH6awbrLs6p5mgUs2ClgImO8Mv7O5XcYKdnMIdGni5vC4Q==} '@clack/core@1.4.3': resolution: {integrity: sha512-/kr3UWNtdJfxZtPgDqUOmG2pvwlmcLGheex5yiZKdwbzZJxhV+HMNR9QNmyY5cGwTNV6LrR7Jtp+KjhUAP1qBQ==} @@ -4079,34 +4079,34 @@ snapshots: '@cipherstash/eql@3.0.4': {} - '@cipherstash/protect-ffi-darwin-arm64@0.30.0': + '@cipherstash/protect-ffi-darwin-arm64@0.31.0': optional: true - '@cipherstash/protect-ffi-darwin-x64@0.30.0': + '@cipherstash/protect-ffi-darwin-x64@0.31.0': optional: true - '@cipherstash/protect-ffi-linux-arm64-gnu@0.30.0': + '@cipherstash/protect-ffi-linux-arm64-gnu@0.31.0': optional: true - '@cipherstash/protect-ffi-linux-x64-gnu@0.30.0': + '@cipherstash/protect-ffi-linux-x64-gnu@0.31.0': optional: true - '@cipherstash/protect-ffi-linux-x64-musl@0.30.0': + '@cipherstash/protect-ffi-linux-x64-musl@0.31.0': optional: true - '@cipherstash/protect-ffi-win32-x64-msvc@0.30.0': + '@cipherstash/protect-ffi-win32-x64-msvc@0.31.0': optional: true - '@cipherstash/protect-ffi@0.30.0': + '@cipherstash/protect-ffi@0.31.0': dependencies: '@neon-rs/load': 0.1.82 optionalDependencies: - '@cipherstash/protect-ffi-darwin-arm64': 0.30.0 - '@cipherstash/protect-ffi-darwin-x64': 0.30.0 - '@cipherstash/protect-ffi-linux-arm64-gnu': 0.30.0 - '@cipherstash/protect-ffi-linux-x64-gnu': 0.30.0 - '@cipherstash/protect-ffi-linux-x64-musl': 0.30.0 - '@cipherstash/protect-ffi-win32-x64-msvc': 0.30.0 + '@cipherstash/protect-ffi-darwin-arm64': 0.31.0 + '@cipherstash/protect-ffi-darwin-x64': 0.31.0 + '@cipherstash/protect-ffi-linux-arm64-gnu': 0.31.0 + '@cipherstash/protect-ffi-linux-x64-gnu': 0.31.0 + '@cipherstash/protect-ffi-linux-x64-musl': 0.31.0 + '@cipherstash/protect-ffi-win32-x64-msvc': 0.31.0 '@clack/core@1.4.3': dependencies: From 66ba41a7e8f2bb0244ed80a6670ec95926b8521f Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 10:43:43 +1000 Subject: [PATCH 2/3] ci: fail fast when CS_CLIENT_KEY is not hex-encoded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protect-ffi 0.31.0 narrowed an explicit `clientKey` to hex only. It used to be decoded by `SecretKey::from_hex`, which falls back to standard padded base64 — the encoding `~/.cipherstash/secretkey.json` stores on disk — so a base64 value pasted into the secret worked. The Neon entry forwards `CS_CLIENT_KEY` straight through as `clientKey`, so this is the exact value that now has to be hex. Without a check, a base64 secret presents as all six credentialed workflows failing simultaneously at client construction, with `invalid clientKey: expected a hex-encoded key` and nothing else — protect-ffi discards the decode error on purpose, because hex's own message names the offending character and its offset, which would put part of a live key into logs and error trackers. Six unrelated-looking red jobs and a message that does not mention encoding is a bad afternoon. It goes in `require-cs-secrets` because every workflow that sets CS_CLIENT_KEY already calls that action — verified across all six — so this is one edit rather than six, and it sits next to the existing presence check it naturally follows. The key is never echoed: the check is a charset and even-length test, and both the success and failure messages report only the length. --- .changeset/lucky-cows-repeat.md | 15 +++++++++ .github/actions/require-cs-secrets/action.yml | 33 +++++++++++++++++++ skills/stash-auth/SKILL.md | 20 +++++++++++ 3 files changed, 68 insertions(+) create mode 100644 .changeset/lucky-cows-repeat.md diff --git a/.changeset/lucky-cows-repeat.md b/.changeset/lucky-cows-repeat.md new file mode 100644 index 000000000..8b6399ceb --- /dev/null +++ b/.changeset/lucky-cows-repeat.md @@ -0,0 +1,15 @@ +--- +'stash': patch +--- + +Document in the bundled `stash-auth` skill that `CS_CLIENT_KEY` must be +hex-encoded. Hex is what `stash env` emits and what the skill's variable table +already stated, but older client versions also accepted the base64 spelling +stored in `~/.cipherstash/secretkey.json`, so a key copied out of that file +used to work. It is now rejected at client construction, with a message that +deliberately withholds detail — so the skill names the symptom and the fix. + +The recovery advice is split by entry point: falling back to the profile store +works on the native entry, but not on `@cipherstash/stack/wasm-inline`, where +`clientId` and `clientKey` are required config and the target runtimes have no +profile store to read. Re-encoding as hex is the fix that works on both. diff --git a/.github/actions/require-cs-secrets/action.yml b/.github/actions/require-cs-secrets/action.yml index 95fee4238..0ee797cf8 100644 --- a/.github/actions/require-cs-secrets/action.yml +++ b/.github/actions/require-cs-secrets/action.yml @@ -41,3 +41,36 @@ runs: if [ "$missing" -ne 0 ]; then exit 1 fi + + # protect-ffi 0.31.0 decodes an explicit `clientKey` as hex ONLY. It used + # to go through `SecretKey::from_hex`, which falls back to standard padded + # base64 — the encoding `~/.cipherstash/secretkey.json` stores — so a + # base64 value pasted into CS_CLIENT_KEY worked. It is now rejected with + # `invalid clientKey: expected a hex-encoded key`, and deliberately nothing + # more: the underlying hex error names the offending character and its + # offset, which would put part of a live key into logs. + # + # Without this check that lands as every credentialed job failing at client + # construction at once, with a message that says nothing about encoding + # being the problem. The env var is forwarded as `clientKey` by the Neon + # entry, so this is the exact value that gets decoded. + # + # The key itself is never echoed — only its length and a pass/fail. + - name: Assert CS_CLIENT_KEY is hex-encoded + shell: bash + env: + CS_CLIENT_KEY: ${{ inputs.client-key }} + run: | + # `[[ =~ ]]` rather than a pipe into grep, because grep matches per + # LINE and `-q` succeeds when ANY line does: a value of + # "deadbeef\n" passed the old spelling whenever its total + # length was even, which is the one shape this step exists to reject. + # Bash anchors the whole string — a newline is not in the class, and + # `$` here is end-of-string, not end-of-line. + if [[ "$CS_CLIENT_KEY" =~ ^[0-9a-fA-F]+$ ]] \ + && [ $(( ${#CS_CLIENT_KEY} % 2 )) -eq 0 ]; then + echo "CS_CLIENT_KEY is hex (${#CS_CLIENT_KEY} chars)." + exit 0 + fi + echo "::error::CS_CLIENT_KEY is not hex-encoded (${#CS_CLIENT_KEY} chars). protect-ffi 0.31+ decodes clientKey as hex only — the base64 fallback was removed. A base64 key (uppercase, '+', '/', or trailing '=') must be re-encoded as hex, or the key read from the profile store instead. Every credentialed suite will otherwise fail at client construction with 'invalid clientKey: expected a hex-encoded key'." + exit 1 diff --git a/skills/stash-auth/SKILL.md b/skills/stash-auth/SKILL.md index 8b206dc57..5a27c953c 100644 --- a/skills/stash-auth/SKILL.md +++ b/skills/stash-auth/SKILL.md @@ -206,6 +206,26 @@ access key is minted with the member role — the CLI never mints admin keys — and is shown exactly once. Give each environment its own minted set; see `stash-deployment` for where each environment's credentials live. +> **`CS_CLIENT_KEY` must be hex.** Hex is what `stash env` emits and what this +> table has always documented, but older versions also accepted the base64 +> spelling that `~/.cipherstash/secretkey.json` stores on disk — so a key +> copied out of that file worked. It no longer does: the client now rejects it +> at construction with `invalid clientKey: expected a hex-encoded key`, and +> the message says nothing further on purpose (the underlying decode error +> names a character of the key and its offset). +> +> If every operation starts failing at construction after an upgrade, check +> the encoding before anything else. **Re-encode the key as hex** — that fix +> works on both entry points. +> +> On the native entry you may instead drop `CS_CLIENT_KEY` and let the client +> read the profile store, which is unaffected: only an explicitly supplied key +> is hex-only. That escape hatch does **not** exist on +> `@cipherstash/stack/wasm-inline`, where `clientId` and `clientKey` are +> required config and the edge runtimes it targets have no `~/.cipherstash` to +> read. Dropping the variable there replaces one construction failure with +> another; re-encoding is the only fix. + ## Client lifetime (user-scoped strategies) An `OidcFederationStrategy` instance holds **one cached CTS token**: From 49325032a84f9e438f09d771c29ccd8dd9ab9e66 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Thu, 6 Aug 2026 11:27:14 +1000 Subject: [PATCH 3/3] test(stack): pin handleError and getErrorCode against foreign error codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protect-ffi 0.31.0 removed the `ProtectError` class both helpers matched with `instanceof`, and the collapse of that branch fixed a bug: the old fallback accepted ANY string-valued `code` and asserted it into `ProtectErrorCode`, so a Node error (`ECONNRESET` off a dropped socket) was handed back as an encryption error code. A caller keying retry-vs-fail off `error.code` read a transport fault as a crypto fault. That fix only had predicate-level coverage — `isProtectErrorCode` was tested directly, but its two call sites were reachable only through live ZeroKMS. Both new blocks fail against the pre-0.31 guard and pass against the current one, credential-free. - `handleError` joins `throwPreservingCode` in the DynamoDB pure-helper suite; they are the two ends of one seam (the latter exists so the code survives `withResult`'s wrapping for the former to read back). Also covers the message-extraction ladder and the errorHandler/logger fan-out. - `getErrorCode` joins `getErrorMessage` in the error-helper suite, including the null/undefined inputs its optional chain exists for. --- .../dynamodb/resolve-decrypt.test.ts | 137 +++++++++++++++++- .../stack/__tests__/error-helpers.test.ts | 41 ++++++ 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts b/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts index 77f31bf4e..4d80c8f6e 100644 --- a/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts +++ b/packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts @@ -13,12 +13,26 @@ * the mirror, and the chainable half of its coverage matters most — the native * clients' encrypt audit trail has no other credential-free test. * + * `throwPreservingCode` and `handleError` are the two ends of the same seam — + * the first exists only so the FFI error code survives `withResult`'s wrapping + * long enough for the second to read it back off the rethrown Error — so they + * are covered here too. + * * Every branch was previously reachable only through live ZeroKMS; these move * that assurance onto the pure CI lane. No credentials, no network. */ import type { Result } from '@byteslice/result' -import { afterEach, describe, expect, it, vi } from 'vitest' import { + afterEach, + beforeEach, + describe, + expect, + it, + type MockInstance, + vi, +} from 'vitest' +import { + handleError, resolveDecryptResult, resolveEncryptResult, throwPreservingCode, @@ -349,3 +363,124 @@ describe('throwPreservingCode', () => { } }) }) + +/** + * The adapter's error funnel — every operation's `catch` ends here, and the + * code it stamps on the way out is what a caller branches on. + * + * protect-ffi 0.31.0 removed the `ProtectError` class `handleError`'s first + * branch matched with `instanceof`, collapsing the two branches into one — and + * the collapse fixed a bug. The old fallback accepted ANY string-valued `code` + * and asserted it into `ProtectErrorCode`, so a Node error arriving from the + * DynamoDB client (`ECONNRESET` on a dropped socket) was handed back as an + * encryption error code, and a caller keying retry-vs-fail off `error.code` + * read a transport fault as a crypto fault. `isProtectErrorCode` checks the + * value against the known set. + * + * That fix had no test of its own: the predicate was covered directly + * (`error-codes.test.ts`), but its use here — the whole point — was reachable + * only through live ZeroKMS. These pin it credential-free. + */ +describe('handleError', () => { + let errorLog: MockInstance + + beforeEach(() => { + // `handleError` always calls the shared logger at `error` level, which the + // default `STASH_STACK_LOG` emits. Silence it so the reporter stays clean. + // The outer `afterEach` un-patches. + errorLog = vi.spyOn(logger, 'error').mockImplementation(() => {}) + }) + + it('does not surface a foreign error code as an encryption error code', () => { + const error = handleError( + { code: 'ECONNRESET', message: 'socket hang up' }, + 'decryptModel', + ) + + expect(error.code).toBe('DYNAMODB_ENCRYPTION_ERROR') + expect(error.name).toBe('EncryptedDynamoDBError') + expect(error.details).toEqual({ context: 'decryptModel' }) + }) + + it('preserves a code the FFI actually emits', () => { + // `UNKNOWN_COLUMN` is a real member of `PROTECT_ERROR_CODES` in + // protect-ffi 0.31.0 — a code the caller is meant to branch on, so the + // guard must not flatten it into the generic one. + const error = handleError( + { code: 'UNKNOWN_COLUMN', message: 'no such column' }, + 'encryptModel', + ) + + expect(error.code).toBe('UNKNOWN_COLUMN') + }) + + it('falls back to the generic code when there is no usable code at all', () => { + for (const raw of [ + {}, + new Error('plain'), + { code: 42 }, + { code: null }, + 'a bare string', + ]) { + expect(handleError(raw, 'decryptModel').code).toBe( + 'DYNAMODB_ENCRYPTION_ERROR', + ) + } + }) + + it('survives the round trip a real failure takes through throwPreservingCode', () => { + // The production path: an operation's `{ failure }` is rethrown by + // `throwPreservingCode` as an Error carrying `code`, `withResult` catches + // it, and `handleError` reads the code back. Both codes must come out the + // far side classified the same way they went in. + const rethrow = (code: string) => { + try { + throwPreservingCode({ message: 'boom', code }) + } catch (error) { + return handleError(error, 'bulkDecryptModels') + } + return expect.unreachable('should have thrown') + } + + expect(rethrow('UNKNOWN_COLUMN').code).toBe('UNKNOWN_COLUMN') + expect(rethrow('ECONNRESET').code).toBe('DYNAMODB_ENCRYPTION_ERROR') + }) + + it('extracts the message from an Error, a plain object, or anything else', () => { + expect( + handleError(new Error('from an Error'), 'decryptModel').message, + ).toBe('from an Error') + expect( + handleError({ message: 'from an object' }, 'decryptModel').message, + ).toBe('from an object') + // A non-string `message` is not a message; fall through to `String(error)`. + expect(handleError({ message: 42 }, 'decryptModel').message).toBe( + '[object Object]', + ) + expect(handleError('bare string', 'decryptModel').message).toBe( + 'bare string', + ) + expect(handleError(null, 'decryptModel').message).toBe('null') + }) + + it('hands the constructed error to both the errorHandler and the caller logger', () => { + const seen: unknown[] = [] + const callerLog = { error: vi.fn() } + + const error = handleError( + { code: 'ECONNRESET', message: 'socket hang up' }, + 'decryptModel', + { errorHandler: (e) => seen.push(e), logger: callerLog }, + ) + + // Identity, not structural equality: the handler must receive the SAME + // object the caller gets back, so a handler reading `.code` sees the + // classified one. + expect(seen).toHaveLength(1) + expect(seen[0]).toBe(error) + expect(callerLog.error).toHaveBeenCalledWith('Error in decryptModel', error) + expect(errorLog).toHaveBeenCalledWith( + expect.stringContaining('DynamoDB error in decryptModel'), + ) + }) +}) diff --git a/packages/stack/__tests__/error-helpers.test.ts b/packages/stack/__tests__/error-helpers.test.ts index adbb37360..8cdc5c1fb 100644 --- a/packages/stack/__tests__/error-helpers.test.ts +++ b/packages/stack/__tests__/error-helpers.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest' +import { getErrorCode } from '@/encryption/helpers/error-code' import { EncryptionErrorTypes, getErrorMessage } from '@/errors' describe('error helpers', () => { @@ -95,4 +96,44 @@ describe('error helpers', () => { expect(getErrorMessage(error)).toBe('') }) }) + + // ------------------------------------------------------- + // getErrorCode + // ------------------------------------------------------- + // + // The `code` half of the pair above, and the one with a sharp edge: + // `getErrorMessage` can safely stringify anything, but a code is a value a + // caller BRANCHES on. protect-ffi 0.31.0 removed the `ProtectError` class + // this matched with `instanceof`, so the check moved to the code's value — + // deliberately not to the presence of a `code` property, because Node sets + // `code` on its own errors. Every failing operation in `encryption/operations` + // passes its caught error through here, so a presence check would report + // `ECONNRESET` from a dropped socket as an encryption error code. + describe('getErrorCode', () => { + it('returns undefined for a Node error code', () => { + expect(getErrorCode({ code: 'ECONNRESET' })).toBeUndefined() + expect(getErrorCode({ code: 'MODULE_NOT_FOUND' })).toBeUndefined() + }) + + it('returns a code the FFI actually emits', () => { + // A real member of `PROTECT_ERROR_CODES` in protect-ffi 0.31.0. + expect(getErrorCode({ code: 'UNKNOWN_COLUMN' })).toBe('UNKNOWN_COLUMN') + }) + + it('reads the code off a real Error, not just a plain object', () => { + const error = Object.assign(new Error('boom'), { + code: 'INVALID_JSON_PATH', + }) + expect(getErrorCode(error)).toBe('INVALID_JSON_PATH') + }) + + it('returns undefined for null, undefined, and a code-less error', () => { + // The implementation optional-chains for exactly this: a `catch` variable + // is `unknown`, and `throw null` / `throw undefined` are legal JS. + expect(getErrorCode(null)).toBeUndefined() + expect(getErrorCode(undefined)).toBeUndefined() + expect(getErrorCode(new Error('no code'))).toBeUndefined() + expect(getErrorCode('a bare string')).toBeUndefined() + }) + }) })