Skip to content

feat(stack): add WASM-inline subpath + Deno verification#496

Merged
coderdan merged 9 commits into
mainfrom
feat/stack-wasm-inline
Jun 19, 2026
Merged

feat(stack): add WASM-inline subpath + Deno verification#496
coderdan merged 9 commits into
mainfrom
feat/stack-wasm-inline

Conversation

@coderdan

@coderdan coderdan commented May 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • New @cipherstash/stack/wasm-inline subpath so Protect runs in Deno, Bun, Cloudflare Workers, Supabase Edge, browsers — anywhere the native protect-ffi NAPI bindings don't work.
  • Deno smoke test (no --allow-ffi) verifies the WASM path is genuinely WASM, not a silent native fallback.
  • Supabase Edge Function example demonstrates the same path end-to-end.

Dependency bumps

  • @cipherstash/protect-ffi 0.23.0 → 0.24.0 (Node API unchanged — strategy-based newClient only lives on /wasm-inline).
  • @cipherstash/auth catalog → 0.37.0-alpha.8 for the WASM AccessKeyStrategy. Both packages added to pnpm-workspace.yaml's release-age exclude.

Draft status

Blocking on a non-alpha @cipherstash/auth release with /wasm-inline — planned for 0.38.0. Bump the catalog when that ships.

What changed

  • packages/stack/src/wasm-inline.ts exports Encryption() + WasmEncryptionClient (encrypt / decrypt round-trip) wired to WASM protect-ffi + AccessKeyStrategy from auth/wasm-inline, plus raw re-exports (newClientRaw, encryptRaw, decryptRaw, isEncrypted, AccessKeyStrategy) for consumers who want the lower-level API.
  • ESM-only bundle: protect-ffi's wasm-inline is ESM and crashes Node CJS require() (ERR_REQUIRE_ESM). tsup drops dist/wasm-inline.cjs via onSuccess; package.json exports omits the require branch for ./wasm-inline.
  • tsconfig.json adds customConditions: ["node"] so bundler resolution picks Node protect-ffi types for the main entries (the /wasm-inline subpath has unconditional types and is unaffected).
  • e2e/wasm/ — Deno smoke test that round-trips an encryption against ZeroKMS/CTS. No --allow-ffi — if anything ever silently fell back to a native binding, the test would fail loudly.
  • examples/supabase-worker/ — port of a working spike, refactored to import from @cipherstash/stack/wasm-inline instead of going direct to @cipherstash/protect-ffi/wasm-inline + @cipherstash/auth/wasm-inline.
  • New wasm-e2e-tests CI job (Deno 2.x on blacksmith runner) exercises the smoke test on every PR. Exposes only the four CS_* secrets — no debug-only host overrides.

Test plan

  • pnpm exec turbo run build --filter @cipherstash/stack produces dist/wasm-inline.{js,d.ts,d.cts} (no .cjs).
  • 453/453 non-PG stack tests pass (pnpm test --exclude searchable-json-pg --exclude supabase).
  • cjs-require.test.ts passes — confirms the dist/wasm-inline.cjs drop is doing its job.
  • New wasm-e2e-tests CI job runs green on this PR.
  • Local Supabase worker check: cd examples/supabase-worker && supabase functions serve --env-file .env.local cipherstash-roundtrip returns { ok: true }.
  • Negative WASM check (manual): remove the local @cipherstash/protect-ffi-{platform} native binary and rerun the Deno test — must still pass.

Notes

  • packages/protect still pins protect-ffi@0.23.0 — out of scope for this PR.
  • The EQL Docker image is still pinned at eql-2.3.1 in tests.yml. 0.24.0 is WASM-focused (not an EQL format bump), so it should be fine, but if run-tests fails post-bump, the image needs to move too.

Summary by CodeRabbit

  • New Features
    • Added @cipherstash/stack/wasm-inline (WASM-based encryption client) with Encryption and isEncrypted, plus encrypted schema builders, released as @cipherstash/stack 0.18.0.
  • Tests
    • Added Deno E2E smoke tests and a real encrypt→decrypt round-trip for wasm-inline, including required-secret checks to avoid silent skips.
  • Documentation
    • Added a Supabase Edge Function example (README + .env.example) demonstrating encrypt/decrypt usage.
  • Chores
    • Updated packaging/build and workspace dependencies, and aligned platform-native optional bindings behavior.

