chore: address CodeRabbit review feedback on PR #473

Fixes the actionable findings CodeRabbit raised on the rebased PR
state.

- `.github/workflows/tests.yml` — the comment justifying the
  postgres-eql:17-2.3.1 image pin still said "protect-ffi 0.21.x".
  After the 0.23.0 bump that's a stale lockstep reference; flip to
  0.23.x so future maintainers don't read it backwards.
- `packages/schema/src/index.ts` — tighten `encryptConfigSchema.v`
  from `z.number()` to `z.literal(1)`. `buildEncryptConfig` only
  emits `1`, and protect-ffi 0.22.0+ rejects any other value at
  `newClient` with `UNSUPPORTED_CONFIG_VERSION`; enforcing it at
  the zod layer fails bad configs earlier (and surfaces a cleaner
  error than crossing into the native module first).
- `packages/{protect,stack}/__tests__/number-protect.test.ts` —
  the "// Forward compatibility: new encryptions should NOT have
  k field" comment contradicted the asserting branch right below
  (post-EQL-v2.3 the discriminator IS present). Reword to match
  the actual contract and tighten `toHaveProperty('k')` to
  `toHaveProperty('k', 'ct')`.
- `packages/{protect,stack}/__tests__/json-protect.test.ts` —
  same tightening for all sites. These columns are typed
  `dataType('json')` without `searchableJson()`, so no ste_vec
  index is configured and the FFI emits a scalar payload — `'ct'`
  is the correct discriminator at every assertion site.

Verified: pnpm build (10/10 packages, full TURBO), pnpm test
(14/14 packages, 1876 passed | 20 skipped).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: calvinbrewer

.github/workflows/tests.yml

@@ -16,7 +16,7 @@ jobs:
     # Postgres + EQL for the integration tests. Official EQL image —
     # PostgreSQL 17 with EQL pre-installed via /docker-entrypoint-initdb.d.
     # Pinned to eql-2.3.1 to match the EQL payload format the code emits
-    # (protect-ffi 0.21.x); bump in lockstep with the protect-ffi upgrade.
+    # (protect-ffi 0.23.x); bump in lockstep with the protect-ffi upgrade.
     services:
       postgres:
         image: ghcr.io/cipherstash/postgres-eql:17-2.3.1

packages/protect/__tests__/json-protect.test.ts

