fix(stack): address review — native FFI leak, Result plumbing, batch bugs

Review found 15 issues. This commit takes the source and test ones.

CRITICAL (1/15) — the entry pulled in the NATIVE protect-ffi.
`@/encryption/helpers/error-code` value-imports `ProtectError` for an
`instanceof` narrow, and protect-ffi is not in tsup `noExternal`, so
`dist/wasm-inline.js` carried a bare `@cipherstash/protect-ffi` import —
the NAPI entry, in the one bundle that exists to avoid it. On Workers /
Edge the non-`node` condition resolves it to a module exporting no
`ProtectError`; under Deno it resolves to the NAPI loader.

I had previously dismissed this as pre-existing. That was wrong: I ran
`git stash -u` on a clean tree, so it stashed nothing and I rebuilt my
own branch and compared it against itself. Re-verified properly by
swapping in main's `wasm-inline.ts` and rebuilding: main emits only the
`/wasm-inline` specifier, mine emitted both. Replaced with a structural
`readErrorCode` — which is the only thing that could ever work here
anyway, since the WASM build ships no error class for `instanceof` to
match. GH #744 is filed on my faulty premise and needs correcting.

New `wasm-inline-bundle-isolation.test.ts` asserts the built bundle's
external imports, so this class of regression fails CI. Verified it
catches the exact regression by reintroducing it (2 of 3 assertions
fail), rather than assuming.

3/15 — `failure.code` could never be populated: `withResult` runs
`onException` first, so the mapper only ever saw a fresh Error. `toError`
now carries a structural `code` onto the synthesized Error, and
`bulkDecrypt` puts each item's FFI code in its per-index message (a batch
has no single code, but dropping it lost the only machine-readable part).

9/15 — `toError`'s `String(ex)` fallback could itself throw on a
null-prototype object, escaping the Result contract entirely and
rejecting the call. Guarded with `safeString`.

10/15 — sparse inputs. `items.map(() => null)` SKIPS holes, so a hole
came back `undefined` rather than the documented `null` while lengths
still matched, which the length guard cannot see. Now `Array.from`.

15/15 — the positional-batch scaffolding was hand-rolled three times, so
a fourth batch method could omit `assertBatchLength` with no build
failure. One `runBatch` helper owns compaction, short-circuit, length
assert and scatter; the sparse fix lives there once.

5/15, 6/15, 7/15 — docs and surface: removed a duplicate stale `@returns`,
migrated the module-header and all five `encryptQuery` examples off the
pre-Result contract (they interpolated the envelope straight into SQL),
corrected the pending `.changeset/wasm-encrypt-query.md` sentence that
still said errors throw, and exported `EncryptionError` /
`EncryptionErrorTypes` so an edge consumer can discriminate
`failure.type` from the same import.

`WasmResult<T>` is now declared locally rather than re-exporting
`@byteslice/result`'s. Re-exporting put that specifier in the emitted
`.d.ts`, which Deno cannot resolve — the same reason `WasmPlaintext`
re-declares protect-ffi's `JsPlaintext`.

14/15 — test gaps: falsy-but-live values (`0` / `''` / `false`) per batch
method, the sparse-hole case, `encryptQueryBulk`'s previously untested
length guard, and the two stale "throws" titles.

840 tests pass. Zero new tsc errors, diffed against main's error set.
0 biome errors. Bundle verified free of the native FFI.

Author: coderdan

.changeset/wasm-encrypt-query.md