Add `@cipherstash/stack/wasm-inline` so Protect runs in Deno, Bun,
Cloudflare Workers, Supabase Edge, and browsers — anywhere the native
protect-ffi NAPI bindings don't work.

Why now: protect-ffi 0.24.0 ships a WASM build; auth 0.37.0-alpha.8
ships the matching `AccessKeyStrategy` for the WASM path. The stack
package is the natural place to expose this combination as a
single import target.

What changed:
- Bump @cipherstash/protect-ffi 0.23.0 → 0.24.0 (Node API unchanged
  — strategy-based newClient only lives on /wasm-inline). Bump
  @cipherstash/auth catalog → 0.37.0-alpha.8 for the WASM strategy.
  Both packages added to pnpm-workspace.yaml's release-age exclude.
- New packages/stack/src/wasm-inline.ts exporting `Encryption()` +
  `WasmEncryptionClient` (encrypt / decrypt round-trip), plus raw
  re-exports of `newClientRaw`/`encryptRaw`/`decryptRaw`/
  `AccessKeyStrategy` for consumers who want the spike-style API.
- ESM-only bundle (the WASM-inline runtime is ESM, CJS require()
  would ERR_REQUIRE_ESM); tsup drops dist/wasm-inline.cjs in
  onSuccess, package.json exports omits the require branch.
- tsconfig customConditions: ["node"] so bundler resolution picks
  Node protect-ffi types for the main entries; /wasm-inline subpath
  has unconditional types so it's unaffected.

Verification:
- e2e/wasm/ Deno smoke test runs encrypt → decrypt round-trip
  against ZeroKMS/CTS with no --allow-ffi, proving the WASM path
  is the only path that could have completed.
- New wasm-e2e-tests CI job (Deno 2.x on blacksmith runner)
  exercises this on every PR.
- examples/supabase-worker/ demonstrates the same code path inside
  a Supabase Edge Function for documentation / runbook use.

Draft: depends on @cipherstash/auth shipping a non-alpha release
with /wasm-inline (planned for 0.38.0); bump catalog when that
lands.
@coderabbitai

coderabbitai Bot commented May 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 81813b31-2193-4050-946e-0fbfbf988a31

📥 Commits

Reviewing files that changed from the base of the PR and between ae76993 and c39e895.

📒 Files selected for processing (4)
  • e2e/wasm/roundtrip.test.ts
  • examples/supabase-worker/.env.example
  • examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
  • packages/stack/src/wasm-inline.ts
✅ Files skipped from review due to trivial changes (1)
  • examples/supabase-worker/.env.example
🚧 Files skipped from review as they are similar to previous changes (3)
  • examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
  • e2e/wasm/roundtrip.test.ts
  • packages/stack/src/wasm-inline.ts

📝 Walkthrough

Walkthrough

Adds an ESM-only @cipherstash/stack/wasm-inline entrypoint (implementation, types, and tests), updates packaging/build (tsup, tsconfig, package exports), adds Deno E2E smoke tests and a CI job, provides a Supabase Edge Function example, and aligns workspace auth/native binding dependencies.

Changes

WASM-inline entry point and E2E infrastructure

