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..995bc5937 --- /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 +`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/.github/workflows/tests.yml b/.github/workflows/tests.yml index 1e782fe9d..9dfd8cca3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -501,6 +501,37 @@ jobs: - name: Typecheck the generated WASM declarations run: pnpm --filter @cipherstash/protect-ffi run test:typecheck:wasm + # 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 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 + # 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/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-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__/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..ec3693e01 --- /dev/null +++ b/packages/stack/__tests__/wasm-inline-core-credential-contract.test.ts @@ -0,0 +1,333 @@ +/** + * 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 — `__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`, + * 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 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 + * 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 + * `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. + * + * 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 { createRequire } from 'node:module' +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 +// 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' } } + }, + }, + } +} + +/** + * 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 + +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() + + await expect( + newClient({ + clientOpts: { clientId: CLIENT_ID }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) + + // 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) + }) + + it('requires `clientId` even when an auth strategy is supplied', async () => { + const { calls, strategy } = federationStrategy() + + await expect( + newClient({ + clientOpts: { clientKey: HEX_BUT_NOT_KEY_MATERIAL }, + [key]: strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow( + /clientOpts\.clientId and clientOpts\.clientKey are required/, + ) + + 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({ + clientOpts: { clientId: CLIENT_ID, clientKey: 'not-hex' }, + [key]: hexStage.strategy, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/invalid clientKey: expected a hex-encoded key/) + + 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/) + + // 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) + }) + + 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({ + 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) + }) + }) +} + +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 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, 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: { + clientId: CLIENT_ID, + clientKey: HEX_BUT_NOT_KEY_MATERIAL, + }, + encryptConfig, + eqlVersion: 3, + }), + ).rejects.toThrow(/Not authenticated/) + }) +}) + +// 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/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/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/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..92eb96bed --- /dev/null +++ b/packages/stack/vitest.wasm-core.config.ts @@ -0,0 +1,79 @@ +import { defineConfig } from 'vitest/config' + +/** + * 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 + * 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 + * 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, + // 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 new file mode 100644 index 000000000..6435cce05 --- /dev/null +++ b/scripts/__tests__/wasm-core-contract-ci.test.mjs @@ -0,0 +1,306 @@ +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 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' + +/** + * `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. 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: + * + * 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 + * 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. + * + * 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'), +) + +/** `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 = [] + 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', + ) +} + +/** + * 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( + 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("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() + 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 }) => + 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) + } + }) + + 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)', () => { + // 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([]) + }) + } +}) 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/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). diff --git a/turbo.json b/turbo.json index 4cfa92f5e..a2448e541 100644 --- a/turbo.json +++ b/turbo.json @@ -90,6 +90,54 @@ "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. + // + // `^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 + // 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"], + "cache": false } } }