fix(stack): stop the wasm-inline client failing every DynamoDB v3 write (#788 review)

Addresses the three review findings on #788, plus a real defect found while
verifying the second one.

The wasm+v2 guard message is now operation-neutral. `assertClientTableVersionMatch`
runs on all four operations, so a plain-JS caller reaching `encryptModel` with a
v2 table got "cannot read legacy EQL v2 items ... would fail at the first read" —
naming an operation that never ran. The pairing is wrong in both directions
anyway (`Encryption()` on that entry rejects a v2 schema), so the message now
says so instead of describing a read.

Verifying that finding surfaced the larger one: `encryptModel` /
`bulkEncryptModels` chained `.audit()` onto the client's result unconditionally.
The native clients return a thenable operation carrying it; the wasm-inline
client's encrypt returns a bare `Promise<WasmResult>` and has no `.audit()`
anywhere. So EVERY EQL v3 write through this adapter on that entry failed with
`client.encryptModel(...).audit is not a function` — surfaced as a
`DYNAMODB_ENCRYPTION_ERROR`, indistinguishable from a genuine encryption fault.
This PR's own changeset claimed the opposite ("EQL v3 tables are unaffected ...
the wasm-inline client keeps working there"); it was true of decrypt only.

`resolveEncryptResult` mirrors the existing `resolveDecryptResult`: tolerate the
bare promise, drop audit metadata observably rather than crashing, and fail
closed on a malformed result so an unencrypted item can never pass through as a
success. The changeset and the `types.ts` docblock are corrected to match.

Regression tests are credential-free. The chainable half matters most — the
native clients' encrypt audit trail had no coverage outside a live-ZeroKMS
suite, so nothing would have caught breaking it.

skills/stash-dynamodb documented v2 decrypt as unconditionally supported and
never mentioned that wasm-inline refuses v2 tables, on the documented entry for
the runtimes most often paired with DynamoDB. It also claimed audit metadata
forwards "regardless of client shape" — the exact phrase this PR removed from
the source as untrue. Both corrected, plus the wasm-inline row in
skills/stash-encryption, which never said the entry is v3-only.

packages/bench/package.json is back to 2-space indent, keeping only the
substantive `test:unit` addition: 53 changed lines down to 1. The repo's Biome
config is `indentStyle: "space"` and the file was spaces on main, so the churn
was neither required nor conformant — Biome ignores `**/package.json` entirely,
verified by direct invocation.

Author: tobyhede

.changeset/dynamodb-skill-wasm-caveat.md

@@ -0,0 +1,17 @@
+---
+'stash': patch
+---
+
+The `stash-dynamodb` and `stash-encryption` skills documented EQL v2 decrypt as
+unconditionally supported, without noting that `@cipherstash/stack/wasm-inline`
+is EQL v3 only. Since that is the documented entry for Deno, Bun, Cloudflare
+Workers and Supabase Edge Functions — runtimes commonly paired with DynamoDB — a
+reader following the skill hit a runtime refusal with no forewarning. Both skills
+now state that legacy v2 items are readable on the native `@cipherstash/stack`
+entry only.
+
+The `stash-dynamodb` API reference also claimed audit metadata forwards to
+ZeroKMS "regardless of client shape". It does not: the wasm-inline client's
+operations return a plain promise with no `.audit()`, so its audit metadata is
+dropped (logged at debug). The reference now says so, and says the operation
+still succeeds.

.changeset/dynamodb-wasm-v2-read.md

@@ -17,8 +17,20 @@ Workers and Supabase Edge Functions, which satisfies the adapter's client type
 structurally and so was accepted with no cast.
 
 The pairing is now rejected at the call site, with a message naming both the
-combination and the fix. EQL v3 tables are unaffected: they are always passed
-the table, so the wasm-inline client keeps working there.
+combination and the fix. The message is operation-neutral: the guard runs on all
+four operations, so a plain-JS caller reaching the write path with a v2 table
+gets a message that does not claim a read it never attempted.
+
+`encryptModel` / `bulkEncryptModels` now also tolerate a client whose encrypt
+returns a plain promise. They chained `.audit()` onto the result
+unconditionally, which the wasm-inline client does not carry — so **every EQL v3
+write through this adapter on that entry** failed with
+`client.encryptModel(...).audit is not a function`, surfaced as a
+`DYNAMODB_ENCRYPTION_ERROR` and so indistinguishable from a real encryption
+fault. The read path already handled this; the write path now matches, via the
+same fail-closed check that rejects a malformed result rather than passing an
+unencrypted item through as a success. Audit metadata still has nowhere to go on
+that client, so it is dropped and logged at debug — the write itself succeeds.
 
 Three comments in this package claimed audit metadata was forwarded "regardless
 of client shape" and that "every client this package ships carries `.audit()` on

packages/bench/package.json

@@ -1,29 +1,29 @@
 {
-	"name": "@cipherstash/bench",
-	"version": "0.0.5-rc.4",
-	"private": true,
-	"description": "Performance / index-engagement benchmarks for stack integrations (Drizzle, encryptedSupabase, Prisma).",
-	"type": "module",
-	"scripts": {
-		"build": "tsc --noEmit",
-		"test:unit": "vitest run --config vitest.unit.config.ts",
-		"db:setup": "tsx src/cli/setup.ts",
-		"db:reset": "tsx src/cli/reset.ts",
-		"test:local": "vitest run",
-		"bench:local": "vitest bench --run"
-	},
-	"dependencies": {
-		"@cipherstash/stack": "workspace:*",
-		"@cipherstash/stack-drizzle": "workspace:*",
-		"drizzle-orm": "0.45.2",
-		"pg": "^8.22.0"
-	},
-	"devDependencies": {
-		"@cipherstash/test-kit": "workspace:*",
-		"@types/node": "^22.20.1",
-		"@types/pg": "^8.20.0",
-		"tsx": "catalog:repo",
-		"typescript": "catalog:repo",
-		"vitest": "catalog:repo"
-	}
+  "name": "@cipherstash/bench",
+  "version": "0.0.5-rc.4",
+  "private": true,
+  "description": "Performance / index-engagement benchmarks for stack integrations (Drizzle, encryptedSupabase, Prisma).",
+  "type": "module",
+  "scripts": {
+    "build": "tsc --noEmit",
+    "test:unit": "vitest run --config vitest.unit.config.ts",
+    "db:setup": "tsx src/cli/setup.ts",
+    "db:reset": "tsx src/cli/reset.ts",
+    "test:local": "vitest run",
+    "bench:local": "vitest bench --run"
+  },
+  "dependencies": {
+    "@cipherstash/stack": "workspace:*",
+    "@cipherstash/stack-drizzle": "workspace:*",
+    "drizzle-orm": "0.45.2",
+    "pg": "^8.22.0"
+  },
+  "devDependencies": {
+    "@cipherstash/test-kit": "workspace:*",
+    "@types/node": "^22.20.1",
+    "@types/pg": "^8.20.0",
+    "tsx": "catalog:repo",
+    "typescript": "catalog:repo",
+    "vitest": "catalog:repo"
+  }
 }

packages/stack/__tests__/dynamodb/resolve-decrypt.test.ts

@@ -1,15 +1,28 @@
 /**
- * Pure unit tests for the two client-shape helpers behind the DynamoDB adapter's
- * decrypt path. Both shipped clients — nominal `EncryptionClient` and the typed
- * EQL v3 client (whose decrypt returns a `MappedDecryptOperation`) — are
- * chainable and carry `.audit()`; the bare-promise branch remains only for a
- * non-conforming custom client. Every branch was previously reachable only
- * through a live ZeroKMS decrypt; these move that assurance onto the pure CI
- * lane. No credentials, no network.
+ * Pure unit tests for the client-shape helpers behind the DynamoDB adapter's
+ * decrypt AND encrypt paths.
+ *
+ * On decrypt, both native clients — nominal `EncryptionClient` and the typed EQL
+ * v3 client (whose decrypt returns a `MappedDecryptOperation`) — are chainable
+ * and carry `.audit()`; the bare-promise branch is taken by the wasm-inline
+ * client and by a non-conforming custom one.
+ *
+ * The same split exists on encrypt, and only the decrypt half handled it: the
+ * encrypt operations chained `.audit()` unconditionally, so the wasm-inline
+ * client failed every write (#788 review follow-up). `resolveEncryptResult` is
+ * the mirror, and the chainable half of its coverage matters most — the native
+ * clients' encrypt audit trail has no other credential-free test.
+ *
+ * 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 { resolveDecryptResult, throwPreservingCode } from '@/dynamodb/helpers'
+import {
+  resolveDecryptResult,
+  resolveEncryptResult,
+  throwPreservingCode,
+} from '@/dynamodb/helpers'
 import { EncryptionOperation } from '@/encryption/operations/base-operation'
 import { MappedDecryptOperation } from '@/encryption/operations/mapped-decrypt'
 import { type EncryptionError, EncryptionErrorTypes } from '@/errors'
@@ -178,6 +191,97 @@ describe('resolveDecryptResult', () => {
   })
 })
 
+/**
+ * The write-path mirror (#788 review follow-up). The encrypt operations used
+ * to chain `.audit()` unconditionally, so a client returning a bare promise
+ * (the wasm-inline entry) failed every encrypt with
+ * `.audit is not a function`. These pin BOTH directions: the chainable path
+ * must still forward metadata — the native clients' audit trail depends on it,
+ * and its only other coverage is a live-credential suite — and the bare path
+ * must resolve instead of throwing.
+ */
+describe('resolveEncryptResult', () => {
+  it('chains .audit and forwards metadata when present (native clients)', async () => {
+    let seen: unknown
+    let calls = 0
+    const operation = {
+      audit(config: { metadata?: Record<string, unknown> }) {
+        calls += 1
+        seen = config.metadata
+        return Promise.resolve({ data: { encrypted: true } })
+      },
+    }
+
+    const result = await resolveEncryptResult(
+      operation,
+      { metadata: { sub: 'u1' } },
+      'encryptModel',
+    )
+
+    expect(result).toEqual({ data: { encrypted: true } })
+    expect(seen).toEqual({ sub: 'u1' })
+    expect(calls).toBe(1)
+  })
+
+  it('awaits a bare promise instead of throwing (wasm-inline encrypt)', async () => {
+    const result = await resolveEncryptResult(
+      Promise.resolve({ data: { encrypted: true } }),
+      { metadata: { dropped: true } },
+      'encryptModel',
+    )
+
+    expect(result).toEqual({ data: { encrypted: true } })
+  })
+
+  it('propagates a failure result unchanged', async () => {
+    const failure = { failure: { message: 'boom', code: 'X' } }
+
+    await expect(
+      resolveEncryptResult(Promise.resolve(failure), {}, 'bulkEncryptModels'),
+    ).resolves.toEqual(failure)
+  })
+
+  it('returns a failure — not a fake success — for a malformed result', async () => {
+    // Fail closed. A bare value cast straight through would hand the caller an
+    // "encrypted" item that was never encrypted.
+    for (const malformed of [{}, 42, undefined]) {
+      const result = await resolveEncryptResult(
+        Promise.resolve(malformed),
+        {},
+        'encryptModel',
+      )
+
+      expect(result.failure).toBeDefined()
+      expect(result.data).toBeUndefined()
+    }
+  })
+
+  it('names the operation in the dropped-metadata log, and stays silent without metadata', async () => {
+    const spy = vi.spyOn(logger, 'debug').mockImplementation(() => {})
+
+    try {
+      await resolveEncryptResult(
+        Promise.resolve({ data: {} }),
+        { metadata: { m: 1 } },
+        'bulkEncryptModels',
+      )
+      expect(spy).toHaveBeenCalledWith(
+        expect.stringContaining('bulkEncryptModels audit metadata ignored'),
+      )
+
+      spy.mockClear()
+      await resolveEncryptResult(
+        Promise.resolve({ data: {} }),
+        {},
+        'encryptModel',
+      )
+      expect(spy).not.toHaveBeenCalled()
+    } finally {
+      spy.mockRestore()
+    }
+  })
+})
+
 describe('throwPreservingCode', () => {
   it('rethrows as an Error carrying the FFI code', () => {
     try {

packages/stack/__tests__/dynamodb/v2-table-forwarding.test.ts

@@ -133,7 +133,13 @@ describe('a client whose decrypt requires the table', () => {
             "Cannot read properties of undefined (reading 'tableName')",
           )
         }
-        return Promise.resolve({ data: {} })
+        // A bare promise — NOT a thenable operation with `.audit()`. That is
+        // the whole point of this stub: it is the shape `WasmEncryptionClient`
+        // returns from `wasmResult`. The bulk methods resolve to an array,
+        // index-aligned with their input, as the real client does.
+        return Promise.resolve(
+          method.startsWith('bulk') ? { data: [{}] } : { data: {} },
+        )
       }
     const client = {
       requiresTableForDecrypt: true,
@@ -156,7 +162,7 @@ describe('a client whose decrypt requires the table', () => {
     // Synchronous: the guard runs when the operation is built, so the failure
     // lands at the call site rather than as a rejected promise later.
     expect(() => dynamo.decryptModel({ pk: 'a' }, usersV2)).toThrow(
-      /wasm-inline client cannot read legacy EQL v2 items/,
+      /wasm-inline client cannot be paired with the legacy EQL v2 table/,
     )
     // Refused before the client is touched, so the user never sees the
     // TypeError about `tableName`.
@@ -168,10 +174,41 @@ describe('a client whose decrypt requires the table', () => {
     const dynamo = encryptedDynamoDB({ encryptionClient: client as never })
 
     expect(() => dynamo.bulkDecryptModels([{ pk: 'a' }], usersV2)).toThrow(
-      /wasm-inline client cannot read legacy EQL v2 items/,
+      /wasm-inline client cannot be paired with the legacy EQL v2 table/,
     )
   })
 
+  /**
+   * #788 review, minor finding.
+   *
+   * The guard runs on all four operations, not just the two read ones, so a
+   * plain-JS caller reaching the write path with a v2 table hits the SAME
+   * message. It must therefore not be phrased for reads only — "would fail at
+   * the first read" names an operation that never ran.
+   *
+   * Typed callers cannot get here (the write overloads are `AnyV3Table`-only,
+   * pinned by `client-compat.test-d.ts`), so this is about the message a JS
+   * caller or a cast lands on, not about reachable behaviour changing.
+   */
+  it('is refused on the v2 WRITE path, with a message that does not claim a read', () => {
+    const { calls, client } = wasmShapedClient([])
+    const dynamo = encryptedDynamoDB({ encryptionClient: client as never })
+
+    for (const call of [
+      () => dynamo.encryptModel({ pk: 'a' } as never, usersV2 as never),
+      () => dynamo.bulkEncryptModels([{ pk: 'a' }] as never, usersV2 as never),
+    ]) {
+      expect(call).toThrow(
+        /wasm-inline client cannot be paired with the legacy EQL v2 table/,
+      )
+      // The read-path phrasing must not survive on a write.
+      expect(call).not.toThrow(/would fail at the first read/)
+      expect(call).not.toThrow(/cannot read legacy EQL v2 items/)
+    }
+
+    expect(calls).toHaveLength(0)
+  })
+
   // v3 tables ARE forwarded the table, so this client works there — the guard
   // must not turn into a blanket rejection of the wasm entry.
   it('is accepted for an EQL v3 table, which is always given the table', async () => {
@@ -183,4 +220,70 @@ describe('a client whose decrypt requires the table', () => {
     expect(calls).toHaveLength(1)
     expect(calls[0]?.argCount).toBe(2)
   })
+
+  /**
+   * #788 review follow-up: the same "v3 tables are unaffected" promise, on the
+   * WRITE path.
+   *
+   * The encrypt operations chained `.audit()` onto the client's result
+   * unconditionally. The native clients return a thenable operation carrying
+   * it; `WasmEncryptionClient.encryptModel` returns a plain
+   * `Promise<WasmResult>` from `wasmResult` (it has no `.audit()` anywhere),
+   * so every v3 encrypt through this adapter died with
+   * `client.encryptModel(...).audit is not a function` — surfaced as a
+   * `DYNAMODB_ENCRYPTION_ERROR` failure, not a crash, so it read as a genuine
+   * encryption fault. The decrypt path already tolerates the bare promise via
+   * `resolveDecryptResult`; the write path must match.
+   */
+  it('encrypts an EQL v3 table even though its encrypt returns a bare promise', async () => {
+    const { calls, client } = wasmShapedClient(['users_v3'])
+    const dynamo = encryptedDynamoDB({ encryptionClient: client as never })
+
+    const single = await dynamo.encryptModel({ pk: 'a' } as never, usersV3)
+    expect(single.failure).toBeUndefined()
+
+    const bulk = await dynamo.bulkEncryptModels([{ pk: 'a' }] as never, usersV3)
+    expect(bulk.failure).toBeUndefined()
+
+    expect(calls.map((c) => c.method)).toEqual([
+      'encryptModel',
+      'bulkEncryptModels',
+    ])
+  })
+
+  /**
+   * Audit metadata has nowhere to go on this client shape, so it is dropped —
+   * but the encrypt must still succeed rather than failing the whole write.
+   * Mirrors the decrypt path's documented behaviour.
+   */
+  it('drops audit metadata on that client rather than failing the encrypt', async () => {
+    const { client } = wasmShapedClient(['users_v3'])
+    const dynamo = encryptedDynamoDB({ encryptionClient: client as never })
+
+    const result = await dynamo
+      .encryptModel({ pk: 'a' } as never, usersV3)
+      .audit({ metadata: { requestId: 'r-1' } })
+
+    expect(result.failure).toBeUndefined()
+  })
+
+  /**
+   * The tolerance must not swallow a malformed result into a fake success —
+   * the same guard `resolveDecryptResult` applies on read.
+   */
+  it('rejects a bare encrypt result that is not { data } or { failure }', async () => {
+    const client = {
+      requiresTableForDecrypt: true,
+      getEncryptConfig: () => ({ v: 1, tables: { users_v3: {} } }),
+      encryptModel: () => Promise.resolve('not-a-result'),
+      bulkEncryptModels: () => Promise.resolve('not-a-result'),
+      decryptModel: () => Promise.resolve({ data: {} }),
+      bulkDecryptModels: () => Promise.resolve({ data: {} }),
+    }
+    const dynamo = encryptedDynamoDB({ encryptionClient: client as never })
+
+    const result = await dynamo.encryptModel({ pk: 'a' } as never, usersV3)
+
+    expect(result.failure?.message).toMatch(/malformed result/)
+  })
 })