Layer / File(s) Summary
Package exports and TypeScript configuration
packages/stack/package.json, packages/stack/tsconfig.json
Bumps package to 0.18.0, adds typesVersions for wasm-inline, exposes ./wasm-inline import entry with types and ESM default, adds @cipherstash/auth and updates protect-ffi to 0.24.0, and sets compilerOptions.customConditions: ["node"] to prefer Node/NAPI exports resolution.
WASM-inline core implementation
packages/stack/src/wasm-inline.ts, packages/stack/__tests__/wasm-inline-normalize.test.ts
New ESM entrypoint implementing WasmEncryptionClient (encrypt/decrypt/isEncrypted), Encryption factory with schema validation and auth strategy resolution, re-exports schema helpers and Encrypted type, defines WasmClientConfig/WasmEncryptionConfig types, normalizes cast_as values, and includes unit tests covering schema drift and multi-table cast normalization.
Build configuration and bundling
packages/stack/tsup.config.ts
Parallel tsup configs added with clean: false to avoid races, main config bundled noExternal expanded to include zod and @byteslice/result, and new ESM-only wasm-inline entry config added with matching bundling strategy.
E2E test infrastructure and smoke test
.github/workflows/tests.yml, e2e/package.json, e2e/wasm/deno.json, e2e/wasm/roundtrip.test.ts
GitHub Actions wasm-e2e-tests job installs Node/pnpm/Deno, builds @cipherstash/stack for WASM artifacts, verifies required CS_* secrets are present, and runs Deno deno test round-trip encrypt→decrypt smoke test (FFI disabled) against wasm-inline.
Supabase Edge Function example
examples/supabase-worker/.env.example, examples/supabase-worker/README.md, examples/supabase-worker/package.json, examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
Adds Edge Function demo validating CS_* env credentials, encrypting/decrypting a fixed plaintext, returning appropriate 400/500 JSON errors, with .env.example template, serve script, and README covering prerequisites, local execution, and deployment.
Workspace dependency alignment
pnpm-workspace.yaml, packages/cli/package.json, packages/wizard/package.json
Pins @cipherstash/auth catalog to 0.38.0 and all seven platform-specific variants, declares optional native bindings in CLI/Wizard packages for pnpm host-matched installation, and excludes @cipherstash/auth packages from minimumReleaseAge checks.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • tobyhede

Poem

"I nibble bytes and hop through code,
A wasm carrot tucked in my load;
Deno and Cloudflare sing along—
Round-trips fixed, the tests are strong;
Hooray, the warren's safe and bold!" 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(stack): add WASM-inline subpath + Deno verification' directly and accurately summarizes the main changes: introducing a new WASM-inline subpath export and adding Deno-based E2E verification. It is clear, specific, and reflects the primary developer objective.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/stack-wasm-inline

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

CI on the first push exposed two issues:

1. `Failed to load native binding for linux-x64` in @cipherstash/wizard
   and the e2e/ suite. Cause: auth 0.37.0-alpha migrated the per-platform
   napi binaries from `optionalDependencies` to `peerDependencies`
   (optional), and pnpm does NOT install platform-matched optional peer
   deps. Revert the catalog to 0.36.0 (which still uses
   optionalDependencies and works for NAPI consumers) and pin
   @cipherstash/stack to 0.37.0-alpha.8 directly — stack only uses the
   `/wasm-inline` subpath, never touches the NAPI bindings.

2. `Import "zod" not a dependency` when Deno loaded dist/wasm-inline.js.
   Cause: tsup left zod as an external import, and Deno's
   nodeModulesDir: auto doesn't resolve bare specifiers without an
   explicit map. Bundle zod and @byteslice/result via noExternal — both
   are small and dependency-free.

Also add --no-check to the Deno test task: stack's dist .d.ts strips
the encryptedTable column-intersection brand when loaded via file URL,
so TypeScript can't see `users.email`. The runtime round-trip is the
only thing this smoke test asserts; column-on-table typing is covered
by the package's own vitest suite.
@changeset-bot

changeset-bot Bot commented May 26, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: c39e895

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

coderdan added 3 commits May 27, 2026 00:39
…lient

The Node entry of protect-ffi normalizes SDK-facing cast_as values
(`string`, `number`, …) to EQL-native (`text`, `double`, …) via
`normalizeEncryptConfig.js` before handing them to the Rust core. The
WASM bindings don't ship that normalization layer, so an
`encryptedColumn('email')` (defaults to `cast_as: 'string'`) was
rejected with `unknown variant 'string', expected one of 'big_int',
'boolean', 'date', 'decimal', 'double', 'float', 'real', 'int', 'json',
'jsonb', 'small_int', 'text', 'timestamp'`.

Walk the EncryptConfig in the WASM Encryption() factory and apply
`toEqlCastAs` to each column's cast_as field. Same mapping the Node
side does internally.
0.38.0 promotes the WASM-inline work that landed across the
0.37.0-alpha series to a stable release. Bump the catalog so all
consumers (wizard, cli, stack) track the same major; drop the
direct 0.37.0-alpha.8 pin from packages/stack and the Deno test's
import map.
…lDeps

