From 518abfd20b78973a987f0fd101e4d4632914ece6 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 28 Jul 2026 11:26:58 +1000 Subject: [PATCH 1/8] docs: stop advertising the WASM entry as browser-capable (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit three places on this branch saying the opposite — including the example a reader is most likely to copy. - `examples/supabase-worker/README.md` and its edge function both listed "modern browsers" among the runtimes the entry works in. The example reads `CS_CLIENT_KEY` from the environment, so it demonstrated the exact thing it claimed was browser-safe. - `packages/stack/tsup.config.ts` listed browsers among "the only runtimes that need wasm-inline", in the same file #810 cites as evidence there is no `browser` export condition. - `skills/stash-supabase` gave `pg` introspection as the reason the factory cannot run in a browser. True but incomplete, and misleading next to #805 ("add the `browser` export condition"): removing `pg` unblocks Workers, not browsers, because `clientKey` is required on every auth path regardless. Also corrects two claims in #810's own prose. `clientKey` is loaded before the core ever CALLS the strategy — it reads `opts.strategy` earlier than that — and the contract test displaces mocks in eight suites plus a stub in one, not "the stub every other wasm suite uses". (cherry picked from commit a438b0a79b98be849d5f1468f5d455f9b9e1e689) --- ...wasm-inline-client-key-not-browser-safe.md | 25 +++++++++++++++++++ examples/supabase-worker/README.md | 4 ++- .../functions/cipherstash-roundtrip/index.ts | 5 ++-- packages/stack/src/wasm-inline.ts | 21 +++++++++++++++- packages/stack/tsup.config.ts | 7 +++--- skills/stash-supabase/SKILL.md | 6 +++++ 6 files changed, 61 insertions(+), 7 deletions(-) create mode 100644 .changeset/wasm-inline-client-key-not-browser-safe.md diff --git a/.changeset/wasm-inline-client-key-not-browser-safe.md b/.changeset/wasm-inline-client-key-not-browser-safe.md new file mode 100644 index 000000000..b5faebbce --- /dev/null +++ b/.changeset/wasm-inline-client-key-not-browser-safe.md @@ -0,0 +1,25 @@ +--- +'@cipherstash/stack': patch +'stash': patch +--- + +Document that `@cipherstash/stack/wasm-inline` is server-side only, and pin the +reason against the core. + +`WasmClientConfig` requires `clientId` and `clientKey` on every auth arm, +including the `authStrategy` (OIDC federation) arm. That read like an +over-declaration the SDK could relax — if federation alone sufficed, a browser +could hold a client without a workspace secret. It cannot. The core requires +both fields regardless of strategy, and loads `clientKey` as encryption key +material *before* it ever calls the auth strategy. Since `clientKey` is a +workspace secret, no configuration of this entry belongs in a browser bundle — +which is why this entry has no `browser` export condition, and will not get one +until the core changes. + +No behaviour change. The types and runtime are unchanged; what changes is that +the constraint is now stated where callers meet it (`WasmClientConfig`, the +auth-strategy re-export, the `stash-encryption` entry-point table, the +`stash-edge` and `stash-supabase` skills, and the `supabase-worker` example, +which had all described this entry as browser-capable) and enforced by contract +tests that run against the real WASM core instead of the mocks and stub the rest +of the wasm suite uses. diff --git a/examples/supabase-worker/README.md b/examples/supabase-worker/README.md index 62c5b99f6..c19a040f2 100644 --- a/examples/supabase-worker/README.md +++ b/examples/supabase-worker/README.md @@ -2,7 +2,9 @@ A minimal demo of using [`@cipherstash/stack`](https://www.npmjs.com/package/@cipherstash/stack) inside a Supabase Edge Function. The function encrypts a hardcoded plaintext value with CipherStash Protect, decrypts it back, and returns the round-trip result as JSON. -The function imports from the `@cipherstash/stack/wasm-inline` subpath — the WASM build of Protect, with the WASM module inlined into the JS bundle. No native bindings are loaded, so it works in Supabase Edge (Deno) and any other V8-only runtime (Cloudflare Workers, Bun, modern browsers). +The function imports from the `@cipherstash/stack/wasm-inline` subpath — the WASM build of Protect, with the WASM module inlined into the JS bundle. No native bindings are loaded, so it works in Supabase Edge (Deno) and any other V8-only runtime (Cloudflare Workers, Bun). + +**Server-side only.** That list is deliberately server-side: the entry requires `CS_CLIENT_KEY`, a workspace secret, on every auth path — including when you supply a per-user `authStrategy` — so it must not be bundled into a browser ([#804](https://github.com/cipherstash/stack/issues/804)). ## Prerequisites diff --git a/examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts b/examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts index db3a49f4d..157472998 100644 --- a/examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts +++ b/examples/supabase-worker/supabase/functions/cipherstash-roundtrip/index.ts @@ -4,8 +4,9 @@ * and decrypt it back, all via WASM (no native bindings). * * Imports `@cipherstash/stack/wasm-inline` — the WASM-inline subpath - * works in any V8-only runtime (Supabase Edge, Cloudflare Workers, Bun, - * Deno, modern browsers). + * works in any V8-only SERVER runtime (Supabase Edge, Cloudflare Workers, + * Bun, Deno). Not the browser: the entry requires `CS_CLIENT_KEY`, a + * workspace secret, on every auth path (#804). * * Usage: * cp ../../.env.example ../../.env.local # fill in your CS_* values diff --git a/packages/stack/src/wasm-inline.ts b/packages/stack/src/wasm-inline.ts index 99a225933..87896b1a8 100644 --- a/packages/stack/src/wasm-inline.ts +++ b/packages/stack/src/wasm-inline.ts @@ -217,11 +217,30 @@ export type WasmPlaintext = * * Mirrors the Node `ClientConfig`: `authStrategy` is the documented field, * `strategy` is retained as a deprecated alias (see below). + * + * NOT BROWSER-SAFE (#804). `clientId` and `clientKey` sit on the base of the + * intersection below, so they are required on EVERY arm — including the + * `authStrategy` arm, which exists precisely so an end user's OIDC JWT does + * the authorising. That is not an over-declaration this entry could relax: + * the core requires both regardless of strategy, and loads `clientKey` as + * encryption key material before it ever calls `strategy.getToken()`. Since + * `clientKey` is a workspace secret, no configuration of this entry can be + * shipped to a browser bundle. Hence no `browser` export condition. + * `__tests__/wasm-inline-core-credential-contract.test.ts` pins that contract + * against the real core — if it starts failing, the core changed and browser + * support is worth revisiting. */ export type WasmClientConfig = { /** Workspace client identifier — required by the WASM client. */ clientId: string - /** Workspace client key — required by the WASM client. */ + /** + * Workspace client key — required by the WASM client on every auth path, + * including `authStrategy`. This is **secret key material**, not an + * identifier: the core decodes it into the keyed permutations the + * searchable-index schemes run on, and does so before authenticating, + * independently of how requests are authorised. Keep it server-side (see + * the type-level note above). + */ clientKey: string /** diff --git a/packages/stack/tsup.config.ts b/packages/stack/tsup.config.ts index c00987893..90382b3f3 100644 --- a/packages/stack/tsup.config.ts +++ b/packages/stack/tsup.config.ts @@ -36,9 +36,10 @@ export default defineConfig([ // an ESM module that dynamically imports the inlined base64 WASM blob; // it cannot be loaded via Node CJS `require()` (ERR_REQUIRE_ESM), and // the only runtimes that need wasm-inline (Deno, Bun, Workers, - // Supabase Edge, browsers) are ESM-first anyway. `package.json`'s - // `./wasm-inline` export deliberately omits the `require` branch to - // match. + // Supabase Edge — all server-side; the entry requires a workspace + // secret, so it is not browser-safe, see #804) are ESM-first anyway. + // `package.json`'s `./wasm-inline` export deliberately omits the + // `require` branch to match. { entry: { 'wasm-inline': 'src/wasm-inline.ts' }, format: ['esm'], diff --git a/skills/stash-supabase/SKILL.md b/skills/stash-supabase/SKILL.md index 442fba453..efff50643 100644 --- a/skills/stash-supabase/SKILL.md +++ b/skills/stash-supabase/SKILL.md @@ -268,6 +268,12 @@ internally — there is no client-side schema to hand-maintain. Introspection needs a direct Postgres connection (`options.databaseUrl`, defaulting to `DATABASE_URL`), so the factory cannot run in a Worker or the browser. +Introspection is not the only thing keeping this out of a browser. On the +WASM entry, `config.clientKey` is a workspace secret and is required on +*every* auth path — supplying a per-user `config.authStrategy` does not +remove it ([#804](https://github.com/cipherstash/stack/issues/804)). Removing +the `pg` dependency would unblock Workers, not browsers. + Options: `{ schemas?, databaseUrl?, config? }` — `config` is the encryption client config (e.g. `config.authStrategy`, see Authentication below). From c4e13d11267e8e033243517899be208e3bdc5350 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 28 Jul 2026 11:26:46 +1000 Subject: [PATCH 2/8] test(stack): add a positive control and drift guard to the clientKey contract (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #810 found the contract test proved less than it claimed, and that two of its stated premises were wrong. Both are fixed by testing them. `expect(calls.getToken).toBe(0)` had no positive control. Nothing in the file ever reached a state where `getToken` was called, so a count of zero was equally consistent with "key loading precedes auth" (the claim) and with "the core never calls `getToken` during `newClient` at all" — a counter that is never incremented reads as zero either way. It does call it. Structurally complete key material — derived from the core's own error messages, not from any credential: `{ p1, p2_from, p2_to, p3 }`, each a `Permutation` — clears the key provider, after which the core calls `getToken` exactly once and fails on the deliberately malformed token the stand-in returns. So auth IS reached during construction, only after the key is loaded, and the counter is live. Still offline: the token never parses, so nothing leaves the process. The claim that a rename of `opts.strategy` would silently hollow out the file turned out to be false, and is now pinned rather than assumed: the core reads the strategy by name BEFORE deserialising the credentials, so omitting it yields `opts.strategy is required`, which matches none of the other tests' regexes. They would fail loudly, not pass vacuously. Also pins the consequence the docs assert but nothing enforced: no `browser` export condition on any subpath. Adding one to quiet a bundler would ship a workspace secret to the browser and leave `src/wasm-inline.ts` silently wrong. Three corrections to comments that overstated the evidence: - "the strategy was never consulted" → never INVOKED. The core does read `opts.strategy` and typecheck its `getToken` ahead of serde; what never happens is the call. - "every other wasm test mocks `newClient`" → every wasm test that constructs a client. `wasm-inline-normalize.test.ts` relies on the alias stub, and `wasm-inline-bundle-isolation.test.ts` never loads protect-ffi at all. - "a stub that throws" → a stub whose `newClient` throws. Its `isEncrypted` returns `false`. The `e2e/wasm/roundtrip.test.ts` pointer now notes that suite covers the `accessKey` arm, so the federation arm reasoned about here has no live coverage anywhere. Fixes the stub's own docblock while adjacent: it claimed protect-ffi exports no `/wasm-inline` subpath, which 0.30.0 does, and which the contract test resolves directly. (cherry picked from commit 2f992d6a43b8a3ad0db6e16d562d83d7fb4a82da) --- .../helpers/stub-protect-ffi-wasm-inline.ts | 16 +- ...sm-inline-core-credential-contract.test.ts | 276 ++++++++++++++++++ 2 files changed, 286 insertions(+), 6 deletions(-) create mode 100644 packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts diff --git a/packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts b/packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts index 8544dc43e..4f8d237a0 100644 --- a/packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts +++ b/packages/stack/__tests__/helpers/stub-protect-ffi-wasm-inline.ts @@ -1,12 +1,16 @@ /** * Test stub for `@cipherstash/protect-ffi/wasm-inline`. * - * The installed `@cipherstash/protect-ffi` only exports `.` — the `/wasm-inline` - * subpath does not exist, so Vitest cannot resolve `src/wasm-inline` (which - * imports it). These no-op stubs let the unit tests that only exercise pure - * helpers (`getColumnName`, `normalizeCastAs`) load the module. Aliased in via - * `vitest.config.ts`. Any test that actually needs WASM behaviour must mock it - * explicitly (see `wasm-inline-column-name.test.ts`). + * These no-op stubs let the unit tests that only exercise pure helpers + * (`getColumnName`, `normalizeCastAs`) load `src/wasm-inline` without paying + * for the real 4MB inlined-WASM module. Aliased in via `vitest.shared.ts` + * (`stackSourceAlias`). Any test that actually needs WASM behaviour must mock + * it explicitly (see `wasm-inline-column-name.test.ts`). + * + * The alias is a convenience, not a necessity: `@cipherstash/protect-ffi` + * does export `./wasm-inline` as of 0.30.0, and + * `wasm-inline-core-credential-contract.test.ts` deliberately bypasses this + * stub to assert against the real core. */ export const decrypt = (): never => { throw new Error( diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts new file mode 100644 index 000000000..a68457fa7 --- /dev/null +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -0,0 +1,276 @@ +/** + * Contract test: the WASM core requires `clientId` AND `clientKey` on EVERY + * auth path — including OIDC federation, the arm that exists so a caller never + * handles a workspace secret. + * + * Why this file exists (#804). `WasmClientConfig` puts `clientId` / `clientKey` + * on the base of its intersection, so they are required even when + * `config.authStrategy` is an `OidcFederationStrategy`. That looked like it + * might be a leftover from the access-key path that the type over-declares — in + * which case a browser could construct a client from a federated JWT alone. It + * is not. The requirement is real, it comes from the Rust core, and `clientKey` + * is a workspace secret, so `@cipherstash/stack/wasm-inline` is not + * browser-safe. That is why there is no `browser` export condition and no + * browser smoke test. + * + * Nothing else in the suite could have caught this: every wasm test that + * constructs a client mocks `newClient`, and `vitest.shared.ts` aliases the + * whole `@cipherstash/protect-ffi/wasm-inline` specifier to a stub whose + * `newClient` throws. So this file resolves the REAL module through Node — + * which the Vite alias does not intercept — and asserts against the actual + * core. + * + * IF THIS TEST FAILS, THAT IS GOOD NEWS. It means the core relaxed the + * requirement and browser support should be re-examined: the `browser` export + * condition (#805), a live browser smoke test, and browser guidance in + * `skills/stash-supabase/SKILL.md` are all blocked on this and nothing else. + * Re-open #804 rather than deleting the assertions. + * + * Runs offline. Every failure asserted here happens during argument + * deserialisation or key loading, before any ZeroKMS / CTS network call, so no + * `CS_*` credentials are needed. The credentialed round-trip lives in + * `e2e/wasm/roundtrip.test.ts` (Deno) — note that it exercises the + * `accessKey` arm, so the federation arm reasoned about here has no live + * coverage anywhere. + */ + +import { readFileSync } from 'node:fs' +import { createRequire } from 'node:module' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' +import { describe, expect, it } from 'vitest' + +// Node's resolver, not Vite's — this is what dodges the stub alias. The +// specifier is then imported as an absolute `file://` URL, which matches no +// alias key, so the real inlined-WASM build is what gets loaded. +const nodeRequire = createRequire(import.meta.url) +const realWasmEntry = pathToFileURL( + nodeRequire.resolve('@cipherstash/protect-ffi/wasm-inline'), +).href + +const { newClient } = (await import(realWasmEntry)) as { + newClient: (opts: unknown) => Promise +} + +// The smallest config the core accepts. `v: 1` is the encrypt-config envelope +// version, unrelated to EQL v2/v3 — `eqlVersion` below selects the wire format. +const encryptConfig = { + v: 1, + tables: { users: { email: { cast_as: 'text', indexes: {} } } }, +} + +const CLIENT_ID = '00000000-0000-4000-8000-000000000000' + +// Valid hex, but NOT a well-formed client key — it decodes to bytes that are +// not a serialised key. Enough to clear hex decoding and reach the key +// provider, which is all the tests below need; none of them requires real key +// material, and none should be read as exercising one. +const HEX_BUT_NOT_KEY_MATERIAL = 'a'.repeat(64) + +// A real `CS_CLIENT_KEY` is hex of a CBOR-serialised key struct (see +// `stash env`, which hex-encodes what ZeroKMS returns). This is the smallest +// input that reaches *into* that struct: CBOR for `{ "p1": h'' }`, which gets +// past the outer map and fails on `p1`'s type. What the core says it wanted +// there is the point of the third test. +const CBOR_KEY_WITH_BAD_P1 = 'a1627031' + '40' + +// Synthetic key material that is STRUCTURALLY complete — enough to clear the +// key provider entirely, which is what makes the positive control below +// possible. Derived from the core's own error messages, not from any real +// credential: the struct is `{ p1, p2_from, p2_to, p3 }`, each a +// `Permutation { permutation: [...] }`. Here every permutation is empty, so +// this is well-formed but cryptographically worthless — it exists only to get +// past key loading and observe what happens next. +const WELL_FORMED_KEY_MATERIAL = + 'a4627031a16b7065726d75746174696f6e80' + // p1: { permutation: [] } + '6770325f66726f6da16b7065726d75746174696f6e80' + // p2_from: { permutation: [] } + '6570325f746fa16b7065726d75746174696f6e80' + // p2_to: { permutation: [] } + '627033a16b7065726d75746174696f6e80' // p3: { permutation: [] } + +/** + * An `OidcFederationStrategy`-shaped stand-in. The core duck-types the + * strategy — it checks `getToken` is a function and calls it — so a plain + * object is a faithful stand-in, and it records whether the call happened, + * which is the point of these tests. + * + * The token is deliberately not a well-formed JWT. That guarantees the one + * test that does reach auth stops at local token parsing, so this file stays + * offline no matter how far into the pipeline a future core gets. + */ +function federationStrategy() { + const calls = { getToken: 0 } + return { + calls, + strategy: { + getToken: async () => { + calls.getToken++ + return { data: { token: 'not-a-real-token' } } + }, + }, + } +} + +describe('protect-ffi WASM core: credential contract under OIDC federation (#804)', () => { + it('requires `clientKey` even when an auth strategy is supplied', async () => { + const { calls, strategy } = federationStrategy() + + await expect( + newClient({ + clientId: CLIENT_ID, + strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/missing field `clientKey`/) + + // Rejected during deserialisation of the options struct — the strategy was + // never INVOKED, so federation cannot substitute for the key. (The core + // does look at `opts.strategy` before this point, to check it is present + // and carries a `getToken`; what never happens is the call.) + expect(calls.getToken).toBe(0) + }) + + it('requires `clientId` even when an auth strategy is supplied', async () => { + const { calls, strategy } = federationStrategy() + + await expect( + newClient({ + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/missing field `clientId`/) + + expect(calls.getToken).toBe(0) + }) + + it('decodes `clientKey` in two stages — hex, then a key provider', async () => { + // Two distinct error classes prove two distinct stages. A field that were + // merely format-checked would have only the first. + const hexStage = federationStrategy() + await expect( + newClient({ + clientId: CLIENT_ID, + clientKey: 'not-hex', + strategy: hexStage.strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/invalid clientKey: invalid hex/) + + const providerStage = federationStrategy() + await expect( + newClient({ + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + strategy: providerStage.strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/Key provider error: Invalid client key/) + + // Neither stage invoked the strategy: key loading strictly precedes auth. + // The last test in this file supplies the other half of that claim, by + // getting past key loading and watching `getToken` fire. + expect(hexStage.calls.getToken).toBe(0) + expect(providerStage.calls.getToken).toBe(0) + }) + + it('decodes `clientKey` into cryptographic key material, not an identifier', async () => { + const { calls, strategy } = federationStrategy() + + // The load-bearing assertion, and the one that separates "the core parses + // this field" from "the core uses this field as a key". Reaching into the + // serialised struct, the core reports that `p1` must be a `Permutation` — + // a keyed permutation, i.e. cryptographic material for the searchable-index + // schemes. Nothing that merely validated a credential's format would decode + // a permutation out of it. That is what makes `clientKey` a secret, and + // therefore what blocks this entry from a browser bundle. + // + // This asserts on the core's internal key layout deliberately. If protect-ffi + // changes it this test fails, and that is the intended prompt to re-read + // #804 rather than to loosen the assertion. + await expect( + newClient({ + clientId: CLIENT_ID, + clientKey: CBOR_KEY_WITH_BAD_P1, + strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/expected struct Permutation/) + + expect(calls.getToken).toBe(0) + }) + + it('reads the strategy off `opts.strategy`, before it deserialises the credentials', async () => { + // Fixes the meaning of every test above. They all assert "even when an + // auth strategy is supplied" — which is only worth anything if the core + // reads the field they supply it on. It does: omitting `strategy` + // entirely beats `missing field \`clientKey\`` to the punch, so the + // strategy is seen before the credentials are even deserialised. + // + // This is also the drift guard. If protect-ffi renamed the option, the + // tests above would be handing the core nothing and quietly testing the + // no-strategy path instead. They would fail rather than pass silently + // (`opts.strategy is required` matches none of their regexes), and this + // test names the reason. + await expect( + newClient({ + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/opts\.strategy is required/) + }) + + it('invokes `getToken` only after key loading succeeds', async () => { + const { calls, strategy } = federationStrategy() + + // The positive control for every `toBe(0)` above. Without it those + // assertions could not tell "auth comes after key loading" apart from + // "auth never happens during `newClient` at all" — a counter that is + // never incremented reads as 0 either way. + // + // Structurally complete key material clears the key provider, and the + // core then calls `getToken` exactly once, failing on the deliberately + // malformed token this stand-in returns. So: auth IS reached during + // construction, it is reached only after the key is loaded, and the + // counter these tests rely on is live. + await expect( + newClient({ + clientId: CLIENT_ID, + clientKey: WELL_FORMED_KEY_MATERIAL, + strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/Invalid token: JWT must have three segments/) + + expect(calls.getToken).toBe(1) + }) +}) + +describe('@cipherstash/stack declares no browser build (#804)', () => { + it('has no `browser` export condition on any subpath', () => { + // The consequence of everything above, and the one part of it a reader + // can act on by accident. `src/wasm-inline.ts` tells callers there is no + // `browser` condition and explains why; nothing enforced that, so adding + // one to fix a bundler complaint would ship a workspace secret to the + // browser and leave the doc silently wrong. + // + // Same rule as the rest of this file: if the core stops requiring + // `clientKey`, come back through #804 — don't just delete this. + const packageJson = JSON.parse( + readFileSync( + path.resolve(fileURLToPath(import.meta.url), '../../package.json'), + 'utf8', + ), + ) as { browser?: unknown; exports: Record } + + expect(packageJson.browser).toBeUndefined() + expect(JSON.stringify(packageJson.exports)).not.toContain('"browser"') + }) +}) From 26bafc8e97f1a8f0eeea84c22332d77212b98499 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 26 Aug 2026 18:27:07 +1000 Subject: [PATCH 3/8] test(stack): port the clientKey contract test to the protect-ffi 0.31 shape The two commits recovered from the #804 branch were written on 2026-07-28, against protect-ffi 0.30. Cherry-picking them onto main compiled but every credential assertion failed: 0.31 moved `clientId` / `clientKey` under `clientOpts` and denies unknown keys at the top level, so the calls were rejected with `unknown field 'clientId'` before reaching anything the test meant to observe. Nesting the credentials fixed three of six. The rest were asserting error strings the core no longer emits: missing field `clientKey` -> clientOpts.clientId and clientOpts.clientKey are required (one message names both; the core does not say which is absent) invalid clientKey: invalid hex -> invalid clientKey: expected a hex-encoded key opts.strategy is required -> Not authenticated The last one carried the file's ordering claim, so it was re-derived rather than re-spelled: probing all three combinations against the real core shows `Not authenticated` wins whenever `strategy` is absent, while omitting the credentials WITH a strategy gives the credential error. The strategy is still read first, which is what makes "even when an auth strategy is supplied" mean anything in the tests above. The finding the file exists to pin is unchanged and now pinned against the shipping core: federation does not remove the `clientKey` requirement. Claude-Session: https://claude.ai/code/session_01E1J2nVGJWVkqvLepDfinRf --- ...sm-inline-core-credential-contract.test.ts | 59 ++++++++++++------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts index a68457fa7..e30800f0a 100644 --- a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -116,17 +116,24 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 await expect( newClient({ - clientId: CLIENT_ID, + clientOpts: { clientId: CLIENT_ID }, strategy, encryptConfig, eqlVersion: 3, }), - ).rejects.toThrow(/missing field `clientKey`/) + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) - // Rejected during deserialisation of the options struct — the strategy was + // Rejected while the core builds its key provider — the strategy was // never INVOKED, so federation cannot substitute for the key. (The core // does look at `opts.strategy` before this point, to check it is present // and carries a `getToken`; what never happens is the call.) + // + // Both credential fields are named in one message: the core does not + // report which of the pair is missing, it requires both. So this test and + // the next assert the same string, and each is carried by the field it + // omits rather than by a distinct error. expect(calls.getToken).toBe(0) }) @@ -135,12 +142,14 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 await expect( newClient({ - clientKey: HEX_BUT_NOT_KEY_MATERIAL, + clientOpts: { clientKey: HEX_BUT_NOT_KEY_MATERIAL }, strategy, encryptConfig, eqlVersion: 3, }), - ).rejects.toThrow(/missing field `clientId`/) + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) expect(calls.getToken).toBe(0) }) @@ -151,19 +160,20 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 const hexStage = federationStrategy() await expect( newClient({ - clientId: CLIENT_ID, - clientKey: 'not-hex', + clientOpts: { clientId: CLIENT_ID, clientKey: 'not-hex' }, strategy: hexStage.strategy, encryptConfig, eqlVersion: 3, }), - ).rejects.toThrow(/invalid clientKey: invalid hex/) + ).rejects.toThrow(/invalid clientKey: expected a hex-encoded key/) const providerStage = federationStrategy() await expect( newClient({ - clientId: CLIENT_ID, - clientKey: HEX_BUT_NOT_KEY_MATERIAL, + clientOpts: { + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + }, strategy: providerStage.strategy, encryptConfig, eqlVersion: 3, @@ -193,8 +203,7 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 // #804 rather than to loosen the assertion. await expect( newClient({ - clientId: CLIENT_ID, - clientKey: CBOR_KEY_WITH_BAD_P1, + clientOpts: { clientId: CLIENT_ID, clientKey: CBOR_KEY_WITH_BAD_P1 }, strategy, encryptConfig, eqlVersion: 3, @@ -207,23 +216,27 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 it('reads the strategy off `opts.strategy`, before it deserialises the credentials', async () => { // Fixes the meaning of every test above. They all assert "even when an // auth strategy is supplied" — which is only worth anything if the core - // reads the field they supply it on. It does: omitting `strategy` - // entirely beats `missing field \`clientKey\`` to the punch, so the - // strategy is seen before the credentials are even deserialised. + // reads the field they supply it on. It does: with `strategy` omitted the + // core answers `Not authenticated` whether or not credentials are + // present, while credentials omitted WITH a strategy gives the credential + // error — so the strategy is seen first. Verified by probing all three + // combinations against the real core, not inferred from the source. // // This is also the drift guard. If protect-ffi renamed the option, the // tests above would be handing the core nothing and quietly testing the // no-strategy path instead. They would fail rather than pass silently - // (`opts.strategy is required` matches none of their regexes), and this - // test names the reason. + // (`Not authenticated` matches none of their regexes), and this test + // names the reason. await expect( newClient({ - clientId: CLIENT_ID, - clientKey: HEX_BUT_NOT_KEY_MATERIAL, + clientOpts: { + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + }, encryptConfig, eqlVersion: 3, }), - ).rejects.toThrow(/opts\.strategy is required/) + ).rejects.toThrow(/Not authenticated/) }) it('invokes `getToken` only after key loading succeeds', async () => { @@ -241,8 +254,10 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 // counter these tests rely on is live. await expect( newClient({ - clientId: CLIENT_ID, - clientKey: WELL_FORMED_KEY_MATERIAL, + clientOpts: { + clientId: CLIENT_ID, + clientKey: WELL_FORMED_KEY_MATERIAL, + }, strategy, encryptConfig, eqlVersion: 3, From 53d4b2a19a38071dbf38c51bc823713779a43ac3 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 31 Aug 2026 10:37:52 +1000 Subject: [PATCH 4/8] ci(stack): run the clientKey contract test where the WASM build exists (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract test added two commits back resolves the REAL `@cipherstash/protect-ffi/wasm-inline` through Node — that is the point of it, since every other wasm suite gets the stub `vitest.shared.ts` aliases in. That entry is `dist/wasm/protect_ffi_inline.js`, wasm-pack output; only the three `.d.ts` beside it are tracked, and protect-ffi is a workspace package now, so `pnpm install` does not produce it. `run-tests` builds the binding without `wasm: 'true'` — deliberately, and its step says so — so the file could never collect there. It did not skip, it failed to COLLECT, which took the Node 24 job red on #953. A local `pnpm --filter @cipherstash/stack test` would have needed cargo and wasm-pack for the same reason. So it moves to its own config, the way the integration suites already do, and runs from `tests.yml`'s `wasm-e2e-tests` job — the one job that builds `dist/wasm/**`. Through turbo rather than a bare `pnpm --filter`, because `workflow-turbo-build-deps.test.mjs` rejects the bare form for a package with buildable workspace dependencies; the task is `cache: false` because the input this suite actually reads is a gitignored build output in ANOTHER package, which `$TURBO_DEFAULT$` cannot see. Three separate pieces now hold the suite up — an exclusion, a script, one workflow step — and deleting any of them leaves a green tree with the contract checked nowhere. `scripts/__tests__/wasm-core-contract-ci.test.mjs` is what fails instead; removing the step was mutation-checked against it. Not verified: that the suite PASSES against a real core. Building dist/wasm locally is a wasm32 cargo build of the cipherstash-client graph and there is no disk for it here. What was verified is that the CI command resolves the task, selects exactly that file, and fails on nothing but the missing artifact. Refs #804 --- .github/workflows/tests.yml | 22 ++++ ...sm-inline-core-credential-contract.test.ts | 11 ++ packages/stack/package.json | 3 +- packages/stack/vitest.config.ts | 10 +- packages/stack/vitest.wasm-core.config.ts | 46 +++++++ .../__tests__/wasm-core-contract-ci.test.mjs | 114 ++++++++++++++++++ turbo.json | 17 +++ 7 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 packages/stack/vitest.wasm-core.config.ts create mode 100644 scripts/__tests__/wasm-core-contract-ci.test.mjs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1e782fe9d..f2863fdc3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -501,6 +501,28 @@ jobs: - name: Typecheck the generated WASM declarations run: pnpm --filter @cipherstash/protect-ffi run test:typecheck:wasm + # The `clientKey` contract test (#804), and the only suite in + # `packages/stack` that loads the REAL WASM core instead of the stub in + # `vitest.shared.ts` — so this is the only job that can run it, for the + # same reason as the step above. It is excluded from stack's default + # vitest config, which runs in `run-tests`, where the binding is built + # without `wasm: 'true'` and `dist/wasm/protect_ffi_inline.js` therefore + # does not exist: the file failed to COLLECT there, which is not a skip. + # + # Before `Build stack` deliberately — it reads protect-ffi's output + # directly and none of stack's, so a core that changed its credential + # contract fails here rather than after another build. + # + # Offline despite the credentials in this job's env: every assertion + # lands during argument deserialisation or key loading, before any + # ZeroKMS / CTS call. It is here for the WASM build, not the secrets. + # + # `scripts/__tests__/wasm-core-contract-ci.test.mjs` fails if this step + # goes away while the exclusion stays — a suite that no job runs reads + # exactly like a suite that passes. + - name: Test the WASM core credential contract (stack) + run: pnpm exec turbo run test:wasm-core --filter @cipherstash/stack + # The Deno smoke tests import the locally-built dist/wasm-inline.js of # BOTH packages via file URLs in e2e/wasm/deno.json — they need fresh # builds. stack-supabase is here for `supabase-declared.test.ts`, which diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts index e30800f0a..7c2921dff 100644 --- a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -32,6 +32,17 @@ * `e2e/wasm/roundtrip.test.ts` (Deno) — note that it exercises the * `accessKey` arm, so the federation arm reasoned about here has no live * coverage anywhere. + * + * WHERE IT RUNS, and why not with the rest of the suite. Loading the real + * module means loading `dist/wasm/protect_ffi_inline.js`, which wasm-pack + * emits and `pnpm install` does not — only the three `.d.ts` beside it are + * tracked. So this file is excluded from `packages/stack/vitest.config.ts` and + * collected by `vitest.wasm-core.config.ts` instead, run by `test:wasm-core` + * from `tests.yml`'s `wasm-e2e-tests` job, the one job that builds it. Left in + * the default config it does not skip — it fails to COLLECT, which is what it + * did in `run-tests` (#953). To run it locally, build the WASM output first: + * `pnpm --filter @cipherstash/protect-ffi run build:wasm` (needs cargo, the + * wasm32 target and wasm-pack). */ import { readFileSync } from 'node:fs' diff --git a/packages/stack/package.json b/packages/stack/package.json index 0b8a954c7..b65ba1a4f 100644 --- a/packages/stack/package.json +++ b/packages/stack/package.json @@ -200,7 +200,8 @@ "test:types": "vitest --run --typecheck.only", "test:types:dist": "tsc --noEmit -p dist-types/tsconfig.json && tsc --noEmit -p dist-types/node16/tsconfig.json", "release": "tsup", - "test:integration": "vitest run --config integration/vitest.config.ts" + "test:integration": "vitest run --config integration/vitest.config.ts", + "test:wasm-core": "vitest run --config vitest.wasm-core.config.ts" }, "devDependencies": { "@cipherstash/eql": "workspace:^", diff --git a/packages/stack/vitest.config.ts b/packages/stack/vitest.config.ts index 5ae79ab89..52cebe7d2 100644 --- a/packages/stack/vitest.config.ts +++ b/packages/stack/vitest.config.ts @@ -1,5 +1,6 @@ import { configDefaults, defineConfig } from 'vitest/config' import { sharedAlias, stackSourceAlias } from '../../vitest.shared' +import { WASM_CORE_SUITE } from './vitest.wasm-core.config' export default defineConfig({ resolve: { @@ -16,7 +17,14 @@ export default defineConfig({ // database and PostgREST. They THROW rather than skip when unconfigured, so // they must never be picked up by `pnpm test` — that is the whole reason // they are a separate config and a separate CI job. - exclude: [...configDefaults.exclude, 'integration/**'], + // + // `WASM_CORE_SUITE` is excluded for a narrower reason: it loads the real + // protect-ffi WASM core, which is wasm-pack output that `pnpm install` + // does not produce, so here it would fail to COLLECT rather than skip. It + // runs from `tests.yml`'s `wasm-e2e-tests` job under + // `vitest.wasm-core.config.ts` — imported rather than spelled twice, so + // the exclusion and the include cannot drift apart. + exclude: [...configDefaults.exclude, 'integration/**', WASM_CORE_SUITE], // Live suites make real ZeroKMS / CTS network round-trips. The vast // majority of these tests already pass an explicit `, 30000)` per-test // timeout (300+ call sites); a handful were written without one and so diff --git a/packages/stack/vitest.wasm-core.config.ts b/packages/stack/vitest.wasm-core.config.ts new file mode 100644 index 000000000..0cb684bf7 --- /dev/null +++ b/packages/stack/vitest.wasm-core.config.ts @@ -0,0 +1,46 @@ +import { defineConfig } from 'vitest/config' + +/** + * The one suite that loads the REAL protect-ffi WASM core. + * + * Deliberately a SEPARATE config from `packages/stack/vitest.config.ts`, for + * the same reason `integration/vitest.config.ts` is one: the default suite has + * to run with nothing but a checkout and `pnpm install`, and this file needs a + * build that neither of those produces. + * + * `@cipherstash/protect-ffi` is a workspace package now, so its `./wasm-inline` + * entry resolves to `dist/wasm/protect_ffi_inline.js` — wasm-pack output, and + * only the three `.d.ts` beside it are tracked in git. Left in the default + * config the file does not skip, it fails to COLLECT + * (`Cannot find module '.../dist/wasm/protect_ffi_inline.js'`), which is what + * turned `run-tests` red: that job builds the binding without `wasm: 'true'`, + * as does the Bun job, and both say so in a comment. A local + * `pnpm --filter @cipherstash/stack test` would have needed cargo and wasm-pack + * too. + * + * So it runs from `tests.yml`'s `wasm-e2e-tests` job — the one job that builds + * `dist/wasm/**` — via the `test:wasm-core` script. + * `scripts/__tests__/wasm-core-contract-ci.test.mjs` holds that wiring + * together: a suite excluded from the default config and invoked by no job + * reads exactly like a suite that passes. + * + * NO ALIASES, and that is the point rather than an omission. + * `stackSourceAlias` maps `@cipherstash/protect-ffi/wasm-inline` to a stub + * whose `newClient` throws, which is right for the unit suites and would make + * this one assert against nothing. The test resolves the module through Node + * rather than the bare specifier, so it dodges that alias wherever it runs; + * leaving the map out here means it does not have to. + */ +export const WASM_CORE_SUITE = + '__tests__/wasm-inline-core-credential-contract.test.ts' + +export default defineConfig({ + test: { + root: __dirname, + include: [WASM_CORE_SUITE], + // The default, set explicitly: a rename that leaves the glob behind must + // fail rather than report a green run of zero files — the one failure mode + // a single-file `include` invites. + passWithNoTests: false, + }, +}) diff --git a/scripts/__tests__/wasm-core-contract-ci.test.mjs b/scripts/__tests__/wasm-core-contract-ci.test.mjs new file mode 100644 index 000000000..9c20c8627 --- /dev/null +++ b/scripts/__tests__/wasm-core-contract-ci.test.mjs @@ -0,0 +1,114 @@ +import { existsSync, readFileSync } from 'node:fs' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import stackVitestConfig from '../../packages/stack/vitest.config.ts' +import wasmCoreVitestConfig, { + WASM_CORE_SUITE, +} from '../../packages/stack/vitest.wasm-core.config.ts' +import { REPO_ROOT } from './lib/repo-root.mjs' +import { readWorkflow, workflowFiles } from './lib/workflows.mjs' + +/** + * `packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts` is + * the only suite in the repo that loads the REAL protect-ffi WASM core, and it + * is therefore the only one that cannot run where the rest of stack's tests do. + * + * That leaves it held up by three separate pieces — an exclusion in stack's + * default vitest config, a `test:wasm-core` script pointing at its own config, + * and one step in the single CI job that builds `dist/wasm/**` — and removing + * ANY of them leaves a green tree. Delete the workflow step and the suite runs + * nowhere; the exclusion keeps `pnpm test` passing and nothing says the + * contract stopped being checked. That is the failure this file exists for, + * and it is the same shape as `packages/protect-ffi/src/integrationSuiteCi.test.ts` + * (a suite whose workflow was deposited where GitHub never reads it) and + * `lintWiring.test.ts`'s "a check nothing invokes reads exactly like a check + * that passes". + * + * The opposite direction is already loud, which is why it is not asserted here: + * put the file back in the default config and `run-tests` fails to COLLECT it + * with `Cannot find module '.../dist/wasm/protect_ffi_inline.js'` — that is the + * failure (#953) that produced this arrangement. + * + * The job is discovered by the `wasm: 'true'` input rather than named, so a + * future job that also builds the WASM output can host the step without + * editing this file — and naming the wrong job cannot pass, since a job that + * does not build wasm cannot run the suite at all. + */ + +const BUILD_FFI = './.github/actions/build-ffi-binding' +const SCRIPT = 'test:wasm-core' +const PACKAGE = '@cipherstash/stack' + +const stackPackageJson = JSON.parse( + readFileSync(join(REPO_ROOT, 'packages/stack/package.json'), 'utf8'), +) + +/** Every `run:` line in the workflow graph, tagged with its job. */ +function runStepsByJob() { + const rows = [] + for (const file of workflowFiles()) { + const workflow = readWorkflow(file) + for (const [jobId, job] of Object.entries(workflow?.jobs ?? {})) { + for (const step of job?.steps ?? []) { + rows.push({ file, jobId, job, step }) + } + } + } + return rows +} + +/** Does this job build protect-ffi's `dist/wasm/**`? */ +function buildsWasm(job) { + return (job?.steps ?? []).some( + (step) => step?.uses === BUILD_FFI && String(step?.with?.wasm) === 'true', + ) +} + +describe('the WASM core credential contract runs somewhere (#804)', () => { + it('the suite the whole arrangement is about still exists', () => { + expect(existsSync(join(REPO_ROOT, 'packages/stack', WASM_CORE_SUITE))).toBe( + true, + ) + }) + + it('is the only file its own config collects', () => { + expect(wasmCoreVitestConfig.test.include).toEqual([WASM_CORE_SUITE]) + // An empty run must be red: with a single-file `include`, a rename is + // otherwise indistinguishable from a pass. + expect(wasmCoreVitestConfig.test.passWithNoTests).toBe(false) + }) + + it('is excluded from the default suite, which has no WASM build', () => { + expect(stackVitestConfig.test.exclude).toContain(WASM_CORE_SUITE) + }) + + it(`is invoked by \`${SCRIPT}\`, pointed at that config`, () => { + const command = stackPackageJson.scripts?.[SCRIPT] + expect(command, `packages/stack has no \`${SCRIPT}\` script`).toBeDefined() + expect(command).toContain('vitest.wasm-core.config.ts') + }) + + it('is run by a CI job that builds the WASM output', () => { + const invocations = runStepsByJob().filter( + ({ step }) => + typeof step?.run === 'string' && + step.run.includes(SCRIPT) && + step.run.includes(PACKAGE), + ) + + expect( + invocations.map(({ file, jobId }) => `${file} / ${jobId}`), + `No workflow job runs \`turbo run ${SCRIPT} --filter ${PACKAGE}\`.\n` + + `${WASM_CORE_SUITE} is excluded from stack's default vitest config, so with no job invoking it the contract is checked NOWHERE — and \`pnpm test\` stays green.\n` + + `Add the step back to a job that passes \`wasm: 'true'\` to ${BUILD_FFI} (today: tests.yml / wasm-e2e-tests), or delete the suite and its config through #804 rather than letting it go quiet.`, + ).not.toHaveLength(0) + + for (const { file, jobId, job } of invocations) { + expect( + buildsWasm(job), + `${file} / ${jobId} runs ${SCRIPT} but does not build protect-ffi's dist/wasm.\n` + + `The suite resolves \`@cipherstash/protect-ffi/wasm-inline\` for real; without \`wasm: 'true'\` on ${BUILD_FFI} it fails to collect.`, + ).toBe(true) + } + }) +}) diff --git a/turbo.json b/turbo.json index 4cfa92f5e..149a38b8d 100644 --- a/turbo.json +++ b/turbo.json @@ -90,6 +90,23 @@ "dependsOn": ["^build", "build"], "inputs": ["$TURBO_DEFAULT$", ".env*"], "cache": false + }, + // `packages/stack`'s WASM-core contract suite — see + // `packages/stack/vitest.wasm-core.config.ts` for why it is not part of + // `test`. A task rather than a bare `pnpm --filter` because + // `scripts/__tests__/workflow-turbo-build-deps.test.mjs` requires one: + // stack depends on workspace packages that emit build output, so a bare + // invocation resolves `dist/` that nothing in the command builds. + // + // `cache: false`, and not for the reason `test` is. The input this suite + // actually reads — protect-ffi's `dist/wasm/protect_ffi_inline.js` — is a + // gitignored build output in ANOTHER package, so `$TURBO_DEFAULT$` cannot + // see it: a rebuilt core with unchanged stack sources would otherwise be a + // cache hit reporting a pass over the previous core. + "test:wasm-core": { + "dependsOn": ["^build"], + "inputs": ["$TURBO_DEFAULT$"], + "cache": false } } } From f1373e11f336b01ef57feb93c8e62e9603af337e Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 31 Aug 2026 11:10:45 +1000 Subject: [PATCH 5/8] docs(stack): correct the claims around the WASM-core suite, and guard the fourth piece (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found four things wrong with the reasoning around the apparatus added in 53d4b2a1. None of them touches the fix itself — the contract test passed against the real core in CI on that commit — but three of the four were claims this repo would later be read as evidence for. "The only suite that loads the REAL WASM core" was false, and it was the stated justification for the whole arrangement. `packages/stack/integration/wasm/**` loads it (its config restores the genuine module over the stub alias), so do protect-ffi's own `wasm-round-trip` / `wasm-error-codes` suites and the three Deno suites in `e2e/wasm/`; three CI jobs build `dist/wasm/**`, not one. Stated in four places, corrected in all four. The true reason this file is not simply in `integration/wasm/**` is narrower and is now written down instead: it is the only one of those suites that needs the core and NOTHING else. `test-kit`'s integration `globalSetup` requires credentials AND a database unconditionally — it throws rather than skips, then runs a real `stash eql install` — and `integration-drizzle.yml` is path-filtered, fork-skipped and matrixed over two databases. A contract about the core would go unchecked on any diff those paths do not select. The guard held three of four load-bearing pieces; `turbo.json` was unasserted. The reviewer's proposed failure — delete the task and it goes quiet — turns out not to exist: turbo 2.x refuses a task the project does not declare, so the step exits 1. The silent one is the EDIT. Measured by dry run: strip `dependsOn` and flip `cache` to true and the build graph collapses from nine tasks to one while the suite becomes cacheable against a hash that cannot see the core it tests. Every other turbo guard stayed green through that mutation. Now asserted, and all four mutations (delete, cache-only, dependsOn-only, both) were confirmed to fail with the message that names the reason. `"inputs": ["$TURBO_DEFAULT$"]` is dropped, though not for the reason given. Turbo does compute an input set for an uncached task — narrowing `inputs` under `cache: false` cuts the hashed set from 170 files to 2 and changes the hash. The key is a no-op because `$TURBO_DEFAULT$` alone IS the default set, cached or not. Five other tasks carry the same no-op line; this one goes because the comment beside it reasons about what turbo can and cannot hash, which reads as though the key were load-bearing. Refs #804 --- .github/workflows/tests.yml | 22 ++++-- ...sm-inline-core-credential-contract.test.ts | 18 +++-- packages/stack/vitest.wasm-core.config.ts | 26 +++++-- .../__tests__/wasm-core-contract-ci.test.mjs | 76 +++++++++++++++---- turbo.json | 25 ++++-- 5 files changed, 130 insertions(+), 37 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f2863fdc3..2b02787f0 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -501,13 +501,21 @@ jobs: - name: Typecheck the generated WASM declarations run: pnpm --filter @cipherstash/protect-ffi run test:typecheck:wasm - # The `clientKey` contract test (#804), and the only suite in - # `packages/stack` that loads the REAL WASM core instead of the stub in - # `vitest.shared.ts` — so this is the only job that can run it, for the - # same reason as the step above. It is excluded from stack's default - # vitest config, which runs in `run-tests`, where the binding is built - # without `wasm: 'true'` and `dist/wasm/protect_ffi_inline.js` therefore - # does not exist: the file failed to COLLECT there, which is not a skip. + # The `clientKey` contract test (#804). It loads the REAL WASM core + # instead of the stub in `vitest.shared.ts`, so it needs the step above: + # excluded from stack's default vitest config, which runs in + # `run-tests`, where the binding is built without `wasm: 'true'` and + # `dist/wasm/protect_ffi_inline.js` therefore does not exist — there the + # file failed to COLLECT, which is not a skip. + # + # Not the only suite that loads the real core, and this is not the only + # job that builds it for one — `integration-drizzle.yml` runs + # `packages/stack`'s `integration/wasm/**` the same way. It is here + # because it is the only one of them that needs NOTHING else: no + # credentials, no database, no PostgREST. The integration job has all + # three and is path-filtered and fork-skipped besides, so hosting a core + # contract there would leave it unchecked on most diffs. See + # `packages/stack/vitest.wasm-core.config.ts` for the long form. # # Before `Build stack` deliberately — it reads protect-ffi's output # directly and none of stack's, so a core that changed its credential diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts index 7c2921dff..bdeb99372 100644 --- a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -13,12 +13,18 @@ * browser-safe. That is why there is no `browser` export condition and no * browser smoke test. * - * Nothing else in the suite could have caught this: every wasm test that - * constructs a client mocks `newClient`, and `vitest.shared.ts` aliases the - * whole `@cipherstash/protect-ffi/wasm-inline` specifier to a stub whose - * `newClient` throws. So this file resolves the REAL module through Node — - * which the Vite alias does not intercept — and asserts against the actual - * core. + * Nothing else could have caught this, for two different reasons. Every wasm + * test in stack's DEFAULT suite that constructs a client mocks `newClient`, + * and `vitest.shared.ts` aliases the whole + * `@cipherstash/protect-ffi/wasm-inline` specifier to a stub whose `newClient` + * throws — so none of them reaches the core at all. The suites that DO reach + * it — `integration/wasm/**` here, protect-ffi's own `wasm-round-trip` and + * `wasm-error-codes`, the Deno smoke tests in `e2e/wasm/` — all hand it a + * complete, real credential, because their point is a round trip. A + * requirement is invisible to a caller that always satisfies it. This file is + * the one that OMITS the credential, so it resolves the REAL module through + * Node — which the Vite alias does not intercept — and asserts against the + * actual core. * * IF THIS TEST FAILS, THAT IS GOOD NEWS. It means the core relaxed the * requirement and browser support should be re-examined: the `browser` export diff --git a/packages/stack/vitest.wasm-core.config.ts b/packages/stack/vitest.wasm-core.config.ts index 0cb684bf7..b6f7a0146 100644 --- a/packages/stack/vitest.wasm-core.config.ts +++ b/packages/stack/vitest.wasm-core.config.ts @@ -1,12 +1,28 @@ import { defineConfig } from 'vitest/config' /** - * The one suite that loads the REAL protect-ffi WASM core. + * The one suite in stack's DEFAULT test run that loads the REAL protect-ffi + * WASM core — NOT the only one in the repo that loads it. `integration/wasm/**` + * does (its config restores the genuine module over the stub alias), as do + * protect-ffi's own `wasm-round-trip` / `wasm-error-codes` suites and the Deno + * smoke tests in `e2e/wasm/`. Three CI jobs build `dist/wasm/**` for them. * - * Deliberately a SEPARATE config from `packages/stack/vitest.config.ts`, for - * the same reason `integration/vitest.config.ts` is one: the default suite has - * to run with nothing but a checkout and `pnpm install`, and this file needs a - * build that neither of those produces. + * What is unusual here is the COMBINATION: this file needs the real core and + * nothing else — no credentials, no database, no PostgREST — because every + * assertion lands before the first network call. That is what puts it in an + * awkward middle. It cannot stay with the unit suites, and joining the + * integration suites would give it dependencies it does not have: + * `packages/test-kit/src/integration/global-setup.ts` requires credentials AND + * a database unconditionally (it throws rather than skips, then runs a real + * `stash eql install`), and `integration-drizzle.yml`, the workflow that runs + * them, is path-filtered, fork-skipped and matrixed over two databases. A + * contract about the core would then go unchecked on any diff those paths do + * not select. + * + * So: a SEPARATE config from `packages/stack/vitest.config.ts`, for the same + * reason `integration/vitest.config.ts` is one — the default suite has to run + * with nothing but a checkout and `pnpm install`, and this file needs a build + * that neither of those produces. * * `@cipherstash/protect-ffi` is a workspace package now, so its `./wasm-inline` * entry resolves to `dist/wasm/protect_ffi_inline.js` — wasm-pack output, and diff --git a/scripts/__tests__/wasm-core-contract-ci.test.mjs b/scripts/__tests__/wasm-core-contract-ci.test.mjs index 9c20c8627..6e86bf278 100644 --- a/scripts/__tests__/wasm-core-contract-ci.test.mjs +++ b/scripts/__tests__/wasm-core-contract-ci.test.mjs @@ -5,24 +5,45 @@ import stackVitestConfig from '../../packages/stack/vitest.config.ts' import wasmCoreVitestConfig, { WASM_CORE_SUITE, } from '../../packages/stack/vitest.wasm-core.config.ts' +import { readJsonc } from './lib/read-jsonc.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' /** - * `packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts` is - * the only suite in the repo that loads the REAL protect-ffi WASM core, and it - * is therefore the only one that cannot run where the rest of stack's tests do. + * `packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts` loads + * the REAL protect-ffi WASM core. It is NOT the only suite that does — + * `packages/stack/integration/wasm/**`, protect-ffi's own `wasm-round-trip` / + * `wasm-error-codes`, and the Deno smoke tests in `e2e/wasm/` all do, and three + * CI jobs build `dist/wasm/**` for them. It is the only one that needs the core + * and NOTHING ELSE — no credentials, no database — which is what put it in its + * own config rather than into the integration suites, whose `globalSetup` + * requires both unconditionally. This docblock claimed the stronger thing until + * review caught it; the arrangement below never depended on it. * - * That leaves it held up by three separate pieces — an exclusion in stack's - * default vitest config, a `test:wasm-core` script pointing at its own config, - * and one step in the single CI job that builds `dist/wasm/**` — and removing - * ANY of them leaves a green tree. Delete the workflow step and the suite runs - * nowhere; the exclusion keeps `pnpm test` passing and nothing says the - * contract stopped being checked. That is the failure this file exists for, - * and it is the same shape as `packages/protect-ffi/src/integrationSuiteCi.test.ts` - * (a suite whose workflow was deposited where GitHub never reads it) and - * `lintWiring.test.ts`'s "a check nothing invokes reads exactly like a check - * that passes". + * What the arrangement does depend on is FOUR pieces, and losing any one leaves + * the contract unchecked: + * + * 1. the exclusion in `packages/stack/vitest.config.ts` + * 2. the `test:wasm-core` script in `packages/stack/package.json` + * 3. the step in the CI job that builds `dist/wasm/**` + * 4. the `test:wasm-core` task in `turbo.json` + * + * Piece 3 is the quiet one: delete the step and the suite runs nowhere while + * the exclusion keeps `pnpm test` green, so nothing says the contract stopped + * being checked. Same shape as + * `packages/protect-ffi/src/integrationSuiteCi.test.ts` (a suite whose workflow + * was deposited where GitHub never reads it) and `lintWiring.test.ts`'s "a + * check nothing invokes reads exactly like a check that passes". + * + * Piece 4 is quiet in a narrower way, and the assertions below say exactly + * which way. DELETING the task is loud — turbo 2.x refuses to run a task the + * project does not declare, so the step exits 1 — but EDITING it is not. + * Dry-run measured: strip `dependsOn` and flip `cache` to true and the build + * graph collapses from nine tasks to one (`@cipherstash/eql#build` and + * `@cipherstash/protect-ffi#build` stop running) while the suite becomes + * cacheable against a hash that cannot see the core it tests. Every other turbo + * guard in this directory stayed green through that mutation, which is why the + * check lives here. * * The opposite direction is already loud, which is why it is not asserted here: * put the file back in the default config and `run-tests` fails to COLLECT it @@ -43,6 +64,9 @@ const stackPackageJson = JSON.parse( readFileSync(join(REPO_ROOT, 'packages/stack/package.json'), 'utf8'), ) +/** `turbo.json` carries comments, so it needs the jsonc reader. */ +const turboJson = readJsonc(join(REPO_ROOT, 'turbo.json')) + /** Every `run:` line in the workflow graph, tagged with its job. */ function runStepsByJob() { const rows = [] @@ -88,6 +112,32 @@ describe('the WASM core credential contract runs somewhere (#804)', () => { expect(command).toContain('vitest.wasm-core.config.ts') }) + it('is a turbo task that builds its deps and never caches', () => { + const task = turboJson.tasks?.[SCRIPT] + expect( + task, + `turbo.json declares no \`${SCRIPT}\` task. The workflow step runs it through turbo, and turbo 2.x refuses a task the project does not declare — so this one fails loudly rather than silently. Re-add it.`, + ).toBeDefined() + + // The two fields, and the two different damages. Without `^build` the + // graph collapses to this task alone and the workspace packages it + // resolves are never built; with caching on, the suite's real input — + // protect-ffi's gitignored `dist/wasm/**`, in another package — is + // invisible to the hash, so a rebuilt core over unchanged stack sources + // is a cache hit reporting a pass over the previous core. + // `?? []` so an ABSENT `dependsOn` — the likelier edit of the two — fails + // on the message below rather than on `toContain(undefined)`, which + // reports an argument-type complaint and buries the reason. + expect( + task.dependsOn ?? [], + `${SCRIPT} must depend on \`^build\`: without it turbo runs the task alone and stack's workspace dependencies go unbuilt.`, + ).toContain('^build') + expect( + task.cache, + `${SCRIPT} must set \`cache: false\`. Its real input is protect-ffi's dist/wasm, which turbo cannot hash — a cached run would report a pass over a core it never loaded.`, + ).toBe(false) + }) + it('is run by a CI job that builds the WASM output', () => { const invocations = runStepsByJob().filter( ({ step }) => diff --git a/turbo.json b/turbo.json index 149a38b8d..9c97b7d58 100644 --- a/turbo.json +++ b/turbo.json @@ -98,14 +98,27 @@ // stack depends on workspace packages that emit build output, so a bare // invocation resolves `dist/` that nothing in the command builds. // - // `cache: false`, and not for the reason `test` is. The input this suite - // actually reads — protect-ffi's `dist/wasm/protect_ffi_inline.js` — is a - // gitignored build output in ANOTHER package, so `$TURBO_DEFAULT$` cannot - // see it: a rebuilt core with unchanged stack sources would otherwise be a - // cache hit reporting a pass over the previous core. + // `cache: false` is the load-bearing half. The input this suite actually + // reads — protect-ffi's `dist/wasm/protect_ffi_inline.js` — is a gitignored + // build output in ANOTHER package, so nothing turbo hashes can see it: with + // caching on, a rebuilt core over unchanged stack sources is a cache hit + // reporting a pass over the PREVIOUS core. + // + // No `inputs` key, deliberately. `["$TURBO_DEFAULT$"]` is the sentinel for + // the default set, so writing it alone resolves to exactly what omitting + // the key does — byte-identical dry-run output, same hash. (It is NOT + // inert because the task is uncached: turbo computes a full input set and + // hash either way, and a narrowed `inputs` under `cache: false` really does + // change both. Five cached tasks above carry the same no-op line; this one + // is dropped rather than copied because the comment beside it would read + // as though input tracking were doing work here.) + // + // `scripts/__tests__/wasm-core-contract-ci.test.mjs` asserts both halves. + // Deleting this task is loud — turbo 2.x refuses an undeclared task and the + // step exits 1 — but editing it is not: strip `dependsOn` and flip the + // cache and the build graph collapses from nine tasks to one, silently. "test:wasm-core": { "dependsOn": ["^build"], - "inputs": ["$TURBO_DEFAULT$"], "cache": false } } From 4ab7a82e30ec2593038c9df9033be1112d37ee30 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 31 Aug 2026 11:41:16 +1000 Subject: [PATCH 6/8] test(stack): assert the credential contract on the key production passes (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contract test supplied its auth strategy as `opts.strategy`. That is protect-ffi's deprecated alias — `NewClientOptions` marks it "Renamed to authStrategy", and `wasm.rs` reads `authStrategy` first and falls back to it — while `src/wasm-inline.ts` builds its call as `authStrategy: strategy`. So the assertion that clientKey is required even under OIDC federation was pinned on a field the shipping path never sets, and the day protect-ffi drops the alias four tests go red with `Not authenticated`, reading as a core regression rather than a rename. Both arms now run, named in their titles, so an alias removal turns exactly one of them red and `STRATEGY_KEYS` says what to do about it. Move the `browser` export-condition assertion into each package's default suite. It reads a manifest — no WASM build, no credentials — but it lived in the file `vitest.config.ts` excludes, so it ran only in `wasm-e2e-tests`: in no local `pnpm test`, and on a fork PR in nothing at all, since that job and `run-tests` both hard-fail at `require-cs-secrets` and `lint` runs only Biome. Adding a `browser` condition to quiet a bundler — the exact accident the comment names — passed everything a contributor runs. `@cipherstash/stack-supabase` gets the same guard. It carries the same "not browser-safe (#804)" note and the same `./wasm-inline` export, and nothing covered it. `vitest.wasm-core.config.ts` set no `testTimeout`, so it inherited Vitest's 5000ms while the sibling config deliberately raises it to 30000 for this package. Every case there instantiates the real inlined core and the last loads a key and calls `getToken` — a flake waiting for a cold runner, in the one job that runs it. `wasm-core-contract-ci.test.mjs` grows the two matching wiring checks. Claude-Session: https://claude.ai/code/session_01THGKcgRdyd2aPt5zBnPVLH --- .../browser-export-condition.test.ts | 38 ++ .../__tests__/wasm-entry-edge-safety.test.ts | 6 + .../browser-export-condition.test.ts | 52 +++ ...sm-inline-core-credential-contract.test.ts | 343 ++++++++++-------- packages/stack/vitest.wasm-core.config.ts | 13 + .../__tests__/wasm-core-contract-ci.test.mjs | 98 +++++ 6 files changed, 390 insertions(+), 160 deletions(-) create mode 100644 packages/stack-supabase/__tests__/browser-export-condition.test.ts create mode 100644 packages/stack/__tests__/browser-export-condition.test.ts diff --git a/packages/stack-supabase/__tests__/browser-export-condition.test.ts b/packages/stack-supabase/__tests__/browser-export-condition.test.ts new file mode 100644 index 000000000..326d0b0b0 --- /dev/null +++ b/packages/stack-supabase/__tests__/browser-export-condition.test.ts @@ -0,0 +1,38 @@ +/** + * `@cipherstash/stack-supabase` declares no `browser` export condition (#804). + * + * The sibling of `packages/stack/__tests__/browser-export-condition.test.ts`, + * and it exists because this package has a `wasm-inline` entry of its own. + * `./wasm-inline` binds the WASM engine from `@cipherstash/stack/wasm-inline`, + * and that engine's core requires `clientKey` — a workspace secret — on every + * auth path, OIDC federation included. So the entry is edge-safe and NOT + * browser-safe, which is what the note at the bottom of `src/wasm-inline.ts` + * says ("it is not browser-safe (#804)"). Nothing enforced it here: the two + * facts a bundler complaint would tempt someone to reconcile — an ESM-only, + * native-free entry that nonetheless must not reach a browser — sit in + * different files. + * + * Separate from `wasm-entry-edge-safety.test.ts`, which asserts the same + * entry's EDGE safety by scanning the emitted bundle and therefore skips when + * `dist/` is absent. This one reads the manifest, so it must not skip: a + * `browser` condition is wrong whether or not anyone has built the package. + */ + +import { readFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +describe('@cipherstash/stack-supabase declares no browser build (#804)', () => { + it('has no `browser` export condition on any subpath', () => { + const packageJson = JSON.parse( + readFileSync( + path.resolve(fileURLToPath(import.meta.url), '../../package.json'), + 'utf8', + ), + ) as { browser?: unknown; exports: Record } + + expect(packageJson.browser).toBeUndefined() + expect(JSON.stringify(packageJson.exports)).not.toContain('"browser"') + }) +}) diff --git a/packages/stack-supabase/__tests__/wasm-entry-edge-safety.test.ts b/packages/stack-supabase/__tests__/wasm-entry-edge-safety.test.ts index 935cc2e17..bdb334680 100644 --- a/packages/stack-supabase/__tests__/wasm-entry-edge-safety.test.ts +++ b/packages/stack-supabase/__tests__/wasm-entry-edge-safety.test.ts @@ -17,6 +17,12 @@ import { describe, expect, it } from 'vitest' * So this asserts on the emitted file. It is a build-output gate, and it skips * when `dist/` is absent so `pnpm test` stays green without a prior build — * the same shape as the adapter-kit edge-safety gate added in #799. + * + * Edge-safe is not browser-safe: the WASM engine this entry binds still needs + * a `clientKey`, which is a workspace secret. That half is asserted in + * `browser-export-condition.test.ts`, a sibling rather than a case here + * because it reads the manifest and so must NOT skip on an unbuilt tree + * (#804). */ const DIST = resolve(dirname(fileURLToPath(import.meta.url)), '../dist') diff --git a/packages/stack/__tests__/browser-export-condition.test.ts b/packages/stack/__tests__/browser-export-condition.test.ts new file mode 100644 index 000000000..3e27a17d1 --- /dev/null +++ b/packages/stack/__tests__/browser-export-condition.test.ts @@ -0,0 +1,52 @@ +/** + * `@cipherstash/stack` declares no `browser` export condition (#804). + * + * The consequence of the WASM core's credential contract, and the one part of + * it a reader can undo by accident. The core requires `clientId` AND + * `clientKey` on EVERY auth path — including OIDC federation, the arm that + * exists so a caller never handles a workspace secret — so + * `@cipherstash/stack/wasm-inline` is not browser-safe. That contract is + * asserted against the real core in + * `__tests__/wasm-inline-core-credential-contract.test.ts`; this file asserts + * the packaging that follows from it. + * + * `src/wasm-inline.ts` tells callers there is no `browser` condition and + * explains why. Nothing enforced it, so adding one to quiet a bundler + * complaint would ship a workspace secret to the browser and leave that + * doc silently wrong. + * + * WHY IT LIVES HERE and not with the contract file. It reads a manifest. It + * needs no WASM build, no credentials and no database, so it belongs in the + * suite every contributor runs. The contract file needs wasm-pack output that + * `pnpm install` does not produce, which is why it is excluded from + * `vitest.config.ts` and run by one CI job — and while this assertion lived + * inside it, it was checked by no local `pnpm --filter @cipherstash/stack + * test`, and on a fork PR by nothing at all (`wasm-e2e-tests` and `run-tests` + * both hard-fail at `require-cs-secrets` there; `lint` runs only Biome). + * `scripts/__tests__/wasm-core-contract-ci.test.mjs` holds it in the default + * suite. + * + * Same rule as the contract file: if the core stops requiring `clientKey`, + * come back through #804 — don't just delete this. The `browser` export + * condition (#805), a live browser smoke test and browser guidance in + * `skills/stash-supabase/SKILL.md` are all blocked on that and nothing else. + */ + +import { readFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' + +describe('@cipherstash/stack declares no browser build (#804)', () => { + it('has no `browser` export condition on any subpath', () => { + const packageJson = JSON.parse( + readFileSync( + path.resolve(fileURLToPath(import.meta.url), '../../package.json'), + 'utf8', + ), + ) as { browser?: unknown; exports: Record } + + expect(packageJson.browser).toBeUndefined() + expect(JSON.stringify(packageJson.exports)).not.toContain('"browser"') + }) +}) diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts index bdeb99372..6690b9655 100644 --- a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -11,7 +11,9 @@ * is not. The requirement is real, it comes from the Rust core, and `clientKey` * is a workspace secret, so `@cipherstash/stack/wasm-inline` is not * browser-safe. That is why there is no `browser` export condition and no - * browser smoke test. + * browser smoke test — `__tests__/browser-export-condition.test.ts` is what + * holds the packaging half of that, in the default suite where everyone runs + * it. * * Nothing else could have caught this, for two different reasons. Every wasm * test in stack's DEFAULT suite that constructs a client mocks `newClient`, @@ -26,11 +28,14 @@ * Node — which the Vite alias does not intercept — and asserts against the * actual core. * - * IF THIS TEST FAILS, THAT IS GOOD NEWS. It means the core relaxed the - * requirement and browser support should be re-examined: the `browser` export - * condition (#805), a live browser smoke test, and browser guidance in - * `skills/stash-supabase/SKILL.md` are all blocked on this and nothing else. - * Re-open #804 rather than deleting the assertions. + * IF THIS TEST FAILS, THAT IS GOOD NEWS — with one exception, named below. It + * means the core relaxed the requirement and browser support should be + * re-examined: the `browser` export condition (#805), a live browser smoke + * test, and browser guidance in `skills/stash-supabase/SKILL.md` are all + * blocked on this and nothing else. Re-open #804 rather than deleting the + * assertions. The exception is the `strategy` arm: that option name is + * deprecated in protect-ffi, and its removal is a rename rather than a + * relaxation. `STRATEGY_KEYS` below says what to do about it. * * Runs offline. Every failure asserted here happens during argument * deserialisation or key loading, before any ZeroKMS / CTS network call, so no @@ -51,10 +56,8 @@ * wasm32 target and wasm-pack). */ -import { readFileSync } from 'node:fs' import { createRequire } from 'node:module' -import path from 'node:path' -import { fileURLToPath, pathToFileURL } from 'node:url' +import { pathToFileURL } from 'node:url' import { describe, expect, it } from 'vitest' // Node's resolver, not Vite's — this is what dodges the stub alias. The @@ -127,123 +130,184 @@ function federationStrategy() { } } -describe('protect-ffi WASM core: credential contract under OIDC federation (#804)', () => { - it('requires `clientKey` even when an auth strategy is supplied', async () => { - const { calls, strategy } = federationStrategy() +/** + * The two option keys the core accepts an auth strategy on, and the reason + * every assertion below runs twice. + * + * `authStrategy` is the SHIPPING path: `src/wasm-inline.ts` builds its call as + * `wasmNewClient({ authStrategy: strategy, ... })`, so that arm is the one + * production takes and the one that has to keep passing. `strategy` is the + * former name — protect-ffi's `NewClientOptions` marks it `@deprecated + * Renamed to authStrategy`, the core reads it only when `authStrategy` is + * absent or nullish, and it is documented as going away. + * + * When protect-ffi drops it, ONLY the `strategy` arm turns red — the core + * will not have seen a strategy at all, so the arm fails at auth with `Not + * authenticated`, or on an unknown field if the key also stops being stripped + * before serde. Either way it is an alias removal, not a change to the + * credential contract: delete the `strategy` entry here and the whole arm + * goes with it. What it must not turn into is a hunt for a core regression, + * and the assertions must not be consolidated back onto a single key — + * running both is exactly what keeps the two failures distinguishable. + */ +const STRATEGY_KEYS = [ + { key: 'authStrategy', role: 'the key production passes' }, + { key: 'strategy', role: 'the deprecated alias' }, +] as const - await expect( - newClient({ - clientOpts: { clientId: CLIENT_ID }, - strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow( - /clientOpts\.clientId and clientOpts\.clientKey are required/, - ) +for (const { key, role } of STRATEGY_KEYS) { + describe(`protect-ffi WASM core: credential contract under OIDC federation, on \`opts.${key}\` (${role}) (#804)`, () => { + it('requires `clientKey` even when an auth strategy is supplied', async () => { + const { calls, strategy } = federationStrategy() - // Rejected while the core builds its key provider — the strategy was - // never INVOKED, so federation cannot substitute for the key. (The core - // does look at `opts.strategy` before this point, to check it is present - // and carries a `getToken`; what never happens is the call.) - // - // Both credential fields are named in one message: the core does not - // report which of the pair is missing, it requires both. So this test and - // the next assert the same string, and each is carried by the field it - // omits rather than by a distinct error. - expect(calls.getToken).toBe(0) - }) + await expect( + newClient({ + clientOpts: { clientId: CLIENT_ID }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) - it('requires `clientId` even when an auth strategy is supplied', async () => { - const { calls, strategy } = federationStrategy() + // Rejected while the core builds its key provider — the strategy was + // never INVOKED, so federation cannot substitute for the key. (The core + // does look at the strategy option before this point, to check it is + // present and carries a `getToken`; what never happens is the call.) + // + // Both credential fields are named in one message: the core does not + // report which of the pair is missing, it requires both. So this test + // and the next assert the same string, and each is carried by the field + // it omits rather than by a distinct error. + expect(calls.getToken).toBe(0) + }) - await expect( - newClient({ - clientOpts: { clientKey: HEX_BUT_NOT_KEY_MATERIAL }, - strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow( - /clientOpts\.clientId and clientOpts\.clientKey are required/, - ) + it('requires `clientId` even when an auth strategy is supplied', async () => { + const { calls, strategy } = federationStrategy() - expect(calls.getToken).toBe(0) - }) + await expect( + newClient({ + clientOpts: { clientKey: HEX_BUT_NOT_KEY_MATERIAL }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) - it('decodes `clientKey` in two stages — hex, then a key provider', async () => { - // Two distinct error classes prove two distinct stages. A field that were - // merely format-checked would have only the first. - const hexStage = federationStrategy() - await expect( - newClient({ - clientOpts: { clientId: CLIENT_ID, clientKey: 'not-hex' }, - strategy: hexStage.strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow(/invalid clientKey: expected a hex-encoded key/) + expect(calls.getToken).toBe(0) + }) - const providerStage = federationStrategy() - await expect( - newClient({ - clientOpts: { - clientId: CLIENT_ID, - clientKey: HEX_BUT_NOT_KEY_MATERIAL, - }, - strategy: providerStage.strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow(/Key provider error: Invalid client key/) + it('decodes `clientKey` in two stages — hex, then a key provider', async () => { + // Two distinct error classes prove two distinct stages. A field that + // were merely format-checked would have only the first. + const hexStage = federationStrategy() + await expect( + newClient({ + clientOpts: { clientId: CLIENT_ID, clientKey: 'not-hex' }, + [key]: hexStage.strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/invalid clientKey: expected a hex-encoded key/) - // Neither stage invoked the strategy: key loading strictly precedes auth. - // The last test in this file supplies the other half of that claim, by - // getting past key loading and watching `getToken` fire. - expect(hexStage.calls.getToken).toBe(0) - expect(providerStage.calls.getToken).toBe(0) - }) + const providerStage = federationStrategy() + await expect( + newClient({ + clientOpts: { + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + }, + [key]: providerStage.strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/Key provider error: Invalid client key/) - it('decodes `clientKey` into cryptographic key material, not an identifier', async () => { - const { calls, strategy } = federationStrategy() + // Neither stage invoked the strategy: key loading strictly precedes + // auth. The last test in this arm supplies the other half of that + // claim, by getting past key loading and watching `getToken` fire. + expect(hexStage.calls.getToken).toBe(0) + expect(providerStage.calls.getToken).toBe(0) + }) - // The load-bearing assertion, and the one that separates "the core parses - // this field" from "the core uses this field as a key". Reaching into the - // serialised struct, the core reports that `p1` must be a `Permutation` — - // a keyed permutation, i.e. cryptographic material for the searchable-index - // schemes. Nothing that merely validated a credential's format would decode - // a permutation out of it. That is what makes `clientKey` a secret, and - // therefore what blocks this entry from a browser bundle. - // - // This asserts on the core's internal key layout deliberately. If protect-ffi - // changes it this test fails, and that is the intended prompt to re-read - // #804 rather than to loosen the assertion. - await expect( - newClient({ - clientOpts: { clientId: CLIENT_ID, clientKey: CBOR_KEY_WITH_BAD_P1 }, - strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow(/expected struct Permutation/) + it('decodes `clientKey` into cryptographic key material, not an identifier', async () => { + const { calls, strategy } = federationStrategy() - expect(calls.getToken).toBe(0) + // The load-bearing assertion, and the one that separates "the core + // parses this field" from "the core uses this field as a key". Reaching + // into the serialised struct, the core reports that `p1` must be a + // `Permutation` — a keyed permutation, i.e. cryptographic material for + // the searchable-index schemes. Nothing that merely validated a + // credential's format would decode a permutation out of it. That is + // what makes `clientKey` a secret, and therefore what blocks this entry + // from a browser bundle. + // + // This asserts on the core's internal key layout deliberately. If + // protect-ffi changes it this test fails, and that is the intended + // prompt to re-read #804 rather than to loosen the assertion. + await expect( + newClient({ + clientOpts: { clientId: CLIENT_ID, clientKey: CBOR_KEY_WITH_BAD_P1 }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/expected struct Permutation/) + + expect(calls.getToken).toBe(0) + }) + + it('invokes `getToken` only after key loading succeeds', async () => { + const { calls, strategy } = federationStrategy() + + // The positive control for every `toBe(0)` above. Without it those + // assertions could not tell "auth comes after key loading" apart from + // "auth never happens during `newClient` at all" — a counter that is + // never incremented reads as 0 either way. + // + // Structurally complete key material clears the key provider, and the + // core then calls `getToken` exactly once, failing on the deliberately + // malformed token this stand-in returns. So: auth IS reached during + // construction, it is reached only after the key is loaded, and the + // counter these tests rely on is live. + await expect( + newClient({ + clientOpts: { + clientId: CLIENT_ID, + clientKey: WELL_FORMED_KEY_MATERIAL, + }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/Invalid token: JWT must have three segments/) + + expect(calls.getToken).toBe(1) + }) }) +} - it('reads the strategy off `opts.strategy`, before it deserialises the credentials', async () => { - // Fixes the meaning of every test above. They all assert "even when an +describe('protect-ffi WASM core: the strategy is read before the credentials (#804)', () => { + it('answers `Not authenticated` when NEITHER key carries a strategy', async () => { + // Fixes the meaning of every arm above. They all assert "even when an // auth strategy is supplied" — which is only worth anything if the core - // reads the field they supply it on. It does: with `strategy` omitted the - // core answers `Not authenticated` whether or not credentials are - // present, while credentials omitted WITH a strategy gives the credential - // error — so the strategy is seen first. Verified by probing all three - // combinations against the real core, not inferred from the source. + // reads the key they supply it on. It does, and this is the control that + // says so: with the strategy omitted entirely the core answers `Not + // authenticated` whether or not credentials are present, while + // credentials omitted WITH a strategy gives the credential error — so the + // strategy is seen first. Verified by probing all three combinations + // against the real core, not inferred from the source. // - // This is also the drift guard. If protect-ffi renamed the option, the - // tests above would be handing the core nothing and quietly testing the - // no-strategy path instead. They would fail rather than pass silently - // (`Not authenticated` matches none of their regexes), and this test - // names the reason. + // This is also the drift guard, and it now guards a pair. `authStrategy` + // is the field the core reads first and the field production passes; + // `strategy` is the fallback it reads only when `authStrategy` is absent. + // If protect-ffi renamed or removed either, that arm would be handing the + // core nothing and quietly exercising THIS path instead — it would fail + // rather than pass silently, because `Not authenticated` matches none of + // its regexes, and this test names the string it would fail with. await expect( newClient({ clientOpts: { @@ -255,54 +319,13 @@ describe('protect-ffi WASM core: credential contract under OIDC federation (#804 }), ).rejects.toThrow(/Not authenticated/) }) - - it('invokes `getToken` only after key loading succeeds', async () => { - const { calls, strategy } = federationStrategy() - - // The positive control for every `toBe(0)` above. Without it those - // assertions could not tell "auth comes after key loading" apart from - // "auth never happens during `newClient` at all" — a counter that is - // never incremented reads as 0 either way. - // - // Structurally complete key material clears the key provider, and the - // core then calls `getToken` exactly once, failing on the deliberately - // malformed token this stand-in returns. So: auth IS reached during - // construction, it is reached only after the key is loaded, and the - // counter these tests rely on is live. - await expect( - newClient({ - clientOpts: { - clientId: CLIENT_ID, - clientKey: WELL_FORMED_KEY_MATERIAL, - }, - strategy, - encryptConfig, - eqlVersion: 3, - }), - ).rejects.toThrow(/Invalid token: JWT must have three segments/) - - expect(calls.getToken).toBe(1) - }) }) -describe('@cipherstash/stack declares no browser build (#804)', () => { - it('has no `browser` export condition on any subpath', () => { - // The consequence of everything above, and the one part of it a reader - // can act on by accident. `src/wasm-inline.ts` tells callers there is no - // `browser` condition and explains why; nothing enforced that, so adding - // one to fix a bundler complaint would ship a workspace secret to the - // browser and leave the doc silently wrong. - // - // Same rule as the rest of this file: if the core stops requiring - // `clientKey`, come back through #804 — don't just delete this. - const packageJson = JSON.parse( - readFileSync( - path.resolve(fileURLToPath(import.meta.url), '../../package.json'), - 'utf8', - ), - ) as { browser?: unknown; exports: Record } - - expect(packageJson.browser).toBeUndefined() - expect(JSON.stringify(packageJson.exports)).not.toContain('"browser"') - }) -}) +// The packaging consequence of everything above — that neither +// `@cipherstash/stack` nor `@cipherstash/stack-supabase` declares a `browser` +// export condition — used to be asserted here. It has moved to +// `__tests__/browser-export-condition.test.ts` (and its sibling in +// `packages/stack-supabase/__tests__/`), because it only reads a manifest: +// hosted in this file it inherited this file's exclusion from the default +// config and so ran in one CI job and in nobody's local test run. Anyone +// following #804 wants both files. diff --git a/packages/stack/vitest.wasm-core.config.ts b/packages/stack/vitest.wasm-core.config.ts index b6f7a0146..aca71fad4 100644 --- a/packages/stack/vitest.wasm-core.config.ts +++ b/packages/stack/vitest.wasm-core.config.ts @@ -58,5 +58,18 @@ export default defineConfig({ // fail rather than report a green run of zero files — the one failure mode // a single-file `include` invites. passWithNoTests: false, + // Not left at vitest's 5000ms, for the same reason the sibling + // `vitest.config.ts` does not leave it there: 5000ms was intermittently + // short for this package. Nothing here talks to the network — every + // assertion lands before the first ZeroKMS / CTS call — but every case + // instantiates the REAL inlined core, and the last one loads a key and + // runs `getToken`. A separate config inherits none of the sibling's + // settings, so this has to be said twice; 30s matches it, and is still + // low enough to surface a genuine hang. + testTimeout: 30000, + // Raised with it rather than considered separately: this suite has no + // hooks today, and the failure mode of a config whose two timeouts + // disagree is that adding one later inherits the number nobody chose. + hookTimeout: 30000, }, }) diff --git a/scripts/__tests__/wasm-core-contract-ci.test.mjs b/scripts/__tests__/wasm-core-contract-ci.test.mjs index 6e86bf278..0a22a7c69 100644 --- a/scripts/__tests__/wasm-core-contract-ci.test.mjs +++ b/scripts/__tests__/wasm-core-contract-ci.test.mjs @@ -5,6 +5,7 @@ import stackVitestConfig from '../../packages/stack/vitest.config.ts' import wasmCoreVitestConfig, { WASM_CORE_SUITE, } from '../../packages/stack/vitest.wasm-core.config.ts' +import supabaseVitestConfig from '../../packages/stack-supabase/vitest.config.ts' import { readJsonc } from './lib/read-jsonc.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' @@ -54,12 +55,34 @@ import { readWorkflow, workflowFiles } from './lib/workflows.mjs' * future job that also builds the WASM output can host the step without * editing this file — and naming the wrong job cannot pass, since a job that * does not build wasm cannot run the suite at all. + * + * Two further checks live here, both about what the arrangement COSTS rather + * than about where the suite runs: + * + * 5. the WASM-core config raises Vitest's 5000ms default timeout. That + * default was already intermittently too short for this package — the + * sibling `vitest.config.ts` says so and raises it — and every case in + * the WASM-core suite instantiates the real inlined core. + * 6. the `browser` export-condition guard sits in a file the DEFAULT stack + * suite collects. It reads a manifest and needs no WASM build, so it has + * no business paying this arrangement's price: hosted in the WASM-core + * file it ran in exactly one CI job and in nobody's local `pnpm test`. */ const BUILD_FFI = './.github/actions/build-ffi-binding' const SCRIPT = 'test:wasm-core' const PACKAGE = '@cipherstash/stack' +/** Vitest's own default, which both stack configs deliberately exceed. */ +const VITEST_DEFAULT_TIMEOUT_MS = 5000 + +/** + * Where the `browser` export-condition guards live, one per package that + * ships a `wasm-inline` entry (#804). Relative to each package root, because + * that is what a vitest `exclude` pattern is relative to. + */ +const BROWSER_GUARD_SUITE = '__tests__/browser-export-condition.test.ts' + const stackPackageJson = JSON.parse( readFileSync(join(REPO_ROOT, 'packages/stack/package.json'), 'utf8'), ) @@ -88,6 +111,29 @@ function buildsWasm(job) { ) } +/** + * Which of a vitest config's `exclude` patterns select `relPath`. + * + * Deliberately not a glob engine. The patterns in play are literal paths, a + * literal directory prefix (`integration/**`) and vitest's own + * anywhere-under-a-directory defaults (node_modules, dist, cypress), and + * those three shapes are what this handles. A real matcher would be a + * dependency for no extra coverage, and a pattern shape it does not + * understand is reported as "not excluded" — the safe direction here only + * because the exclusion this guards against is written by hand, in one of + * those shapes, by someone moving the file back. + */ +function excludedBy(patterns, relPath) { + const segments = relPath.split('/') + return (patterns ?? []).filter((pattern) => { + if (pattern === relPath) return true + const literal = pattern.split('*')[0].replace(/\/$/, '') + if (literal && relPath.startsWith(`${literal}/`)) return true + const anywhere = /^\*\*\/([^*/]+)\/\*\*$/.exec(pattern) + return anywhere ? segments.includes(anywhere[1]) : false + }) +} + describe('the WASM core credential contract runs somewhere (#804)', () => { it('the suite the whole arrangement is about still exists', () => { expect(existsSync(join(REPO_ROOT, 'packages/stack', WASM_CORE_SUITE))).toBe( @@ -106,6 +152,22 @@ describe('the WASM core credential contract runs somewhere (#804)', () => { expect(stackVitestConfig.test.exclude).toContain(WASM_CORE_SUITE) }) + it("gives each case more than vitest's 5s default to run in", () => { + // A separate config inherits none of the sibling's settings, and the one + // most easily lost is the one nothing references: `vitest.config.ts` + // raises this exact default for this exact package, because 5000ms was + // intermittently short there. Every case in the WASM-core suite + // instantiates the real inlined core — the last one gets through key + // loading into `getToken` — so the same risk applies, and it would + // present as a flake in the one job that runs it rather than as a + // failure anyone can reproduce. + expect( + wasmCoreVitestConfig.test.testTimeout, + `vitest.wasm-core.config.ts must set an explicit \`testTimeout\` above vitest's ${VITEST_DEFAULT_TIMEOUT_MS}ms default.\n` + + `Every case there instantiates the REAL inlined WASM core, and the sibling vitest.config.ts already raises this default for this package because ${VITEST_DEFAULT_TIMEOUT_MS}ms was intermittently flaky.`, + ).toBeGreaterThan(VITEST_DEFAULT_TIMEOUT_MS) + }) + it(`is invoked by \`${SCRIPT}\`, pointed at that config`, () => { const command = stackPackageJson.scripts?.[SCRIPT] expect(command, `packages/stack has no \`${SCRIPT}\` script`).toBeDefined() @@ -162,3 +224,39 @@ describe('the WASM core credential contract runs somewhere (#804)', () => { } }) }) + +describe('the `browser` export-condition guard runs everywhere (#804)', () => { + // The guard asserts that neither package declares a `browser` export + // condition, because `wasm-inline` needs a `clientKey` — a workspace secret + // — on every auth path. It is a manifest read: no WASM build, no + // credentials, no database. + // + // It lived in the WASM-core contract file, which the default stack config + // excludes, so it ran ONLY in `tests.yml`'s `wasm-e2e-tests` job — the one + // job that builds WASM output this assertion does not need. It ran in no + // local `pnpm test`, and on a fork PR in nothing at all (`wasm-e2e-tests` + // and `run-tests` both hard-fail at `require-cs-secrets` there, and `lint` + // runs only Biome). Moving it into each package's default suite is what + // these two cases hold in place. + const guards = [ + { pkg: 'packages/stack', config: stackVitestConfig }, + { pkg: 'packages/stack-supabase', config: supabaseVitestConfig }, + ] + + for (const { pkg, config } of guards) { + it(`${pkg} keeps its guard in the default suite`, () => { + const relative = BROWSER_GUARD_SUITE + expect( + existsSync(join(REPO_ROOT, pkg, relative)), + `${pkg}/${relative} is missing.\n` + + `That file is the only thing stopping a \`browser\` export condition being added to quiet a bundler, which would ship a workspace secret to the browser. If it moved, move this expectation with it — and keep it somewhere \`pnpm --filter ${pkg.replace('packages/', '@cipherstash/')} test\` collects.`, + ).toBe(true) + + expect( + excludedBy(config.test.exclude, relative), + `${pkg}/${relative} is excluded from that package's default vitest config.\n` + + `Excluded, it runs only where something invokes a second config — which is exactly the arrangement that kept it out of every local test run and every fork PR before #804.`, + ).toEqual([]) + }) + } +}) From 9038aa1dece7f964753d58d33d940de209cfcbe9 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 31 Aug 2026 11:41:31 +1000 Subject: [PATCH 7/8] docs: say the entry is server-side where deployers read it, and stop overstating the diff (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The changeset claimed the constraint was now stated in "the auth-strategy re-export, the `stash-encryption` entry-point table, the `stash-edge` and `stash-supabase` skills". Only `stash-supabase` was touched; the re-export is unchanged, and neither of the other two was in the diff. That text ships verbatim into `@cipherstash/stack`'s CHANGELOG. Rather than only trimming it, land the piece that was worth landing. `skills/stash-edge` is what a customer's agent reads when deploying the WASM entry and it did not contain the word "browser" at all, so it now carries the constraint at the head of its Credentials section: `clientKey` is a workspace secret required on every auth path including `authStrategy`, because the core loads it as key material before it ever calls the strategy. The `stash-encryption` entry-point row gains a clause pointing there — that row calls this "the **edge** entry" and lists V8 runtimes, which is the reading that put "modern browsers" in the example README in the first place. The changeset now names only what landed, and narrows "had all described this entry as browser-capable" to the two places that actually did. `turbo.json`'s `test:wasm-core` comment did not say that `^build` cannot make the task runnable: `@cipherstash/protect-ffi#build` is `tsc` declaring `outputs: ["lib/**"]`, and the input the suite loads comes from `build:wasm`, which is a package.json script and no turbo task at all. Left as a comment rather than a dependency on purpose — declaring the wasm build would put a Rust toolchain on the path of everyone who runs the task, which is what protect-ffi's script split exists to prevent. Claude-Session: https://claude.ai/code/session_01THGKcgRdyd2aPt5zBnPVLH --- .../wasm-inline-client-key-not-browser-safe.md | 12 ++++++------ skills/stash-edge/SKILL.md | 12 +++++++++++- skills/stash-encryption/SKILL.md | 2 +- turbo.json | 18 ++++++++++++++++++ 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/.changeset/wasm-inline-client-key-not-browser-safe.md b/.changeset/wasm-inline-client-key-not-browser-safe.md index b5faebbce..995bc5937 100644 --- a/.changeset/wasm-inline-client-key-not-browser-safe.md +++ b/.changeset/wasm-inline-client-key-not-browser-safe.md @@ -17,9 +17,9 @@ which is why this entry has no `browser` export condition, and will not get one until the core changes. No behaviour change. The types and runtime are unchanged; what changes is that -the constraint is now stated where callers meet it (`WasmClientConfig`, the -auth-strategy re-export, the `stash-encryption` entry-point table, the -`stash-edge` and `stash-supabase` skills, and the `supabase-worker` example, -which had all described this entry as browser-capable) and enforced by contract -tests that run against the real WASM core instead of the mocks and stub the rest -of the wasm suite uses. +the constraint is now stated where callers meet it — `WasmClientConfig`, the +`stash-edge` skill, the `stash-encryption` entry-point table, and, where this +entry had been described as browser-capable, the `stash-supabase` skill and +the `supabase-worker` example — and enforced by contract tests that run +against the real WASM core instead of the mocks and stubs the rest of the wasm +suite uses. diff --git a/skills/stash-edge/SKILL.md b/skills/stash-edge/SKILL.md index 6964af2a1..4df34ad04 100644 --- a/skills/stash-edge/SKILL.md +++ b/skills/stash-edge/SKILL.md @@ -1,6 +1,6 @@ --- name: stash-edge -description: Run CipherStash encryption on edge and non-Node runtimes with the `@cipherstash/stack/wasm-inline` entry — Deno, Supabase Edge Functions, Cloudflare Workers, and Bun. Covers the import specifier per runtime, the four mandatory `CS_*` variables and minting them with `stash env`, how keysets and credentials interact on the edge (what must match is the keyset — `stash-zerokms` is canonical), how the WASM client surface differs from the native typed client, and why an EQL v3 schema module cannot be shared across the two entries. Use when adding encryption to a Supabase Edge Function, a Worker, or a Deno service; when a native module fails to load in a deployed runtime; when wiring `CS_*` secrets into an edge deploy; or when encrypted search returns zero rows on the edge but works locally. +description: Run CipherStash encryption on edge and non-Node runtimes with the `@cipherstash/stack/wasm-inline` entry — Deno, Supabase Edge Functions, Cloudflare Workers, and Bun. Covers the import specifier per runtime, the four mandatory `CS_*` variables and minting them with `stash env`, how keysets and credentials interact on the edge (what must match is the keyset — `stash-zerokms` is canonical), how the WASM client surface differs from the native typed client, why the entry is server-side only and never belongs in a browser bundle, and why an EQL v3 schema module cannot be shared across the two entries. Use when adding encryption to a Supabase Edge Function, a Worker, or a Deno service; when a native module fails to load in a deployed runtime; when wiring `CS_*` secrets into an edge deploy; or when encrypted search returns zero rows on the edge but works locally. --- # Encryption on the Edge (WASM entry) @@ -122,6 +122,16 @@ The edge client takes **all four** `CS_*` values explicitly. There is no credential discovery: `~/.cipherstash` does not exist in a Worker or an Edge Function container, and there is no device-code login to fall back on. +> [!IMPORTANT] +> **Server-side only — this entry never goes in a browser bundle.** +> `clientKey` is a workspace secret, and it is required on *every* auth path, +> including `authStrategy` (OIDC federation): the core loads it as encryption +> key material *before* it ever calls the strategy, so per-user federation +> does not stand in for it. That is why there is no `browser` export +> condition, and there will not be one until the core changes +> ([#804](https://github.com/cipherstash/stack/issues/804)). Every runtime +> this entry targets is a server — Deno, a Worker, Bun — not a page. + ```ts const client = await Encryption({ schemas: [users], diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index e6ccae032..85ff50d06 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -190,7 +190,7 @@ The SDK never logs plaintext data. | `@cipherstash/stack/types` | All TypeScript types | | `@cipherstash/stack-drizzle` | Drizzle ORM integration for EQL v3 schemas — the package root, EQL v3 only (see the `stash-drizzle` skill) | | `@cipherstash/stack-supabase` | `encryptedSupabase` wrapper for Supabase — EQL v3 only (see the `stash-supabase` skill) | -| `@cipherstash/stack/wasm-inline` | The **edge** entry — Deno, Bun, Cloudflare Workers, Supabase Edge Functions. Its own `Encryption` factory plus its own copy of the v3 authoring surface, `EncryptionErrorTypes`, and the WASM build of protect-ffi inlined into the bundle. No native binding, so no bundler externalisation needed. **EQL v3 only** — `Encryption()` here rejects a v2 schema, and its operations return plain Results with no `.audit()` or `.withLockContext()` chaining, so **values written here cannot be identity-bound** and it cannot read what the native entry wrote under a lock context. **ESM-only, and its schema types do not interchange with the other entries'** — see the `stash-edge` skill. | +| `@cipherstash/stack/wasm-inline` | The **edge** entry — Deno, Bun, Cloudflare Workers, Supabase Edge Functions. Its own `Encryption` factory plus its own copy of the v3 authoring surface, `EncryptionErrorTypes`, and the WASM build of protect-ffi inlined into the bundle. No native binding, so no bundler externalisation needed. **EQL v3 only** — `Encryption()` here rejects a v2 schema, and its operations return plain Results with no `.audit()` or `.withLockContext()` chaining, so **values written here cannot be identity-bound** and it cannot read what the native entry wrote under a lock context. **ESM-only, and its schema types do not interchange with the other entries'** — see the `stash-edge` skill, which also covers why it is **server-side only**: `clientKey` is a workspace secret required on every auth path, so this entry never belongs in a browser bundle. | | `@cipherstash/stack/dynamodb` | `encryptedDynamoDB` — encrypt/write is **EQL v3 only** (`types.*`); decrypt still reads existing v2 items via `{ storedEqlVersion: 2 }`, on both the native and `wasm-inline` entries. See the `stash-dynamodb` skill | | `@cipherstash/stack/schema` | Low-level encrypt-config types and validation helpers; it is not a schema-authoring DSL | | `@cipherstash/stack/encryption` | The `Encryption` factory and the chainable operation classes its methods return (`EncryptOperation`, `DecryptOperation`, `EncryptQueryOperation`, `BulkEncryptModelsOperation`, …). Import these only to *name* an operation's type; author schemas and build the client from `@cipherstash/stack/v3` | diff --git a/turbo.json b/turbo.json index 9c97b7d58..a2448e541 100644 --- a/turbo.json +++ b/turbo.json @@ -98,6 +98,24 @@ // stack depends on workspace packages that emit build output, so a bare // invocation resolves `dist/` that nothing in the command builds. // + // `^build` does not make this task runnable, and that is deliberate. It + // builds the workspace JS this package resolves — but not the input the + // suite actually loads, which is protect-ffi's gitignored + // `dist/wasm/protect_ffi_inline.js`. Nothing on the `build` path emits + // it: `@cipherstash/protect-ffi#build` is `tsc` declaring + // `outputs: ["lib/**"]`, and the wasm output comes from that package's + // `build:wasm` script (cargo + wasm-pack), which is a package.json script + // and not a turbo task at all. CI supplies it in the one job that runs + // this task, through `./.github/actions/build-ffi-binding` with + // `wasm: 'true'`; locally you run `build:wasm` by hand first. Anywhere + // else `turbo run test:wasm-core` fails at collect with `Cannot find + // module`. Declaring the wasm build as a dependency here would fix that + // by putting a Rust toolchain on the path of everyone who runs the task, + // which is exactly what protect-ffi's script split exists to prevent (see + // AGENTS.md, "Working on protect-ffi": the default `test` and `build` + // never invoke cargo). So this task being runnable in one job only is the + // chosen trade, not an oversight to repair. + // // `cache: false` is the load-bearing half. The input this suite actually // reads — protect-ffi's `dist/wasm/protect_ffi_inline.js` — is a gitignored // build output in ANOTHER package, so nothing turbo hashes can see it: with From 56864394a52aae740033e44291d96791754f5934 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 31 Aug 2026 11:54:05 +1000 Subject: [PATCH 8/8] docs(stack): pin the fact the WASM-core arrangement argues from (#804) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review argued the separate config, turbo task and CI guard are redundant, because `integration/wasm/**` already loads the real core and `integration-drizzle.yml` already builds `dist/wasm/**` with `wasm: 'true'`. Both halves of that are true. The argument still fails, on one fact: `packages/test-kit/src/integration/global-setup.ts` requires `cipherstash` AND `database` unconditionally and throws rather than skipping, then runs a real `stash eql install`. Moving the contract test there would cost it credentials, a database and a built CLI — everything it currently needs none of. That fact lived in prose in three docblocks and nothing enforced it. Making global-setup skip when unconfigured is the obvious "let people run integration tests locally" change, and it would make the redundancy argument correct while every docblock still said otherwise. One case in `wasm-core-contract-ci.test.mjs` now pins both halves — the unconditional requirement list, and that it is a throw and not a skip. Three sentences were also wrong and are corrected. The contract file said every suite that reaches the core hands it a complete real credential; protect-ffi's `wasm-error-codes` does not — it passes no `clientOpts` at all, and misses #804 for a different reason (its cases fail in config validation, and none supplies an auth strategy). The guard's docblock made the same "only one that needs nothing else" claim. The CI step comment claimed the integration job has PostgREST, which `integration-db` documents as empty for the `postgres` variant, and said "most diffs" where the condition is every diff the path filter does not select. `vitest.wasm-core.config.ts` opened by calling this "the one suite in stack's DEFAULT test run", twenty lines above explaining that it is excluded from that run. It is the one suite in the package that loads the core outside the integration harness. Claude-Session: https://claude.ai/code/session_01THGKcgRdyd2aPt5zBnPVLH --- .github/workflows/tests.yml | 9 ++-- ...sm-inline-core-credential-contract.test.ts | 16 +++--- packages/stack/vitest.wasm-core.config.ts | 14 +++-- .../__tests__/wasm-core-contract-ci.test.mjs | 54 +++++++++++++++++-- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 2b02787f0..9dfd8cca3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -511,10 +511,11 @@ jobs: # Not the only suite that loads the real core, and this is not the only # job that builds it for one — `integration-drizzle.yml` runs # `packages/stack`'s `integration/wasm/**` the same way. It is here - # because it is the only one of them that needs NOTHING else: no - # credentials, no database, no PostgREST. The integration job has all - # three and is path-filtered and fork-skipped besides, so hosting a core - # contract there would leave it unchecked on most diffs. See + # because it needs NOTHING else: no credentials, no database. Those + # suites' `globalSetup` requires both unconditionally and throws rather + # than skipping, and that job is path-filtered and fork-skipped besides, + # so hosting a core contract there would leave it unchecked on every diff + # those paths do not select. See # `packages/stack/vitest.wasm-core.config.ts` for the long form. # # Before `Build stack` deliberately — it reads protect-ffi's output diff --git a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts index 6690b9655..ec3693e01 100644 --- a/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -20,13 +20,15 @@ * and `vitest.shared.ts` aliases the whole * `@cipherstash/protect-ffi/wasm-inline` specifier to a stub whose `newClient` * throws — so none of them reaches the core at all. The suites that DO reach - * it — `integration/wasm/**` here, protect-ffi's own `wasm-round-trip` and - * `wasm-error-codes`, the Deno smoke tests in `e2e/wasm/` — all hand it a - * complete, real credential, because their point is a round trip. A - * requirement is invisible to a caller that always satisfies it. This file is - * the one that OMITS the credential, so it resolves the REAL module through - * Node — which the Vite alias does not intercept — and asserts against the - * actual core. + * it miss this contract from both sides. The round-trip ones — + * `integration/wasm/**` here, protect-ffi's own `wasm-round-trip`, the Deno + * smoke tests in `e2e/wasm/` — hand it a complete, real credential, and a + * requirement is invisible to a caller that always satisfies it. protect-ffi's + * `wasm-error-codes` passes no `clientOpts` at all, but every case there fails + * in config validation, before the credential check, and none supplies an auth + * strategy. This file is the one that omits the credential WHILE supplying a + * strategy, so it resolves the REAL module through Node — which the Vite alias + * does not intercept — and asserts against the actual core. * * IF THIS TEST FAILS, THAT IS GOOD NEWS — with one exception, named below. It * means the core relaxed the requirement and browser support should be diff --git a/packages/stack/vitest.wasm-core.config.ts b/packages/stack/vitest.wasm-core.config.ts index aca71fad4..92eb96bed 100644 --- a/packages/stack/vitest.wasm-core.config.ts +++ b/packages/stack/vitest.wasm-core.config.ts @@ -1,11 +1,15 @@ import { defineConfig } from 'vitest/config' /** - * The one suite in stack's DEFAULT test run that loads the REAL protect-ffi - * WASM core — NOT the only one in the repo that loads it. `integration/wasm/**` - * does (its config restores the genuine module over the stub alias), as do - * protect-ffi's own `wasm-round-trip` / `wasm-error-codes` suites and the Deno - * smoke tests in `e2e/wasm/`. Three CI jobs build `dist/wasm/**` for them. + * The one suite in `packages/stack` that loads the REAL protect-ffi WASM core + * outside the integration harness. It is in neither of the package's other two + * configs — excluded from `vitest.config.ts` (see WHERE IT RUNS below) and not + * joined to `integration/vitest.config.ts` — which is what this third one is + * for. And it is NOT the only suite in the repo that loads the core: + * `integration/wasm/**` does (its config restores the genuine module over the + * stub alias), as do protect-ffi's own `wasm-round-trip` / `wasm-error-codes` + * suites and the Deno smoke tests in `e2e/wasm/`. Three CI jobs build + * `dist/wasm/**` for them. * * What is unusual here is the COMBINATION: this file needs the real core and * nothing else — no credentials, no database, no PostgREST — because every diff --git a/scripts/__tests__/wasm-core-contract-ci.test.mjs b/scripts/__tests__/wasm-core-contract-ci.test.mjs index 0a22a7c69..6435cce05 100644 --- a/scripts/__tests__/wasm-core-contract-ci.test.mjs +++ b/scripts/__tests__/wasm-core-contract-ci.test.mjs @@ -6,6 +6,7 @@ import wasmCoreVitestConfig, { WASM_CORE_SUITE, } from '../../packages/stack/vitest.wasm-core.config.ts' import supabaseVitestConfig from '../../packages/stack-supabase/vitest.config.ts' +import { requireIntegrationEnv } from '../../packages/test-kit/src/env.ts' import { readJsonc } from './lib/read-jsonc.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow, workflowFiles } from './lib/workflows.mjs' @@ -15,11 +16,17 @@ import { readWorkflow, workflowFiles } from './lib/workflows.mjs' * the REAL protect-ffi WASM core. It is NOT the only suite that does — * `packages/stack/integration/wasm/**`, protect-ffi's own `wasm-round-trip` / * `wasm-error-codes`, and the Deno smoke tests in `e2e/wasm/` all do, and three - * CI jobs build `dist/wasm/**` for them. It is the only one that needs the core - * and NOTHING ELSE — no credentials, no database — which is what put it in its - * own config rather than into the integration suites, whose `globalSetup` - * requires both unconditionally. This docblock claimed the stronger thing until - * review caught it; the arrangement below never depended on it. + * CI jobs build `dist/wasm/**` for them. Nor is it the only one that needs the + * core and NOTHING ELSE — protect-ffi's `wasm-error-codes` is deliberately + * credential-free too, but it lives in `integration-tests/`, whose other files + * need Docker and credentials and whose workflow is path-filtered and + * fork-skipped: the fate this arrangement declines. What put THIS suite in its + * own config is that its alternative, `packages/stack/integration/**`, has a + * `globalSetup` requiring credentials AND a database unconditionally, throwing + * rather than skipping — pinned by the last case in the first block below, + * because until then that fact lived in prose alone. This docblock claimed the + * stronger thing until review caught it; the arrangement below never depended + * on it. * * What the arrangement does depend on is FOUR pieces, and losing any one leaves * the contract unchecked: @@ -223,6 +230,43 @@ describe('the WASM core credential contract runs somewhere (#804)', () => { ).toBe(true) } }) + + it('the integration harness it declined to join still refuses to run unconfigured', () => { + // The one fact the whole arrangement rests on, and the only one of them + // that lived in prose alone. If `globalSetup` ever became conditional — + // skip when unconfigured, the obvious "make integration tests easier to + // run locally" change — then `integration/wasm/**` WOULD host this + // contract, every docblock explaining why it does not would be quietly + // wrong, and nothing would say so. The reviewer who next proposes the + // move should find this red rather than find prose. + const globalSetup = readFileSync( + join(REPO_ROOT, 'packages/test-kit/src/integration/global-setup.ts'), + 'utf8', + ) + // Both requirements in the UNCONDITIONAL base literal. `pgrest` is the one + // pushed under an `if`, and that asymmetry is exactly the claim. + expect( + globalSetup, + 'packages/test-kit/src/integration/global-setup.ts no longer requires BOTH `cipherstash` and `database` unconditionally.\n' + + `If that is deliberate, ${WASM_CORE_SUITE} can move into packages/stack/integration/wasm/ and this whole arrangement (config, script, turbo task, CI step) can go with it — see the docblocks in vitest.wasm-core.config.ts and tests.yml, which argue from this fact.`, + ).toMatch( + /const requirements: Requirement\[\] = \[\s*'cipherstash',\s*'database',?\s*\]/, + ) + + // ...and it is enforced by a THROW, not a skip. `DATABASE_URL` is the + // requirement with no `~/.cipherstash` fallback, so clearing it is + // deterministic on a developer machine and in CI alike. + const saved = process.env['DATABASE_URL'] + delete process.env['DATABASE_URL'] + try { + expect( + () => requireIntegrationEnv(['cipherstash', 'database']), + '`requireIntegrationEnv` no longer throws on a missing requirement. A skip here is what would let the integration suites run unconfigured, which is the premise this arrangement denies.', + ).toThrow(/Integration suite cannot run/) + } finally { + if (saved !== undefined) process.env['DATABASE_URL'] = saved + } + }) }) describe('the `browser` export-condition guard runs everywhere (#804)', () => {