fix(stack): code-review round on #676 — validation parity and test hardening

Product fix: the WASM encryptQuery/encryptQueryBulk now run the same
pre-FFI guards as every other query path (assertValidNumericValue,
assertValueIndexCompatibility) — NaN/Infinity/out-of-int64 bigint and
numeric-on-match-index fail with the named errors the Node entry raises
instead of an opaque serde failure or a silently no-match term. Both
paths now share one toFfiQueryTerm() so the serde-omission and ope-swap
subtleties can't drift between single and bulk; the deliberate as-never
casts at the FFI seam carry biome-ignore reasons per AGENTS.md.

Test hardening from the same review:
- FFI stub gains encryptQuery/encryptQueryBulk so unmocked unit tests
  fail with the stub's named error, not 'is not a function'
- query-types.test.ts gains roundtrip's ffi:false permissions pin
- wasm adapter expectRejected discriminates capability rejections from
  infrastructure failures (mirrors the Supabase adapter's doctrine)
- empty in/notIn lists throw (parity with drizzle inArrayOp) instead of
  rendering 'WHERE ()'
- independent ZeroKMS round-trips run concurrently (per-field encrypts,
  bulk-insert rows, between lo/hi terms)
- dropped the redundant '| null' from encryptQuery's signature and the
  gratuitous 'null as unknown as string' casts it appeared to justify
- fixed the nonexistent .env.example pointer in both Deno suites and
  the stale skip-behavior comments in tests.yml/integration-drizzle.yml
- three new unit pins for the validation behavior (828 total green)

Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w

Author: coderdan

.github/workflows/integration-drizzle.yml

@@ -129,8 +129,9 @@ jobs:
       # The Drizzle adapter suites (incl. the Clerk-federated lock-context one)
       # now live in @cipherstash/stack-drizzle (run below via its own
       # test:integration). This glob scopes what still lives in @cipherstash/stack:
-      # the adapter-agnostic `shared/` core suites and the model-path
-      # `identity/matrix-identity` suite.
+      # the adapter-agnostic `shared/` core suites, the model-path
+      # `identity/matrix-identity` suite, and the `wasm/` family suite (the
+      # `@cipherstash/stack/wasm-inline` adapter over the shared v3 matrix).
       CS_IT_SUITE: >-
         integration/shared/**/*.integration.test.ts,
         integration/identity/**/*.integration.test.ts,
@@ -172,7 +173,7 @@ jobs:
       # different package now). Its globalSetup calls the same EQL v3 install, but
       # `isInstalled` short-circuits against the DB the first invocation already
       # provisioned — a fast no-op check, not a second schema apply.
-      - name: Shared core + identity integration suites
+      - name: Shared core + identity + wasm integration suites
         run: pnpm --filter @cipherstash/stack run test:integration
 
       - name: Stop ${{ matrix.db }}

.github/workflows/tests.yml

@@ -294,10 +294,10 @@ jobs:
       - name: Build stack
         run: pnpm exec turbo run build --filter @cipherstash/stack
 
-      # roundtrip.test.ts skips itself (Deno.test.ignore) when any of the four
-      # CS_* env vars is missing — fine locally, but in CI a silent skip would
-      # let a rotated / cleared secret hide a real WASM regression behind a
-      # green job. Fail loudly instead.
+      # The e2e/wasm suites FAIL when any of the four CS_* env vars is
+      # missing (requireEnv throws — no skip gating), so a rotated / cleared
+      # secret can't hide a real WASM regression behind a green job. This
+      # preflight just fails faster, before the Deno module downloads.
       - name: Require CipherStash secrets
         uses: ./.github/actions/require-cs-secrets
         with:

e2e/wasm/query-types.test.ts

@@ -56,8 +56,9 @@ function requireEnv(): Record<(typeof REQUIRED_ENV)[number], string> {
     // harness — no skipIf credential gates).
     throw new Error(
       `Missing required env: ${missing.join(', ')}. This suite needs real ` +
-        'CipherStash credentials — set the CS_* variables (see .env.example) ' +
-        'or run via the CI job, which injects them.',
+        'CipherStash credentials — export the four CS_* variables (or put them ' +
+        'in a repo-root .env; see AGENTS.md "Environment variables") or run ' +
+        'via the CI job, which injects them.',
     )
   }
   return values
@@ -93,6 +94,16 @@ function assertContainmentNeedle(term: unknown, label: string) {
 
 Deno.test({
   name: 'stack/wasm-inline: encryptQuery covers every v3 query type',
+  permissions: {
+    env: true,
+    net: true,
+    read: true,
+    sys: true,
+    // No FFI permission, same pin as roundtrip.test.ts: if protect-ffi ever
+    // silently fell back to a native binding under Deno, the call would
+    // reject — so a green run proves the WASM path minted every term.
+    ffi: false,
+  },
   async fn() {
     const env = requireEnv()
     const client = await Encryption({
@@ -190,7 +201,7 @@ Deno.test({
         queryType: 'equality',
       },
       {
-        value: null as unknown as string,
+        value: null,
         table: catalog,
         column: catalog.email,
       },

e2e/wasm/roundtrip.test.ts

@@ -51,8 +51,9 @@ function requireEnv(): Record<(typeof REQUIRED_ENV)[number], string> {
     // harness — no skipIf credential gates).
     throw new Error(
       `Missing required env: ${missing.join(', ')}. This suite needs real ` +
-        'CipherStash credentials — set the CS_* variables (see .env.example) ' +
-        'or run via the CI job, which injects them.',
+        'CipherStash credentials — export the four CS_* variables (or put them ' +
+        'in a repo-root .env; see AGENTS.md "Environment variables") or run ' +
+        'via the CI job, which injects them.',
     )
   }
   return values
@@ -159,7 +160,7 @@ Deno.test({
     // Bulk form is position-stable, nulls pass through.
     const bulk = await client.encryptQueryBulk([
       { value: plaintext, column: users.email, table: users },
-      { value: null as unknown as string, column: users.email, table: users },
+      { value: null, column: users.email, table: users },
     ])
     assertEquals(bulk.length, 2)
     assertExists(bulk[0])

packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts

@@ -20,6 +20,18 @@ export const encrypt = (): never => {
   )
 }
 
+export const encryptQuery = (): never => {
+  throw new Error(
+    '[test stub]: protect-ffi/wasm-inline encryptQuery not implemented',
+  )
+}
+
+export const encryptQueryBulk = (): never => {
+  throw new Error(
+    '[test stub]: protect-ffi/wasm-inline encryptQueryBulk not implemented',
+  )
+}
+
 export const isEncrypted = (): boolean => false
 
 export const newClient = (): never => {

packages/stack/__tests__/wasm-inline-query.test.ts

@@ -170,6 +170,48 @@ describe('WasmEncryptionClient.encryptQuery', () => {
     ).toBeNull()
     expect(ffi.encryptQuery).not.toHaveBeenCalled()
   })
+
+  // The same pre-FFI guards the native client runs — an invalid value must
+  // fail with the NAMED error before any FFI/network crossing, not with an
+  // opaque serde failure (or a silently no-match term) from inside WASM.
+  it('rejects NaN with the named validation error before the FFI', async () => {
+    const c = await client()
+    await expect(
+      c.encryptQuery(Number.NaN, {
+        table: users,
+        column: users.age,
+        queryType: 'orderAndRange',
+      }),
+    ).rejects.toThrow('[encryption]: Cannot encrypt NaN value')
+    expect(ffi.encryptQuery).not.toHaveBeenCalled()
+  })
+
+  it('rejects a numeric value against a match index before the FFI', async () => {
+    const c = await client()
+    await expect(
+      c.encryptQuery(42, {
+        table: users,
+        column: users.bio,
+        queryType: 'freeTextSearch',
+      }),
+    ).rejects.toThrow(/Cannot use 'match' index with numeric value/)
+    expect(ffi.encryptQuery).not.toHaveBeenCalled()
+  })
+
+  it('applies the same validation on the bulk path', async () => {
+    const c = await client()
+    await expect(
+      c.encryptQueryBulk([
+        {
+          value: Number.POSITIVE_INFINITY,
+          table: users,
+          column: users.age,
+          queryType: 'orderAndRange',
+        },
+      ]),
+    ).rejects.toThrow('[encryption]: Cannot encrypt Infinity value')
+    expect(ffi.encryptQueryBulk).not.toHaveBeenCalled()
+  })
 })
 
 describe('WasmEncryptionClient.encryptQueryBulk', () => {
@@ -181,7 +223,7 @@ describe('WasmEncryptionClient.encryptQueryBulk', () => {
     const c = await client()
     const out = await c.encryptQueryBulk([
       { value: 'a@b.com', table: users, column: users.email },
-      { value: null as unknown as string, table: users, column: users.email },
+      { value: null, table: users, column: users.email },
       {
         value: 'needle',
         table: users,
@@ -207,7 +249,7 @@ describe('WasmEncryptionClient.encryptQueryBulk', () => {
   it('short-circuits an all-null batch', async () => {
     const c = await client()
     const out = await c.encryptQueryBulk([
-      { value: null as unknown as string, table: users, column: users.email },
+      { value: null, table: users, column: users.email },
     ])
     expect(out).toEqual([null])
     expect(ffi.encryptQueryBulk).not.toHaveBeenCalled()

packages/stack/integration/wasm/adapter.ts

@@ -225,14 +225,18 @@ export function makeWasmAdapter(): IntegrationAdapter {
 
   async function encryptRow(row: PlainRow): Promise<Record<string, unknown>> {
     const assignments: Record<string, unknown> = { row_key: row.rowKey }
-    for (const [slug, value] of Object.entries(row.values)) {
-      const encrypted = await client.encrypt(toWasmPlaintext(value), {
-        table: tableSchema,
-        column: col(slug).column,
-      })
-      assertWireEnvelope(slug, encrypted)
-      assignments[slug] = encrypted
-    }
+    // Field encrypts are independent ZeroKMS round-trips — run them
+    // concurrently rather than paying fields × RTT per row.
+    await Promise.all(
+      Object.entries(row.values).map(async ([slug, value]) => {
+        const encrypted = await client.encrypt(toWasmPlaintext(value), {
+          table: tableSchema,
+          column: col(slug).column,
+        })
+        assertWireEnvelope(slug, encrypted)
+        assignments[slug] = encrypted
+      }),
+    )
     return assignments
   }
 
@@ -253,6 +257,12 @@ export function makeWasmAdapter(): IntegrationAdapter {
       }
       case 'in':
       case 'notIn': {
+        // Parity with the production operators (drizzle inArrayOp): an
+        // empty list would otherwise render `WHERE ()` — a syntax error,
+        // not empty-IN semantics.
+        if (op.values.length === 0) {
+          throw new Error(`${op.kind} requires a non-empty list of values`)
+        }
         // One encryptQueryBulk crossing for the whole list — the bulk path
         // gets exercised on every family this way.
         const { column, eqlType } = col(op.column)
@@ -274,8 +284,10 @@ export function makeWasmAdapter(): IntegrationAdapter {
       }
       case 'between':
       case 'notBetween': {
-        const lo = await term(op.column, op.lo, op.kind)
-        const hi = await term(op.column, op.hi, op.kind)
+        const [lo, hi] = await Promise.all([
+          term(op.column, op.lo, op.kind),
+          term(op.column, op.hi, op.kind),
+        ])
         // Parenthesised conjunction, mirroring the Drizzle dialect's
         // load-bearing parentheses (NOT binds tighter than AND).
         const range = `(eql_v3.gte("${op.column}", $1::jsonb::${lo.cast}) AND eql_v3.lte("${op.column}", $2::jsonb::${hi.cast}))`
@@ -372,20 +384,40 @@ export function makeWasmAdapter(): IntegrationAdapter {
     },
 
     async insertBulk(_spec: TableSpec, rows: readonly PlainRow[]) {
-      for (const row of rows) {
-        await insertRow(await encryptRow(row))
-      }
+      // Rows are independent (distinct row_key PKs, no ordering
+      // dependency) — encrypt and insert them concurrently.
+      await Promise.all(
+        rows.map(async (row) => insertRow(await encryptRow(row))),
+      )
     },
 
     async run(_spec: TableSpec, op: QueryOp) {
       return run(op)
     },
 
+    // Discriminate capability rejections from infrastructure failures —
+    // this adapter's run() path is fully live (ZeroKMS + real SQL), so a
+    // bare catch would record a connection reset or a dropped table as a
+    // passing negative test. Same doctrine as the Supabase adapter's
+    // expectRejected. Two shapes are legitimate rejections here:
+    //  - client-side: resolveIndexType / the pre-FFI validators throw when
+    //    the queryType isn't configured on the column;
+    //  - server-side (order ops only): domains without an ordering index
+    //    have no eql_v3.ord_term overload, so Postgres rejects the call.
     async expectRejected(_spec: TableSpec, op: QueryOp) {
       try {
         await run(op)
-      } catch {
-        return
+      } catch (error) {
+        const message = error instanceof Error ? error.message : String(error)
+        const isCapabilityRejection =
+          /is not configured on column|has no indexes configured|\[encryption\]:/.test(
+            message,
+          ) || /ord_term/.test(message)
+        if (isCapabilityRejection) return
+        throw new Error(
+          `Expected ${op.kind} on "${op.column}" to be rejected by a capability error, but got an unrelated failure: ${message}`,
+          { cause: error },
+        )
       }
       throw new Error(
         `Expected ${op.kind} on "${op.column}" to be rejected, but it ran.`,