@cipherstash/auth 0.37.0+ ships per-platform native bindings as
optional peerDependencies. pnpm doesn't auto-install platform-matched
optional peer deps, so on fresh Linux CI no binary lands in
node_modules and the NAPI loader throws:

  Failed to load native binding for linux-x64. Ensure the optional
  dependency "@cipherstash/auth-linux-x64-gnu" is installed.

Declare the six platform sub-packages as `optionalDependencies` on
the two workspace packages that actually `import auth from
'@cipherstash/auth'` (wizard, cli). `optionalDependencies` IS
platform-aware in pnpm — each sub-package's own `os`/`cpu` filter
picks one binary for the host and silently skips the rest.

@cipherstash/stack itself doesn't need this — it only uses the
`/wasm-inline` subpath, which never loads the NAPI shim.
@coderdan coderdan marked this pull request as ready for review May 27, 2026 00:10
@coderdan coderdan requested a review from a team as a code owner May 27, 2026 00:10

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/tests.yml:
- Around line 164-176: The new GitHub Actions job "wasm-e2e-tests" exposes
credentials by omitting a permissions block and using checkout with default
persisted credentials; add a minimal permissions map (e.g., permissions:
contents: read and only other actions-required permissions) at the job level for
least privilege and update the "Checkout Repo" step (actions/checkout@v6) to
include persist-credentials: false so the runner does not leave the token in the
workspace or pass it to subsequent steps.

In `@examples/supabase-worker/.env.example`:
- Around line 4-7: The .env.example lists CS_WORKSPACE_CRN but the code does not
consume it; either remove the CS_WORKSPACE_CRN line from the example to keep
envs minimal and accurate, or if it is required, wire it into the app by reading
process.env.CS_WORKSPACE_CRN in the initialization/config loader (where other
vars like CS_CLIENT_ID/CS_CLIENT_KEY/CS_CLIENT_ACCESS_KEY are consumed) and
ensure the app uses that value; update the .env.example to match whichever
choice you make.

In `@examples/supabase-worker/package.json`:
- Around line 1-11: Add Node and pnpm enforcement to this example package
manifest by adding an "engines" entry with "node": ">=22" and a "packageManager"
field set to "pnpm@9" in the package.json; locate the top-level manifest object
in examples/supabase-worker/package.json (the file containing name, version,
description and scripts) and insert the keys "engines": {"node": ">=22"} and
"packageManager": "pnpm@9" so the example follows the repo policy.

In `@examples/supabase-worker/README.md`:
- Around line 12-50: The README's "Run locally", "Deploy" and "What this proves"
sections are missing required install and test instructions; update the README
to include an explicit "Install" step (e.g., "pnpm install" and any prerequisite
Node/pnpm versions), expand the "Run locally" section to show how to populate
.env.local and run the app locally, add a "Tests" section with the exact
command(s) to run unit/integration tests (e.g., "pnpm test" or "pnpm -w test"
and any setup needed before running tests), and append a short "Native module
externalization" note stating that native modules are externalized in the
Supabase Edge Functions build and no native dependencies are required (or list
any required externals); keep the headings consistent with "Run locally",
"Deploy" and "What this proves" so reviewers can find the changes easily.

