fix(stack,stack-supabase): edge-safe shared seam — process-free adapter-kit + structural v3 columns - #799
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 5761f6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Addresses review on #799. - ColumnMap now throws at construction when a builder in an AnyV3Table's columnBuilders fails the structural v3 probe, instead of silently omitting it. An omitted column dropped out of v3Columns and its filter operands went to PostgREST as plaintext — the exact leak this work closes, from the other direction. Regression tests prove construction throws and no PostgREST request is issued. - Harden the logger env guard against a process defined without env.
coderdan
reviewed
Jul 26, 2026
coderdan
reviewed
Jul 26, 2026
coderdan
approved these changes
Jul 26, 2026
coderdan
left a comment
coderdan
left a comment
Contributor
There was a problem hiding this comment.
Some test improvement suggestions but otherwise looks good.
freshtonic
approved these changes
Jul 26, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
Reviewed both fixes in the shared adapter seam, verifying each claim against source and running the gates myself.
Approving. Two well-scoped, well-tested defect fixes.
Verified
- Fix 1 guards the module-scope
process.envread in the core logger so@cipherstash/stack/adapter-kit(re-exported to Supabase, Drizzle, Prisma Next) imports cleanly on process-free runtimes. The edge-safety gate asserts against the real emitteddist/adapter-kit.js, not a proxy, andturbo.jsonwirestest→buildso it can't silently skip in CI. - Fix 2 replaces a cross-bundle-fragile
instanceof EncryptedV3Columngate with a structural four-method probe and fails closed on unrecognised builders — closing a silent plaintext-leak path. The design is sound:columnBuildersis typed to hold only v3 columns, and the completeness test iterating alltypes.*domains confirms every real v3 column passes the probe, so the throw can't misfire on valid tables. code:check→ 0 errors;@cipherstash/stackbuilds; fullstack-supabasesuite +test:typespass. ESM/CJS export maps intact (./adapter-kitkeeps bothimport/requirewith types;EncryptedV3Columnis still exported — only the import incolumn-map.tswas removed). Both changesets present and correctly scoped as patches.
Non-blocking nits
__tests__/helpers/process-free-realm.mjsrejects all bare specifiers — stricter than production resolution; works only because the current externals arenoExternal/bundled. Documented as intentional, but a future legitimate external would fail this gate and need a harness update rather than signalling a real regression.- PR description says the suite goes 466 → 470; the actual run reports 471. Trivial doc drift.
- No skill change needed here (public surface unchanged) — but the
stash-supabase"factory can't run in a Worker" guidance is worth a re-check in the later portable-entry plans that actually make the factory edge-capable.
7 tasks
…v3] prefix Both assertions on the unrecognised-builder path matched `/\[supabase v3\]/`, which 32 messages across this package satisfy — two of them thrown by `ColumnMap` itself. Neither test could tell which error it caught. Demonstrated rather than assumed: making `assertNoPropertyDbNameCollision` throw unconditionally, so the fail-closed probe is bypassed entirely, left both tests GREEN. (Deleting the probe outright does turn them red — construction then succeeds and nothing throws. It is the bypass, not the deletion, the loose matcher was blind to.) `supabase-v3-wire.test.ts` is the more serious of the two: it guards the harm — no query string emitted — not just the mechanism. Both now pin the identity clause and both interpolations. Stops short of the advisory tail, which carries six unescaped `/` and would make the matcher a syntax error rather than a stricter test. Re-running the same bypass mutant now fails both. Also corrects a misattributed citation the assertion's comment repeats: the v2 `build()`/`getName()` at `schema/index.ts:257,264` are `EncryptedField`'s. `EncryptedColumn` opens at :275, so its own are at :442,449.
…t a time `isV3ColumnLike` had no test that could distinguish a four-probe gate from a two-probe one. Every builder reaching it either satisfied all four probes (the 40 catalog domains, the wasm-authored double) or missed two at once (the v2 stub) — so no fixture was ever one member away from passing. Measured with a mutation harness: deleting any single conjunct, weakening any single `typeof` to `true`, or dropping the object/null guard entirely left the whole 471-test suite green. Nine surviving mutants of those enumerated. (The `'x' in builder` half of each conjunct is an equivalent mutant — for any non-exotic object the `typeof` half subsumes it — so those four are correctly left alive.) The new cases are each one member apart from a conforming builder, so each fails if and only if its own probe goes. All nine die, and precisely: deleting a conjunct fails exactly its two tests, weakening a `typeof` exactly its one, and the two halves of the object/null guard are split so a mutant that drops one half names which half it dropped (1 test vs 4). Also covered: prototype-borne members (the real `Encrypted*Column` classes carry all four on the prototype, so `in` must not become `Object.hasOwn`), and a real `encryptedColumn().equality()` v2 builder rather than a hand-rolled stub — the case the four-probe design exists for. Neither kills a mutant the others miss; both are kept because they turn a 335-test avalanche and an opaque `true !== false` into one precise, self-describing failure. `isV3ColumnLike` is exported to allow this. `column-map.ts` is not re-exported from `src/index.ts` and the package publishes only `.`, so the published surface is unchanged: the built `dist/index.d.ts` and `.d.cts` contain no mention of it, and both bundles' runtime exports remain exactly `encryptedSupabase, encryptedSupabaseV3`.
…fect The process-free realm harness rejects every bare specifier, which is stricter than the resolution any real runtime performs. It passes today only because everything the adapter-kit graph reaches is bundled via tsup `noExternal` — load-bearing, not incidental: the chunk adapter-kit imports carries inlined `evlog`, so without that entry it would emit a bare import. `@cipherstash/auth` and `@cipherstash/protect-ffi` stay external and are simply unreachable from that graph; if either became reachable, the gate would fail with a message reading like a self-containment defect. The header and the throw now say what the constraint rests on and what to do about it. No behaviour change — verified by pointing the harness at `dist/index.js`, whose bare `@cipherstash/auth` import produces the reworded error. `turbo.json`'s override also replaced the root `dependsOn` rather than extending it, resolving `test` to `["build"]` and dropping the inherited `["^build"]`. This is defence-in-depth rather than a live bug: ordering survives transitively via `build`'s own `^build`, as `packages/prisma-next` demonstrates with the identical override and real workspace deps. Made explicit anyway, matching how the root spells `test:e2e`; prisma-next is left alone deliberately, since the same reasoning says it is not broken either. Lastly, `logger-edge-safety.test.ts` described a `bundling-isolation.test.ts` that spawns this harness against the WASM entry as though it existed. It does not yet — reworded to future tense, and the turbo/bare-invocation skip asymmetry noted where a reader will hit it.
A v2 `EncryptedTable` is structurally identical to a v3 one — same `tableName`,
same `columnBuilders` — and only `buildColumnKeyMap()` tells them apart. So
nothing that inspected shape caught the swap, and TypeScript only helps callers
who are actually type-checking.
Two paths, two raw TypeErrors, both naming an internal method rather than the
version mismatch behind it:
- `encryptedSupabase({ schemas })` sailed past the record-key check and died in
`verifyDeclaredSchemas` as `builder.getEqlType is not a function`. This is the
reachable one — the realistic mistake is migrating from v2 and passing the old
`schemas` through.
- Constructing the query builder directly died one layer down in `ColumnMap`, on
the constructor's very first statement, as `table.buildColumnKeyMap is not a
function`. Not reachable from the factory today (`mergeDeclaredTables` rebuilds
a fresh v3 table), so this half is defence-in-depth at the seam — symmetric
with the column-level fail-closed guard already there, which cannot cover it
because it runs after the constructor has already crashed.
Both now name the table and state the fix. Each guard is load-bearing: removing
either puts its original TypeError back.
The check routes through `hasBuildColumnKeyMap` rather than a second
hand-written spelling, per that function's own doctrine — "a second hand-written
spelling of the check is how a v2 envelope eventually gets built for a v3 table
once the marker drifts". It is re-exported from `@cipherstash/stack/adapter-kit`,
the first-party adapter seam, and deliberately NOT promoted to `./types`:
deciding which wire version a table targets is adapter plumbing, not end-user
API. Verified edge-safe — `dist/adapter-kit.js` still evaluates in a
process-free realm with no bare specifiers (`OK 14 exports`).
`skills/stash-supabase` gains the error under "Legacy: EQL v2", where a
migrating reader will hit it.
coderdan
added a commit
that referenced
this pull request
Jul 28, 2026
The precondition for retrying #798 stage 4, not a consequence of it. Stage 4 was reverted because the WASM entry threw `ReferenceError: process is not defined` on import — it had begun importing `@/utils/logger`, whose `initStackLogger()` read `process.env` at module scope. The useful part is not the defect but what missed it: 1073 unit tests, the type tests and the bundle-isolation test were all green while the entry was unusable in every runtime it exists for. Nothing could have caught it. `wasm-inline-bundle-isolation.test.ts` reads import SPECIFIERS, so a global is invisible to it. The Deno e2e runs under Deno, which provides `process`. The unit suites run under Node, likewise. So this points the harness #799 built for `dist/adapter-kit.js` at `dist/wasm-inline.js` — which that test's own docblock anticipated ("the portable-entry plan will point the same harness at the WASM entry"). It matters more here than there: the shared operation layer widens this entry's transitive graph, and anything that graph reaches ships to the edge. The harness gains an allowed-externals argument. `adapter-kit` has no bare specifiers; the WASM entry has two, the `/wasm-inline` subpaths of protect-ffi and auth, which Deno and Workers resolve through an import map (and `e2e/wasm/deno.json` maps exactly those). They are STUBBED rather than loaded: the question is whether the graph evaluates without `process`, and loading the real binding adds a megabyte of unrelated behaviour and its own globals for no extra signal. Stub exports are `undefined` on purpose — module-scope code that calls into an external at import time is itself a portability defect and should fail loudly. Anything not named is still rejected, so a new external on this entry fails here too, which is the same native-leak class the isolation test guards. Verified by breaking it: injecting an unguarded module-scope `process.env` read into `wasm-inline.ts` and rebuilding makes this fail with the ReferenceError above. Passing today at 56 exports.
tobyhede
added a commit
that referenced
this pull request
Jul 28, 2026
Addresses review on #799. - ColumnMap now throws at construction when a builder in an AnyV3Table's columnBuilders fails the structural v3 probe, instead of silently omitting it. An omitted column dropped out of v3Columns and its filter operands went to PostgREST as plaintext — the exact leak this work closes, from the other direction. Regression tests prove construction throws and no PostgREST request is issued. - Harden the logger env guard against a process defined without env.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan 1 of the portable
encryptedSupabaseseries (docs/superpowers/plans/2026-07-24-edge-safe-shared-seam.md). Two independent defect fixes in the shared seam every first-party adapter's Worker / Edge build imports. Both prerequisites for the rest of the series; neither needs any of it.Base:
remove-v2(the EQL v3 integration branch this line of work targets), notmain— the INDEX notes plan 1 defers its CI gate to plan 4 and the four plans ship as one PR, so this should not land standalone againstmain.Fix 1 —
@cipherstash/stack/adapter-kitimportable without aprocessglobalThe shared core logger read
process.env.STASH_STACK_LOGunguarded while initialising at module scope, so importing adapter-kit — which re-exports that logger — threwReferenceError: process is not definedbefore any user code ran, on any runtime with noprocess(Workers, Deno isolates, Supabase Edge Functions). The read is now guarded.Not Supabase-only:
@cipherstash/stack-supabase,@cipherstash/stack-drizzleand@cipherstash/prisma-nextall value-import adapter-kit, so all three are fixed here.No behaviour change on Node — the logger is guarded, not deferred:
STASH_STACK_LOG, its values, itserrordefault, and the point at which it is configured are unchanged.New regression gate:
packages/stack/__tests__/helpers/process-free-realm.mjsevaluates an emitted ESM graph in anode:vmrealm with noprocess;logger-edge-safety.test.tsspawns it againstdist/adapter-kit.js(skips whendist/is absent).packages/stack/turbo.jsonmakestestdepend on the package's ownbuildso the gate can't silently skip in CI.Fix 2 — recognise EQL v3 columns structurally, not by class identity
ColumnMapgated onbuilder instanceof EncryptedV3Column. tsup emits that class twice (dist/adapter-kit.jsanddist/wasm-inline.jsare separate esbuild runs), so a table authored withencryptedTable/typesfrom@cipherstash/stack/wasm-inline— what the edge examples use — failed theinstanceoffor every column.v3Columnscame out empty and the adapter treated encrypted columns as plaintext: filter operands reached PostgREST in the clear, while::jsonbcasts and result decryption kept working (different path), so the failure was silent. Tables authored from@cipherstash/stack/eql/v3were never affected — same class copy the adapter imports.The check is now a structural four-method probe (
isV3ColumnLike), mirroringhasBuildColumnKeyMap— the repo's canonical answer to the same two-copies-of-a-class hazard. Four probes, not two, so a v2 column builder is still rejected.Tests
@cipherstash/stack-supabase: 466 → 489. The two fixes above added 5 (structural-recognition, v2-rejection, wire-level plaintext-leak, fail-closed-at-construction, and a deterministic full-types.*-catalog completeness guard); review feedback added 18 more — see below.@cipherstash/stack-supabase test:types: 44, no type errors.dist/adapter-kit.js.Changesets
@cipherstash/stack: patch (logger guard) + minor (newadapter-kitexport)@cipherstash/stack-supabase: patch × 2stash: patch (skill update)Review feedback addressed
Three commits on top of the original two fixes. No changeset: test-only, plus one
exporton a module-private function that does not reach the published surface, plus repo tooling.Assertions pinned to the fail-closed message (
02b1734d). Both.toThrow(/\[supabase v3\]/)matchers were prefix-only, and 32 messages in this package satisfy that prefix — two thrown byColumnMapitself. Measured rather than assumed: withassertNoPropertyDbNameCollisionthrowing unconditionally so the fail-closed probe is bypassed, both tests stayed green. They now pin the identity clause and both interpolations, and the same bypass mutant fails both. Extended tosupabase-v3-wire.test.tsas well as the flagged site — it is the one guarding the harm (no query string emitted), not just the mechanism.isV3ColumnLikeunit-tested one member at a time (d34a16b6, new__tests__/column-map-predicate.test.ts, +16 tests). No existing test could distinguish a four-probe gate from a two-probe one: every builder reaching the predicate either satisfied all four probes or missed two at once, so nothing was ever one member away from passing. A mutation harness confirmed nine surviving mutants — deleting any single conjunct, weakening any singletypeoftotrue, or dropping the object/null guard all left the 471-test suite green. All nine now die, and precisely: a conjunct deletion fails exactly its two tests, atypeofweakening exactly its one, and the guard's two halves are split so a mutant naming one half fails 1 test vs 4.The predicate is exported to allow this. Verified not to widen the published surface:
dist/index.d.tsand.d.ctscontain no mention of it, and both bundles' runtime exports remain exactlyencryptedSupabase, encryptedSupabaseV3.Edge-safety harness reworded (
f503d8d0). A new bare specifier now reads as "allowlist it here" rather than "process-global regression" — the gate passes only because everything the adapter-kit graph reaches is bundled vianoExternal, which is load-bearing (the chunk it imports carries inlinedevlog). Also madeturbo.json'sdependsOnextend rather than replace the root's, and corrected a comment describing abundling-isolation.test.tsthat does not exist yet.No skill change in those three commits — none touches a public API, the CLI surface, or a user-facing workflow (Fix 3 below does, and updates
stash-supabaseaccordingly). Thestash-supabase"factory cannot run in a Worker" guidance remains accurate throughout:introspect.tsstill opens a directpgconnection, so it stays a re-check for the later portable-entry plan.Fix 3 — diagnose an EQL v2 table instead of crashing
Found while adding the predicate tests above, fixed on request.
A v2
EncryptedTableis structurally identical to a v3 one — sametableName, samecolumnBuilders— and onlybuildColumnKeyMap()tells them apart, so nothing inspecting shape caught the swap. Two paths each died with a rawTypeErrornaming an internal method rather than the version mismatch:encryptedSupabase({ schemas })with v2 tables — the realistic mistake when migrating — sailed past the record-key check and died inverifyDeclaredSchemasasbuilder.getEqlType is not a function.ColumnMap, on the constructor's first statement, astable.buildColumnKeyMap is not a function. Not reachable from the factory (mergeDeclaredTablesrebuilds a fresh v3 table), but symmetric with the column-level fail-closed guard already there — which cannot cover it, since it runs after the constructor has already crashed.Both now name the table and state the fix, and each guard is load-bearing: removing either puts its original
TypeErrorback.The check routes through
hasBuildColumnKeyMaprather than a second hand-written spelling, per that function's own doctrine. It is re-exported from@cipherstash/stack/adapter-kit— the first-party adapter seam this PR is about — and deliberately not promoted to./types: choosing a wire version is adapter plumbing, not end-user API. Verified edge-safe:dist/adapter-kit.jsstill evaluates in a process-free realm with no bare specifiers (OK 14 exports).skills/stash-supabasegains the error under "Legacy: EQL v2", where a migrating reader will hit it.Notes for review
A prior
/code-reviewpass flagged a "duplicateconst v2" (verified false positive — one block-scoped declaration,tscclean) and a probabilisticfc.constantFromsample in the catalog test (applied — swapped to a deterministic loop overObject.keys(types)for guaranteed full-catalog coverage, dropping the lonefast-checkimport).