@@ -14,5 +14,6 @@ free-text match, ORE range, and JSON containment/selector — with the same
 index-type resolution as the native client (explicit `queryType`, or
 inference from the column's configured indexes). Cast the term to the
 column's `eql_v3.query_<domain>` type in SQL to reach the indexed operators.
-Errors throw, consistent with the WASM surface's `encrypt`/`decrypt`; the
-bulk form is position-stable (`null` values pass through as `null`).
+Both return `{ data } | { failure }`, consistent with the WASM surface's
+`encrypt`/`decrypt`; the bulk form is position-stable (`null` values pass
+through as `null`).

e2e/wasm/query-types.test.ts

@@ -72,6 +72,26 @@ const catalog = encryptedTable('wasm_query_matrix', {
 })
 
 /** A v3 SCALAR query term: versioned envelope (`v: 3`), NO ciphertext. */
+/**
+ * Unwrap a `{ data } | { failure }` Result (#741 — every fallible method on
+ * this entry returns one). Asserting on the envelope instead of its payload
+ * silently passes: `term.v` is `undefined` on a Result, so a shape check reads
+ * as "not v3" rather than "you forgot to unwrap".
+ */
+function unwrap<T>(
+  result:
+    | { data: T; failure?: never }
+    | { data?: never; failure: { message: string } },
+  label: string,
+): T {
+  assertEquals(
+    result.failure,
+    undefined,
+    `${label}: ${result.failure?.message}`,
+  )
+  return result.data as T
+}
+
 function assertV3Term(term: unknown, label: string) {
   assertExists(term, `${label}: encryptQuery returned null`)
   const obj = term as Record<string, unknown>
@@ -119,42 +139,54 @@ Deno.test({
 
     // equality → unique index
     assertV3Term(
-      await client.encryptQuery('alice@example.com', {
-        table: catalog,
-        column: catalog.email,
-        queryType: 'equality',
-      }),
+      unwrap(
+        await client.encryptQuery('alice@example.com', {
+          table: catalog,
+          column: catalog.email,
+          queryType: 'equality',
+        }),
+        'equality',
+      ),
       'equality',
     )
 
     // freeTextSearch → match index
     assertV3Term(
-      await client.encryptQuery('needle phrase', {
-        table: catalog,
-        column: catalog.bio,
-        queryType: 'freeTextSearch',
-      }),
+      unwrap(
+        await client.encryptQuery('needle phrase', {
+          table: catalog,
+          column: catalog.bio,
+          queryType: 'freeTextSearch',
+        }),
+        'freeTextSearch',
+      ),
       'freeTextSearch',
     )
 
     // orderAndRange → ore index (numeric)
     assertV3Term(
-      await client.encryptQuery(42, {
-        table: catalog,
-        column: catalog.age,
-        queryType: 'orderAndRange',
-      }),
+      unwrap(
+        await client.encryptQuery(42, {
+          table: catalog,
+          column: catalog.age,
+          queryType: 'orderAndRange',
+        }),
+        'orderAndRange',
+      ),
       'orderAndRange',
     )
 
     // searchableJson, string value → ste_vec_selector (JSONPath). By
     // contract the selector "term" is a BARE selector-hash string — no
     // envelope — bound as the text argument of `->` / `->>`.
-    const selector = await client.encryptQuery('$.theme', {
-      table: catalog,
-      column: catalog.prefs,
-      queryType: 'searchableJson',
-    })
+    const selector = unwrap(
+      await client.encryptQuery('$.theme', {
+        table: catalog,
+        column: catalog.prefs,
+        queryType: 'searchableJson',
+      }),
+      'selector',
+    )
     assertExists(selector, 'selector: encryptQuery returned null')
     assertEquals(
       typeof selector,
@@ -171,30 +203,36 @@ Deno.test({
     // strict {sv: [...]}, no version envelope — per the eql_v3.query_jsonb
     // wire contract)
     assertContainmentNeedle(
-      await client.encryptQuery(
-        { theme: 'dark' },
-        {
-          table: catalog,
-          column: catalog.prefs,
-          queryType: 'searchableJson',
-        },
+      unwrap(
+        await client.encryptQuery(
+          { theme: 'dark' },
+          {
+            table: catalog,
+            column: catalog.prefs,
+            queryType: 'searchableJson',
+          },
+        ),
+        'searchableJson/containment',
       ),
       'searchableJson/containment',
     )
 
     // Omitted queryType → inference from the column's indexes (TextEq has
     // exactly one: unique), mirroring the native client.
     assertV3Term(
-      await client.encryptQuery('bob@example.com', {
-        table: catalog,
-        column: catalog.email,
-      }),
+      unwrap(
+        await client.encryptQuery('bob@example.com', {
+          table: catalog,
+          column: catalog.email,
+        }),
+        'inference',
+      ),
       'inference',
     )
 
     // Bulk: one round trip across mixed query types, position-stable with
     // nulls passing through.
-    const bulk = await client.encryptQueryBulk([
+    const bulkResult = await client.encryptQueryBulk([
       {
         value: 'alice@example.com',
         table: catalog,
@@ -225,6 +263,7 @@ Deno.test({
         queryType: 'searchableJson',
       },
     ])
+    const bulk = unwrap(bulkResult, 'bulk')
     assertEquals(bulk.length, 5)
     assertV3Term(bulk[0], 'bulk/equality')
     assertEquals(bulk[1], null, 'bulk: null value must yield null')

e2e/wasm/roundtrip.test.ts

@@ -102,10 +102,19 @@ Deno.test({
 
     const plaintext = `wasm-v3-smoke-${crypto.randomUUID()}@example.com`
 
-    const encrypted = await client.encrypt(plaintext, {
+    // Every fallible method returns `{ data } | { failure }` (#741). Unwrap
+    // at each boundary — passing an envelope on would fail in ways that look
+    // like an encryption bug rather than a plumbing one.
+    const encryptResult = await client.encrypt(plaintext, {
       column: users.email,
       table: users,
     })
+    assertEquals(
+      encryptResult.failure,
+      undefined,
+      `encrypt() failed: ${encryptResult.failure?.message}`,
+    )
+    const encrypted = encryptResult.data
 
     assertEquals(
       isEncrypted(encrypted),
@@ -138,17 +147,28 @@ Deno.test({
     }
     assertEquals(String(wire.v), '3', 'wire envelope is not v3')
 
-    const decrypted = await client.decrypt(encrypted)
-    assertEquals(decrypted, plaintext, 'round-trip plaintext mismatch')
+    const decryptResult = await client.decrypt(encrypted)
+    assertEquals(
+      decryptResult.failure,
+      undefined,
+      `decrypt() failed: ${decryptResult.failure?.message}`,
+    )
+    assertEquals(decryptResult.data, plaintext, 'round-trip plaintext mismatch')
 
     // 5. (#662) Searchable encryption is reachable on the edge: mint a v3
     //    QUERY TERM for the column's free-text index. Terms are
     //    ciphertext-free needles — assert the wire shape, not decryption.
-    const term = (await client.encryptQuery(plaintext, {
+    const termResult = await client.encryptQuery(plaintext, {
       column: users.email,
       table: users,
       queryType: 'freeTextSearch',
-    })) as Record<string, unknown> | null
+    })
+    assertEquals(
+      termResult.failure,
+      undefined,
+      `encryptQuery() failed: ${termResult.failure?.message}`,
+    )
+    const term = termResult.data as Record<string, unknown> | null
     assertExists(term, 'encryptQuery() returned null for live plaintext')
     assertEquals(term.v, 3, 'query term is not EQL v3')
     assertEquals(
@@ -158,12 +178,48 @@ Deno.test({
     )
 
     // Bulk form is position-stable, nulls pass through.
-    const bulk = await client.encryptQueryBulk([
+    const bulkResult = await client.encryptQueryBulk([
       { value: plaintext, column: users.email, table: users },
       { value: null, column: users.email, table: users },
     ])
+    assertEquals(
+      bulkResult.failure,
+      undefined,
+      `encryptQueryBulk() failed: ${bulkResult.failure?.message}`,
+    )
+    const bulk = bulkResult.data
     assertEquals(bulk.length, 2)
     assertExists(bulk[0])
     assertEquals(bulk[1], null)
+
+    // 6. (#741) The value-level bulk ops — the whole reason a list read on the
+    //    edge is one ZeroKMS round trip instead of N. Nothing else in the repo
+    //    exercises these against real ZeroKMS.
+    const second = `wasm-v3-bulk-${crypto.randomUUID()}@example.com`
+    const bulkEncrypted = await client.bulkEncrypt([
+      { plaintext, column: users.email, table: users },
+      { plaintext: null, column: users.email, table: users },
+      { plaintext: second, column: users.email, table: users },
+    ])
+    assertEquals(
+      bulkEncrypted.failure,
+      undefined,
+      `bulkEncrypt() failed: ${bulkEncrypted.failure?.message}`,
+    )
+    const payloads = bulkEncrypted.data
+    assertEquals(payloads.length, 3, 'bulkEncrypt is not index-aligned')
+    assertEquals(payloads[1], null, 'null plaintext did not yield null')
+    assertExists(payloads[0])
+    assertExists(payloads[2])
+    assertEquals(isEncrypted(payloads[0]), true, 'bulkEncrypt[0] not a payload')
+
+    const bulkDecrypted = await client.bulkDecrypt(payloads)
+    assertEquals(
+      bulkDecrypted.failure,
+      undefined,
+      `bulkDecrypt() failed: ${bulkDecrypted.failure?.message}`,
+    )
+    // Round-trips at the ORIGINAL indices, with the null hole preserved.
+    assertEquals(bulkDecrypted.data, [plaintext, null, second])
   },
 })

packages/stack/__tests__/helpers/expect-result.ts

@@ -1,28 +1,25 @@
-import type { Result } from '@byteslice/result'
-import type { EncryptionError } from '../../src/errors'
+import type { WasmResult } from '../../src/wasm-inline'
 
 /**
- * Narrow a `Result` to its success arm in tests.
+ * Narrow a `WasmResult` to its success arm in tests.
  *
- * `Result<S, F>` is `Success<S> | Failure<F>`, and `Failure` has NO `data`
- * property — so `result.data` doesn't type-check until the union is narrowed.
- * Every assertion on a successful payload would otherwise need its own
- * `if (r.failure) throw` preamble.
+ * `WasmResult<S>` is a `{ data } | { failure }` union, so `result.data` does
+ * not type-check until it is narrowed. Every assertion on a successful payload
+ * would otherwise need its own `if (r.failure) throw` preamble.
  *
- * `F` is pinned to `EncryptionError` rather than left generic: the library
- * constrains it to `FailureCase | Error`, and a bare type parameter neither
- * satisfies that constraint nor lets TypeScript discriminate the union.
- * Every caller here is an encryption operation anyway.
+ * Typed against `WasmResult` specifically, NOT `@byteslice/result`'s `Result`:
+ * inference across the two unions collapses `S` to `S | undefined`, so every
+ * downstream property access reads as possibly-undefined.
  *
  * Throws (rather than using `expect`) so it stays usable outside an assertion
  * context and reports the failure's own message, which is far more useful than
  * a bare "expected data to be defined".
  */
-export function expectData<S>(result: Result<S, EncryptionError>): S {
+export function expectData<S>(result: WasmResult<S>): S {
   if (result.failure) {
     throw new Error(
       `expected { data } but got { failure } (${result.failure.type}): ${result.failure.message}`,
     )
   }
-  return result.data
+  return result.data as S
 }