@@ -55,7 +55,7 @@ describe('JSON encryption and decryption', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -107,7 +107,7 @@ describe('JSON encryption and decryption', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -149,7 +149,7 @@ describe('JSON encryption and decryption', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -176,7 +176,7 @@ describe('JSON encryption and decryption', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -214,9 +214,9 @@ describe('JSON model encryption and decryption', () => {
     }
 
     // Verify encrypted fields
-    expect(encryptedModel.data.email).toHaveProperty('k')
-    expect(encryptedModel.data.address).toHaveProperty('k')
-    expect(encryptedModel.data.json).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.address).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.json).toHaveProperty('k', 'ct')
 
     // Verify non-encrypted fields remain unchanged
     expect(encryptedModel.data.id).toBe('1')
@@ -254,8 +254,8 @@ describe('JSON model encryption and decryption', () => {
     }
 
     // Verify encrypted fields
-    expect(encryptedModel.data.email).toHaveProperty('k')
-    expect(encryptedModel.data.address).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.address).toHaveProperty('k', 'ct')
     expect(encryptedModel.data.json).toBeNull()
 
     const decryptedResult = await protectClient.decryptModel<User>(
@@ -289,8 +289,8 @@ describe('JSON model encryption and decryption', () => {
     }
 
     // Verify encrypted fields
-    expect(encryptedModel.data.email).toHaveProperty('k')
-    expect(encryptedModel.data.address).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.address).toHaveProperty('k', 'ct')
     expect(encryptedModel.data.json).toBeUndefined()
 
     const decryptedResult = await protectClient.decryptModel<User>(
@@ -326,13 +326,13 @@ describe('JSON bulk encryption and decryption', () => {
     expect(encryptedData.data).toHaveLength(3)
     expect(encryptedData.data[0]).toHaveProperty('id', 'user1')
     expect(encryptedData.data[0]).toHaveProperty('data')
-    expect(encryptedData.data[0].data).toHaveProperty('k')
+    expect(encryptedData.data[0].data).toHaveProperty('k', 'ct')
     expect(encryptedData.data[1]).toHaveProperty('id', 'user2')
     expect(encryptedData.data[1]).toHaveProperty('data')
-    expect(encryptedData.data[1].data).toHaveProperty('k')
+    expect(encryptedData.data[1].data).toHaveProperty('k', 'ct')
     expect(encryptedData.data[2]).toHaveProperty('id', 'user3')
     expect(encryptedData.data[2]).toHaveProperty('data')
-    expect(encryptedData.data[2].data).toHaveProperty('k')
+    expect(encryptedData.data[2].data).toHaveProperty('k', 'ct')
 
     // Now decrypt the data
     const decryptedData = await protectClient.bulkDecrypt(encryptedData.data)
@@ -380,13 +380,13 @@ describe('JSON bulk encryption and decryption', () => {
     expect(encryptedData.data).toHaveLength(3)
     expect(encryptedData.data[0]).toHaveProperty('id', 'user1')
     expect(encryptedData.data[0]).toHaveProperty('data')
-    expect(encryptedData.data[0].data).toHaveProperty('k')
+    expect(encryptedData.data[0].data).toHaveProperty('k', 'ct')
     expect(encryptedData.data[1]).toHaveProperty('id', 'user2')
     expect(encryptedData.data[1]).toHaveProperty('data')
     expect(encryptedData.data[1].data).toBeNull()
     expect(encryptedData.data[2]).toHaveProperty('id', 'user3')
     expect(encryptedData.data[2]).toHaveProperty('data')
-    expect(encryptedData.data[2].data).toHaveProperty('k')
+    expect(encryptedData.data[2].data).toHaveProperty('k', 'ct')
 
     // Now decrypt the data
     const decryptedData = await protectClient.bulkDecrypt(encryptedData.data)
@@ -447,12 +447,12 @@ describe('JSON bulk encryption and decryption', () => {
     }
 
     // Verify encrypted fields for each model
-    expect(encryptedModels.data[0].email).toHaveProperty('k')
-    expect(encryptedModels.data[0].address).toHaveProperty('k')
-    expect(encryptedModels.data[0].json).toHaveProperty('k')
-    expect(encryptedModels.data[1].email).toHaveProperty('k')
-    expect(encryptedModels.data[1].address).toHaveProperty('k')
-    expect(encryptedModels.data[1].json).toHaveProperty('k')
+    expect(encryptedModels.data[0].email).toHaveProperty('k', 'ct')
+    expect(encryptedModels.data[0].address).toHaveProperty('k', 'ct')
+    expect(encryptedModels.data[0].json).toHaveProperty('k', 'ct')
+    expect(encryptedModels.data[1].email).toHaveProperty('k', 'ct')
+    expect(encryptedModels.data[1].address).toHaveProperty('k', 'ct')
+    expect(encryptedModels.data[1].json).toHaveProperty('k', 'ct')
 
     // Verify non-encrypted fields remain unchanged
     expect(encryptedModels.data[0].id).toBe('1')
@@ -511,7 +511,7 @@ describe('JSON encryption with lock context', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient
       .decrypt(ciphertext.data)
@@ -557,8 +557,8 @@ describe('JSON encryption with lock context', () => {
     }
 
     // Verify encrypted fields
-    expect(encryptedModel.data.email).toHaveProperty('k')
-    expect(encryptedModel.data.json).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.json).toHaveProperty('k', 'ct')
 
     const decryptedResult = await protectClient
       .decryptModel(encryptedModel.data)
@@ -606,10 +606,10 @@ describe('JSON encryption with lock context', () => {
     expect(encryptedData.data).toHaveLength(2)
     expect(encryptedData.data[0]).toHaveProperty('id', 'user1')
     expect(encryptedData.data[0]).toHaveProperty('data')
-    expect(encryptedData.data[0].data).toHaveProperty('k')
+    expect(encryptedData.data[0].data).toHaveProperty('k', 'ct')
     expect(encryptedData.data[1]).toHaveProperty('id', 'user2')
     expect(encryptedData.data[1]).toHaveProperty('data')
-    expect(encryptedData.data[1].data).toHaveProperty('k')
+    expect(encryptedData.data[1].data).toHaveProperty('k', 'ct')
 
     // Decrypt with lock context
     const decryptedData = await protectClient
@@ -670,8 +670,8 @@ describe('JSON nested object encryption', () => {
     }
 
     // Verify encrypted fields
-    expect(encryptedModel.data.email).toHaveProperty('k')
-    expect(encryptedModel.data.metadata?.profile).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
+    expect(encryptedModel.data.metadata?.profile).toHaveProperty('k', 'ct')
     expect(encryptedModel.data.metadata?.settings?.preferences).toHaveProperty(
       'c',
     )
@@ -714,7 +714,7 @@ describe('JSON nested object encryption', () => {
     }
 
     // Verify null fields are preserved
-    expect(encryptedModel.data.email).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
     expect(encryptedModel.data.metadata?.profile).toBeNull()
     expect(encryptedModel.data.metadata?.settings?.preferences).toBeNull()
 
@@ -753,7 +753,7 @@ describe('JSON nested object encryption', () => {
     }
 
     // Verify undefined fields are preserved
-    expect(encryptedModel.data.email).toHaveProperty('k')
+    expect(encryptedModel.data.email).toHaveProperty('k', 'ct')
     expect(encryptedModel.data.metadata?.profile).toBeUndefined()
     expect(encryptedModel.data.metadata?.settings?.preferences).toBeUndefined()
 
@@ -799,7 +799,7 @@ describe('JSON edge cases and error handling', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -847,7 +847,7 @@ describe('JSON edge cases and error handling', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -948,7 +948,7 @@ describe('JSON advanced scenarios', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -975,7 +975,7 @@ describe('JSON advanced scenarios', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1010,7 +1010,7 @@ describe('JSON advanced scenarios', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1045,7 +1045,7 @@ describe('JSON advanced scenarios', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1081,7 +1081,7 @@ describe('JSON advanced scenarios', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1140,7 +1140,7 @@ describe('JSON error handling and edge cases', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1169,7 +1169,7 @@ describe('JSON error handling and edge cases', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 
@@ -1206,7 +1206,7 @@ describe('JSON error handling and edge cases', () => {
     }
 
     // Verify encrypted field
-    expect(ciphertext.data).toHaveProperty('k')
+    expect(ciphertext.data).toHaveProperty('k', 'ct')
 
     const plaintext = await protectClient.decrypt(ciphertext.data)
 

packages/protect/__tests__/number-protect.test.ts

@@ -304,10 +304,10 @@ describe('Bulk encryption and decryption', () => {
     expect(encryptedData.data[2]).toHaveProperty('data')
     expect(encryptedData.data[2].data).toHaveProperty('c')
 
-    // Forward compatibility: new encryptions should NOT have k field
-    expect(encryptedData.data[0].data).toHaveProperty('k')
-    expect(encryptedData.data[1].data).toHaveProperty('k')
-    expect(encryptedData.data[2].data).toHaveProperty('k')
+    // EQL v2.3: scalar encryptions carry the `k: 'ct'` discriminator
+    expect(encryptedData.data[0].data).toHaveProperty('k', 'ct')
+    expect(encryptedData.data[1].data).toHaveProperty('k', 'ct')
+    expect(encryptedData.data[2].data).toHaveProperty('k', 'ct')
 
     // Verify all encrypted values are different
     const getCiphertext = (data: { c?: unknown } | null | undefined) => data?.c

packages/schema/src/index.ts

@@ -89,8 +89,12 @@ const tableSchema = z.record(columnSchema).default({})
 
 const tablesSchema = z.record(tableSchema).default({})
 
+// `v` is locked to `1` because protect-ffi `0.22.0+` rejects any other value at
+// `newClient` with `UNSUPPORTED_CONFIG_VERSION`. Enforcing it here means a bad
+// hand-rolled config fails at zod-validation time instead of crossing into the
+// native module first.
 export const encryptConfigSchema = z.object({
-  v: z.number(),
+  v: z.literal(1),
   tables: tablesSchema,
 })