In `@examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts`:
- Around line 69-77: Wrap the current response payload in the required success
contract by returning an object with a top-level data property; replace the
current Response.json body that returns ok, plaintext, decrypted,
isEncrypted(...), and ciphertextIdentifier with { data: { ok: decrypted ===
plaintext, plaintext, decrypted, isEncrypted: isEncrypted(encrypted),
ciphertextIdentifier: (encrypted as { i?: unknown }).i } } and keep the same
headers — update the Response.json call in the function returning this response
(where isEncrypted is used) so the example conforms to the { data } contract.
- Around line 79-83: The catch block currently returns raw error fields
including stack and does not follow the { failure } contract; update the catch
to map errors to a stable failure shape (e.g., return Response.json({ failure: {
type: 'internal_error', code: err.code ?? 'unknown_error', message: 'Internal
server error' } }, { status: 500 })) and remove any stack or internal detail;
locate the catch that casts to const err and replace the payload with the stable
{ failure } object (keep err.code normalized if useful but never expose
err.stack or raw messages).

In `@packages/cli/package.json`:
- Around line 53-61: The optionalDependencies block in packages/cli/package.json
pins six `@cipherstash/auth-`<platform> packages to 0.38.0 which drifts from the
main `@cipherstash/auth` catalog; fix by adding those six package names
(`@cipherstash/auth-darwin-arm64`, `@cipherstash/auth-darwin-x64`,
`@cipherstash/auth-linux-arm64-gnu`, `@cipherstash/auth-linux-x64-gnu`,
`@cipherstash/auth-linux-x64-musl`, `@cipherstash/auth-win32-x64-msvc`) to the
workspace catalog (e.g., pnpm-workspace.yaml or a dedicated catalog used by the
repo) so you can replace the hardcoded versions with catalog:repo references in
the optionalDependencies block, or alternatively keep the hardcoded pins but
document and centralize their bumping process if you cannot add them to the
catalog now.

In `@packages/stack/package.json`:
- Around line 182-187: The ./wasm-inline subpath export currently only has an
"import" branch, breaking CommonJS consumers; add a "require" branch under the
"./wasm-inline" export in package.json that points to the CJS bundle (e.g.,
"./dist/wasm-inline.cjs.js" or the existing CJS artifact), and ensure the
corresponding artifact exists in dist/publish output (or regenerate/build it via
the wasm-inline build target); update the export entry for "./wasm-inline" to
include both "import" and "require" keys so
require("`@cipherstash/stack/wasm-inline`") works.

In `@packages/stack/src/wasm-inline.ts`:
- Around line 136-142: The public constructor on WasmEncryptionClient
(constructor(client: unknown)) exposes the raw client and allows invalid
instances; make the constructor non-public (private or protected) and provide an
internal/static factory method or export a top-level Encryption() factory that
accepts the raw client and returns a WasmEncryptionClient, updating any usages
to call the factory instead of new WasmEncryptionClient; apply the same change
for the duplicate constructor occurrence referenced in the diff so construction
is hidden until the public API is stable.
- Around line 144-163: The public wrapper narrows JSON to objects only; update
the `encrypt` and `decrypt` signatures and any related typing so JSON payloads
can be null or arrays too—e.g., broaden the plaintext/return union in
`encrypt(plaintext: ...)` and `decrypt(...): Promise<...>` to include null and
array types (for example add `null` and `unknown[]` or replace with a shared
JsonValue type) and keep calls to `wasmEncrypt`/`wasmDecrypt` and the use of
`EncryptOptions`, `Encrypted`, `wasmEncrypt`, and `wasmDecrypt` unchanged.
- Around line 239-251: resolveStrategy currently silently prefers cfg.strategy
when both cfg.strategy and cfg.accessKey are provided; change it to explicitly
reject conflicting inputs by checking if both are set and throwing a clear Error
stating that accessKey and strategy are mutually exclusive. Specifically, in the
resolveStrategy function add an initial guard like "if (cfg.strategy &&
cfg.accessKey) throw new Error('[encryption]: WASM entry requires either
`config.strategy` or `config.accessKey`, not both')" otherwise keep the existing
behavior of returning cfg.strategy or creating AccessKeyStrategy via
AccessKeyStrategy.create(cfg.region ?? DEFAULT_REGION, cfg.accessKey). Ensure
the thrown message matches the existing error style and references
WasmClientConfig/conflicting inputs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d4d3ba2a-662b-411f-b440-d1855a852fff

📥 Commits

Reviewing files that changed from the base of the PR and between b9c4fe5 and 75e6f6a.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (15)
  • .github/workflows/tests.yml
  • e2e/package.json
  • e2e/wasm/deno.json
  • e2e/wasm/roundtrip.test.ts
  • examples/supabase-worker/.env.example
  • examples/supabase-worker/README.md
  • examples/supabase-worker/package.json
  • examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
  • packages/cli/package.json
  • packages/stack/package.json
  • packages/stack/src/wasm-inline.ts
  • packages/stack/tsconfig.json
  • packages/stack/tsup.config.ts
  • packages/wizard/package.json
  • pnpm-workspace.yaml

Comment thread .github/workflows/tests.yml
Comment thread examples/supabase-worker/.env.example Outdated
Comment thread examples/supabase-worker/package.json
Comment thread examples/supabase-worker/README.md
Comment thread packages/cli/package.json Outdated
Comment thread packages/stack/package.json
Comment thread packages/stack/src/wasm-inline.ts
Comment thread packages/stack/src/wasm-inline.ts
Comment thread packages/stack/src/wasm-inline.ts Outdated
Six fixes from a recall-biased high-effort review:

1. Bump @cipherstash/stack 0.17.0 → 0.18.0 so the Supabase example's
   `npm:@cipherstash/stack@^0.18.0/wasm-inline` import resolves once
   this lands on npm. The minor bump matches the new public subpath.

2. Make `WasmClientConfig.clientId` and `clientKey` required (was `?`),
   and use a discriminated union for `accessKey` vs `strategy` so the
   compiler enforces one-or-the-other. Previously a `{region, accessKey}`
   config typechecked but forwarded `{clientId: undefined, clientKey:
   undefined}` to the WASM client and failed authentication at first
   encrypt.

3. `normalizeCastAs` now throws synchronously if `toEqlCastAs` returns
   `undefined` (i.e. someone added a new SDK-facing cast_as variant
   without extending the switch). The previous behaviour silently
   handed `undefined` to the WASM serde, which surfaced as an opaque
   `unknown variant 'null'` startup crash.

4. Drop the `decryptRaw` / `encryptRaw` / `newClientRaw` /
   `AccessKeyStrategy` re-exports. They were a footgun — a consumer
   reaching `newClientRaw` bypasses `normalizeCastAs` and hits the
   exact same `unknown variant 'string'` failure the high-level
   `Encryption()` was patched to avoid. Anyone who needs raw access
   can import from `@cipherstash/protect-ffi/wasm-inline` or
   `@cipherstash/auth/wasm-inline` directly.

5. Move the six `@cipherstash/auth-<platform>` packages into the
   `pnpm-workspace.yaml` catalog and reference them via `catalog:repo`
   from `wizard` and `cli`'s `optionalDependencies`. Previously each
   sub-package was hard-pinned to the literal `0.38.0`, so a catalog
   bump of `@cipherstash/auth` alone would drift the platform binary
   versions and break the NAPI loader.

6. The `wasm-e2e-tests` job now asserts the four required `CS_*`
   secrets are present BEFORE running `deno task test`. Without this,
   a rotated / cleared CI secret made the test silently `ignore` and
   the job reported green, hiding any real WASM regression.
Comment thread packages/stack/src/wasm-inline.ts Outdated
Comment thread examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts Outdated
Comment thread .github/workflows/tests.yml
Comment thread packages/stack/src/wasm-inline.ts Outdated
Comment thread packages/stack/tsup.config.ts Outdated
Comment thread packages/stack/src/wasm-inline.ts Outdated
Twelve fixes from a high-effort review pass:

CI hardening:
  - Add least-privilege permissions: contents: read to the new
    wasm-e2e-tests job; disable persist-credentials on checkout.
  - Document node-gyp install — it's for the workspace-wide pnpm
    install (node-pty), not the WASM path itself.

Supabase worker example:
  - Drop CS_WORKSPACE_CRN from .env.example — the worker doesn't
    read it (parity gap with the Node entry tracked separately).
  - Sanitise the 500 response: log err.stack to the operator, never
    return it to the caller. Mark the response shape debug-only.
  - Add engines.node + packageManager to follow repo policy.
  - Expand README: install step, native-modules note, and a brief
    "what this verifies" / "automated coverage" pointer to the
    Deno smoke test in e2e/wasm/.

@cipherstash/stack/wasm-inline:
  - Hide WasmEncryptionClient constructor behind an internal
    symbol-gated factory so callers can't wrap arbitrary objects.
  - Broaden plaintext type to include arrays (WasmPlaintext is
    recursive, matching protect-ffi's JsPlaintext).
  - Tighten the strategy type to the actual AccessKeyStrategy class
    from @cipherstash/auth/wasm-inline instead of a structural
    duck-type. Custom token stores still flow via
    AccessKeyStrategy.create({ store }).
  - Mark region @deprecated — a follow-up will replace it with a
    workspaceCrn that the auth strategy derives region from
    (tracked in a separate bug).
  - Export normalizeCastAs for unit-test coverage.

Build pipeline:
  - Split tsup back into two configs: main entries dual ESM+CJS,
    wasm-inline ESM-only. Move dist cleanup to a prebuild npm
    script so both configs run with clean: false and don't race.
    The previous onSuccess deny-list was brittle (Toby/Claude).
  - Side-effect: dist/wasm-inline.d.cts no longer emitted, closing
    the orphan-types-file concern from the same review.

Tests:
  - New __tests__/wasm-inline-normalize.test.ts: exercises every
    CastAs variant, asserts toEqlCastAs is exhaustive, and verifies
    normalizeCastAs throws on drift instead of handing undefined
    to WASM serde. Catches future enum additions before the
    e2e step.

CodeRabbit findings deferred:
  - The "{ data } success contract" and "{ failure } error shape"
    are skipped — no such convention in this repo's other examples.
  - Restoring CJS subpath for wasm-inline skipped — protect-ffi's
    wasm-inline runtime is ESM-only; a require branch would
    re-introduce ERR_REQUIRE_ESM for downstream consumers.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (3)
packages/stack/src/wasm-inline.ts (3)

314-317: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a runtime guard for conflicting strategy + accessKey inputs.

Line 315 relies on TS narrowing only; JS callers can still supply both fields and get silent precedence behavior.

Suggested patch
 function resolveStrategy(cfg: WasmClientConfig): AccessKeyStrategy {
+  if ((cfg as { strategy?: unknown }).strategy && (cfg as { accessKey?: unknown }).accessKey) {
+    throw new Error(
+      '[encryption]: `config.strategy` and `config.accessKey` are mutually exclusive',
+    )
+  }
   if (cfg.strategy) return cfg.strategy
   // Discriminated union guarantees this branch implies `accessKey` is set.
   return AccessKeyStrategy.create(cfg.region, cfg.accessKey as string)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stack/src/wasm-inline.ts` around lines 314 - 317, In
resolveStrategy, add a runtime guard that throws a clear error when both
cfg.strategy and cfg.accessKey are provided (since TS narrowing isn't enforced
at runtime); specifically, at the top of the function check if cfg.strategy !==
undefined && cfg.accessKey !== undefined and throw an Error explaining the
conflicting inputs, otherwise preserve the existing behavior of returning
cfg.strategy if present or calling AccessKeyStrategy.create(cfg.region,
cfg.accessKey as string).

198-201: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

WasmEncryptionClient.__construct is publicly callable and re-opens arbitrary client wrapping.

Line 199 exposes a public static constructor path, so callers can still instantiate WasmEncryptionClient with invalid raw objects.

Suggested patch
 export class WasmEncryptionClient {
@@
-  /** `@internal` */
-  static __construct(client: unknown): WasmEncryptionClient {
-    return new WasmEncryptionClient(INTERNAL_CONSTRUCT, client)
+  /** `@internal` */
+  static __construct(token: symbol, client: unknown): WasmEncryptionClient {
+    if (token !== INTERNAL_CONSTRUCT) {
+      throw new Error(
+        '[encryption]: WasmEncryptionClient cannot be constructed directly — use the Encryption() factory.',
+      )
+    }
+    return new WasmEncryptionClient(INTERNAL_CONSTRUCT, client)
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stack/src/wasm-inline.ts` around lines 198 - 201, The static method
WasmEncryptionClient.__construct is currently publicly callable despite the
`@internal` tag, allowing arbitrary objects to be wrapped; make this factory
inaccessible by either (a) marking __construct as a private static method (so
external callers cannot call WasmEncryptionClient.__construct) or (b) hardening
the constructor guard: change the constructor to require a unique, module-scoped
Symbol and assert the passed token equals INTERNAL_CONSTRUCT (throw if not), and
update __construct to pass that Symbol—refer to
WasmEncryptionClient.__construct, the class constructor, and INTERNAL_CONSTRUCT
when making this change.

108-113: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

WasmPlaintext still omits null, which narrows JSON payload support.

The public wasm-inline wrapper accepts arrays now, but Line 108-113 / Line 218-221 still type-exclude null, which can block valid JSON plaintext/decrypt flows for TS consumers.

Suggested patch
 export type WasmPlaintext =
   | string
   | number
   | boolean
+  | null
   | Record<string, unknown>
   | WasmPlaintext[]
In `@cipherstash/protect-ffi` v0.24.0 (wasm-inline), what is the exact plaintext type accepted by encrypt/decrypt (e.g., JsPlaintext)? Does it include null?

Also applies to: 218-221

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stack/src/wasm-inline.ts` around lines 108 - 113, WasmPlaintext
currently omits null which prevents valid JSON/null values from passing through
encrypt/decrypt; update the WasmPlaintext type declaration (export type
WasmPlaintext) to include null in the union and ensure any other plaintext
aliases (e.g., the corresponding JsPlaintext or other exported plaintext types
referenced later around the second occurrence) are updated the same way so
arrays can contain null elements too; locate the WasmPlaintext type and the
repeated definition near the other mention (lines around the second occurrence)
and add null to the union in both places.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@packages/stack/src/wasm-inline.ts`:
- Around line 314-317: In resolveStrategy, add a runtime guard that throws a
clear error when both cfg.strategy and cfg.accessKey are provided (since TS
narrowing isn't enforced at runtime); specifically, at the top of the function
check if cfg.strategy !== undefined && cfg.accessKey !== undefined and throw an
Error explaining the conflicting inputs, otherwise preserve the existing
behavior of returning cfg.strategy if present or calling
AccessKeyStrategy.create(cfg.region, cfg.accessKey as string).
- Around line 198-201: The static method WasmEncryptionClient.__construct is
currently publicly callable despite the `@internal` tag, allowing arbitrary
objects to be wrapped; make this factory inaccessible by either (a) marking
__construct as a private static method (so external callers cannot call
WasmEncryptionClient.__construct) or (b) hardening the constructor guard: change
the constructor to require a unique, module-scoped Symbol and assert the passed
token equals INTERNAL_CONSTRUCT (throw if not), and update __construct to pass
that Symbol—refer to WasmEncryptionClient.__construct, the class constructor,
and INTERNAL_CONSTRUCT when making this change.
- Around line 108-113: WasmPlaintext currently omits null which prevents valid
JSON/null values from passing through encrypt/decrypt; update the WasmPlaintext
type declaration (export type WasmPlaintext) to include null in the union and
ensure any other plaintext aliases (e.g., the corresponding JsPlaintext or other
exported plaintext types referenced later around the second occurrence) are
updated the same way so arrays can contain null elements too; locate the
WasmPlaintext type and the repeated definition near the other mention (lines
around the second occurrence) and add null to the union in both places.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b9b4008-4b93-4358-8b4d-0139d1a78b55

📥 Commits

Reviewing files that changed from the base of the PR and between 93d92a0 and 4660b00.

📒 Files selected for processing (10)
  • .github/workflows/tests.yml
  • e2e/wasm/roundtrip.test.ts
  • examples/supabase-worker/.env.example
  • examples/supabase-worker/README.md
  • examples/supabase-worker/package.json
  • examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts
  • packages/stack/__tests__/wasm-inline-normalize.test.ts
  • packages/stack/package.json
  • packages/stack/src/wasm-inline.ts
  • packages/stack/tsup.config.ts
✅ Files skipped from review due to trivial changes (2)
  • examples/supabase-worker/package.json
  • examples/supabase-worker/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
  • e2e/wasm/roundtrip.test.ts
  • packages/stack/package.json
  • examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts

coderdan added 2 commits June 19, 2026 23:05
- WasmPlaintext: allow null (JSON null is a valid encrypt/decrypt value)
- resolveStrategy: throw on conflicting accessKey+strategy (and on
  neither) so JS callers that bypass the TS discriminated union fail
  loudly instead of silently preferring one
- drop the public WasmEncryptionClient.__construct factory; the
  symbol-gated constructor is now the single (internal) entry point so
  arbitrary objects can no longer be wrapped
…1.aws

Addresses Toby's review note. Updates the JSDoc example, the config
field doc, the Supabase example .env + handler fallback. The e2e test
keeps ap-southeast-2.aws (its actual CI workspace region); its comment
now notes that's distinct from the documented default.
@coderdan coderdan merged commit 8ab3368 into main Jun 19, 2026
9 checks passed
@coderdan coderdan deleted the feat/stack-wasm-inline branch June 19, 2026 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants