fix(stack): consume protect-ffi 0.31.0

.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.

.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.

.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<anything>" 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

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",

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",

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'),
+    )
+  })
+})

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

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] }]),

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)
     })
   })
 

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()
+    })
+  })
 })