Conversation
🦋 Changeset detectedLatest commit: 69a1156 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 skippedDraft detected. 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 |
7 tasks
This was referenced Jul 24, 2026
tobyhede
added a commit
that referenced
this pull request
Jul 24, 2026
…tion fix: review remediation for #772 — rewriter data loss, supabase row types, shipped-doc corrections
Contributor
Code review of #780's content, retargeted hereThis review ran against One finding changed materially on re-verification — see the correction under Type surface below. Two candidates were dropped before this list as pre-existing on The headline is a cluster, not a listFindings 1–4 are all 1. Quote-parity drift makes a commented
|
tobyhede
added a commit
that referenced
this pull request
Jul 25, 2026
…icit schema Two ways a corpus could still lose ciphertext to the ADD+DROP+RENAME, both found by the #772 review. Neither is caught by the fail-closed `declared` rule, because in both the column really is declared — as plaintext, by the original CREATE TABLE. Finding 3 — chained conversions. `indexColumnDeclarations` reads CREATE TABLE, ADD COLUMN and RENAME, but never the strict matcher's own target. A corpus carrying an earlier `SET DATA TYPE eql_v2_encrypted` (a stack version that predates this sweep) left the column looking plaintext, so a later domain change dropped its ciphertext. Record the conversion as the sweep walks, in file-sorted order — not in the corpus-wide index, which has no order and would flag the legitimate first conversion too. Finding 4 — `"users"` and `"public"."users"` are the same table; Postgres resolves the unqualified name through `search_path`. drizzle-kit emits unqualified, while hand-written SQL and this sweep's own renderSafeAlter output are qualified, so a corpus mixing the two split one column across two keys: the already-encrypted guard missed, and the ALTER dropped ciphertext. Collapse the implicit schema in `columnKey`. Only that one — `"app"."users"` stays distinct, and there is no case folding, since TABLE_REF matches quoted identifiers only. Both fixes land in both copies of the rewriter. 7 tests each: the two destructive cases, the wrong-reason case finding 4 also produced (`source-unknown` for a column declared two files up), and four guards for the ways a naive fix breaks — the first conversion in a chain, a chain across different columns, a commented-out earlier ALTER, and a real non-public schema.
tobyhede
added a commit
that referenced
this pull request
Jul 25, 2026
…ls CI The rewriter lives twice — packages/wizard/src/lib and packages/cli/src/commands/db — at ~99% similarity, and neither package depends on the other. Four #772 review findings each needed the identical fix in both places; a fix landing in one still passes that package's own suite, and this code emits DROP COLUMN. Extraction is not available this cycle: packages/utils has no package.json, @cipherstash/test-kit is private and build-less, and both bin entries set skipNodeModulesBundle, so a shared workspace package would survive into dist as an unresolvable bare specifier. That leaves publishing a new package into a fixed mid-RC release group or adding noExternal to two CLI bundles — both worse than the drift they would prevent, for now. So compare instead. Everything outside an explicit `#region wizard-only` marker must match byte-for-byte modulo comments, imports and the tool name in the emitted header. Verified the guard actually fires: perturbing either copy fails with a message naming both files and pointing at the region marker.
This was referenced Jul 25, 2026
tobyhede
added a commit
that referenced
this pull request
Jul 26, 2026
The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.
freshtonic
requested changes
Jul 27, 2026
freshtonic
left a comment
freshtonic
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the umbrella branch structurally, verified against source, and ran build/lint/unit tests in an isolated worktree (commit f7b8220).
Requesting changes — but to be clear up front: this means "not ready to merge as intended," not "the completed work is wrong." The landed work is genuinely strong (green build/lint/unit tests, a correctly preserved and tested v2 read path, no dangling references to the deleted packages). The reasons it can't merge are inherent to it being a draft umbrella branch mid-stack.
Blockers
- Stated end-state unmet — v2 authoring/emission still present.
packages/stack/src/index.ts:9still re-exports the v2 DSL (encryptedColumn,encryptedField,encryptedTable), andresolveEqlVersion(encryption/index.ts:97-131) returnsundefinedfor an all-v2 schema, which protect-ffi 0.30 defaults to wire v2 — soEncryption({ schemas: [v2Table] })still emits v2 with no native-entry rejection (onlywasm-inline.tsrejects). By design (PRs 9–12 outstanding), but it blocks merge of the umbrella. - CLI still carries the entire v2 surface (
--eql-version 2,--latest, v2 SQL, v2 drizzle/supabase routing). PR 8 outstanding. - Branch is
CONFLICTING, 9 commits behind main / 57 ahead (the body's "one commit behind" is stale). Needs rebase + conflict resolution (packages/stack-drizzle/README.md) before it can merge. - Phantom changeset bump —
.changeset/remove-eql-v2-packages.mdbumps@cipherstash/nextjs: patch, butpackages/nextjsdepends only onjoseand imports no@cipherstash/*package. It would ship a nextjs CHANGELOG entry about removing three packages it never used. Drop the line.
Major
- Several unconsumed changesets (
adapter-package-split.md,eql-v3-drizzle.md, and 6 others) advertise@cipherstash/stack-drizzle/v3/@cipherstash/stack/eql/v3/drizzleas migration targets, but this PR removes the./v3subpath (a hard break). They render into the same 1.0.0-rc.5 CHANGELOG asremove-eql-v2-drizzle-root.md, producing directly contradictory guidance. Pre-existing on main, but this PR makes them false — reconcile before release.
Minor / nits
.changeset/remove-eql-v2-migrate-classifier.mdispatchbut the change makeslistEncryptedColumns/resolveEncryptedColumnsilently omit v2 columns under an unchanged signature —minorcommunicates it better (moot inside migrate's own major).- PR description is stale vs HEAD — several "Fix before merge" items are already resolved (root README swept to v3;
packages/stack/README.md:446-451now usesencryptedSupabase;drizzle-encrypted-indexes.mdno longer advertises/v3). Refresh so reviewers/CHANGELOG readers aren't chasing fixed items.
What's done right (worth calling out)
- v2 decrypt preservation is correct and tested:
integration/shared/v2-decrypt-compat.integration.test.tscase #1c (a v3-configured client that never registered the v2 table decrypts v2 ciphertext — the actual customer invariant), plus a credential-free envelope-split path (23/23). The test documents its own fragility and instructs future PRs to keep #1c alive. - No dangling references to the deleted
@cipherstash/protect/@cipherstash/schema/@cipherstash/protect-dynamodbin source/config. - Export-map integrity intact (every changed package keeps
import+require; the only removal,stack-drizzle./v3, is the intended documented hard break with sound reasoning). - Meta files kept honest per AGENTS.md (
AGENTS.md,SECURITY.md,README.md, all seven skills); thestash-cliskill still accurately documents the v2 flags the not-yet-removed CLI ships, so skill/manifest stay consistent for the current state.
Caveat: I could not run the live ZeroKMS suite (no credentials), so the #1c v3-decrypts-v2 invariant is verified by inspection only — confirm it green in CI before merge.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
Follow-up to 1d14412, from an adversarial cross-check of that commit. Blocking, and self-inflicted: `git checkout origin/main -- packages/cli/package.json` (the em-dash revert) also imported main's Dependabot bump of posthog-node to ^5.41.0, which this branch's lockfile predates. Every CI job starts with `pnpm install --frozen-lockfile`, so the PR died at step 1 before a test ran. Back to ^5.40.0, matching the lockfile and packages/wizard. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass\(\$1\)/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
Follow-up to 1d14412, from an adversarial cross-check of that commit. Blocking, and self-inflicted: `git checkout origin/main -- packages/cli/package.json` (the em-dash revert) also imported main's Dependabot bump of posthog-node to ^5.41.0, which this branch's lockfile predates. Every CI job starts with `pnpm install --frozen-lockfile`, so the PR died at step 1 before a test ran. Back to ^5.40.0, matching the lockfile and packages/wizard. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass\(\$1\)/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
fix(stack,bench): wasm-inline DynamoDB v2 reads, and a bench seed that never encrypted (#772 review findings 10, 12)
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
Follow-up to 1d14412, from an adversarial cross-check of that commit. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass\(\$1\)/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
fix(cli): mixed-table encrypt lifecycle, and a stash init scaffold that compiles (#772 review findings 6, 7)
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
…tch their own rot Follow-up to the #772 review findings, addressing what the review of that review turned up. `stash init`'s setup prompt named `createEncryptionOperators` unconditionally. That symbol is exported by `@cipherstash/stack-drizzle` alone, so a Supabase or Prisma Next project was sent after a package that is not in its dependency tree — the previous `protectOps.eq` was wrong for everyone, this was wrong for three integrations out of four. Step 5 now branches through `queryOperatorGuidance()`, following the `migrationCommands()` pattern already in the file: `ops.eq` for Drizzle, the `encryptedSupabase` wrapper's own filters for Supabase, the `eql*` column operators for Prisma Next, and `client.encryptQuery(...)` for plain Postgres — which is also pointed at `stash-encryption`, since it is the default integration and installs no integration skill, making "see the integration skill" a dangling pointer. The package-path linter reported an untracked-but-present package as "does not exist" — finding 15's false alarm pointed the other way, at a directory sitting right there on disk. The live set now unions `git ls-files --others --exclude-standard`. `--directory` is deliberately not passed: it collapses an all-ignored directory to one entry and would resurrect exactly the `dist/`-and- `node_modules/` shells the linter exists to catch (verified both ways). git failing now exits 2 with an actionable message instead of a raw ENOENT stack trace and exit 1, which was indistinguishable from a genuine lint failure, and an empty live set refuses to run rather than flagging every reference at once. The `scans scripts/ but not its fixtures` test asserted the repo was clean — byte-for-byte what the suite's first test already asserted, and passing whether or not `scripts` was in TARGETS. It now plants an offender in the scanned directory. Verified by mutation: it dies when `scripts` is dropped, as the untracked-package test dies without the union and the git-failure test dies when the exit code is flipped. The runners linter now requires each allowlist entry to exist and to still contain an unexcused `npx` literal. It immediately found a second stale entry the sibling linter structurally cannot see, since it only matches the `packages/<name>` shape: `setup-prompt.ts` has had no `npx` since its switch moved to `utils.ts`. Removed. Also corrects two claims: the test comment saying `packages/drizzle` never existed in git history (it was added speculatively by c671560, became load-bearing 31 minutes later with 9d259e6, and rotted when 413ca39 deleted the package), and the `AnyEncryptedTable` doc comment still promising v3 is "purely additive and no existing caller has to change" — true of decrypt, false of encrypt since the v2 write overloads were removed.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
…tch their own rot Follow-up to the #772 review findings, addressing what the review of that review turned up. `stash init`'s setup prompt named `createEncryptionOperators` unconditionally. That symbol is exported by `@cipherstash/stack-drizzle` alone, so a Supabase or Prisma Next project was sent after a package that is not in its dependency tree — the previous `protectOps.eq` was wrong for everyone, this was wrong for three integrations out of four. Step 5 now branches through `queryOperatorGuidance()`, following the `migrationCommands()` pattern already in the file: `ops.eq` for Drizzle, the `encryptedSupabase` wrapper's own filters for Supabase, the `eql*` column operators for Prisma Next, and `client.encryptQuery(...)` for plain Postgres — which is also pointed at `stash-encryption`, since it is the default integration and installs no integration skill, making "see the integration skill" a dangling pointer. The package-path linter reported an untracked-but-present package as "does not exist" — finding 15's false alarm pointed the other way, at a directory sitting right there on disk. The live set now unions `git ls-files --others --exclude-standard`. `--directory` is deliberately not passed: it collapses an all-ignored directory to one entry and would resurrect exactly the `dist/`-and- `node_modules/` shells the linter exists to catch (verified both ways). git failing now exits 2 with an actionable message instead of a raw ENOENT stack trace and exit 1, which was indistinguishable from a genuine lint failure, and an empty live set refuses to run rather than flagging every reference at once. The `scans scripts/ but not its fixtures` test asserted the repo was clean — byte-for-byte what the suite's first test already asserted, and passing whether or not `scripts` was in TARGETS. It now plants an offender in the scanned directory. Verified by mutation: it dies when `scripts` is dropped, as the untracked-package test dies without the union and the git-failure test dies when the exit code is flipped. The runners linter now requires each allowlist entry to exist and to still contain an unexcused `npx` literal. It immediately found a second stale entry the sibling linter structurally cannot see, since it only matches the `packages/<name>` shape: `setup-prompt.ts` has had no `npx` since its switch moved to `utils.ts`. Removed. Also corrects two claims: the test comment saying `packages/drizzle` never existed in git history (it was added speculatively by c671560, became load-bearing 31 minutes later with 9d259e6, and rotted when 413ca39 deleted the package), and the `AnyEncryptedTable` doc comment still promising v3 is "purely additive and no existing caller has to change" — true of decrypt, false of encrypt since the v2 write overloads were removed.
tobyhede
added a commit
that referenced
this pull request
Jul 27, 2026
fix(skills,stack,scripts): shipped decrypt guidance, and a sound package-path linter (#772 review findings 14, 15)
…t-dynamodb) PR 1 of the EQL v2 final removal (#707). Delete the closed v2-only dependency chain — @cipherstash/protect-dynamodb → @cipherstash/protect → @cipherstash/schema — and every reference to it. Nothing outside the three imported them (@cipherstash/stack depends only on the separate @cipherstash/protect-ffi). They are superseded by @cipherstash/stack: - @cipherstash/protect -> @cipherstash/stack - @cipherstash/schema -> @cipherstash/stack/schema - @cipherstash/protect-dynamodb -> @cipherstash/stack/dynamodb (encryptedDynamoDB) Existing EQL v2 ciphertext stays decryptable through @cipherstash/stack; this removes the v2 authoring/emission surface, not the read path. Reference cleanup (dangling refs that would break build/CI): - e2e/package.json @cipherstash/protect dep edge - root package.json build:js turbo filter - tests.yml protect/protect-dynamodb .env steps (would fail `touch` on gone dirs) and the bun-job test loop - rebuild-docs.yml trigger tag (@cipherstash/protect@* -> @cipherstash/stack@*) - integration-{drizzle,prisma-next,supabase}.yml packages/schema/** path filters - lint-no-hardcoded-runners allowlist entry - e2e package-managers BIN fixture (dead) + two stale source comments Changeset / RC housekeeping: - delete schema-stevec-standard-pin.md (only target was the deleted schema) - prune the three from pre.json initialVersions - add deletion-notice changeset on @cipherstash/stack + @cipherstash/nextjs Meta honesty: SECURITY.md package list, AGENTS.md Repository Layout, nextjs package description.
…icit schema Two ways a corpus could still lose ciphertext to the ADD+DROP+RENAME, both found by the #772 review. Neither is caught by the fail-closed `declared` rule, because in both the column really is declared — as plaintext, by the original CREATE TABLE. Finding 3 — chained conversions. `indexColumnDeclarations` reads CREATE TABLE, ADD COLUMN and RENAME, but never the strict matcher's own target. A corpus carrying an earlier `SET DATA TYPE eql_v2_encrypted` (a stack version that predates this sweep) left the column looking plaintext, so a later domain change dropped its ciphertext. Record the conversion as the sweep walks, in file-sorted order — not in the corpus-wide index, which has no order and would flag the legitimate first conversion too. Finding 4 — `"users"` and `"public"."users"` are the same table; Postgres resolves the unqualified name through `search_path`. drizzle-kit emits unqualified, while hand-written SQL and this sweep's own renderSafeAlter output are qualified, so a corpus mixing the two split one column across two keys: the already-encrypted guard missed, and the ALTER dropped ciphertext. Collapse the implicit schema in `columnKey`. Only that one — `"app"."users"` stays distinct, and there is no case folding, since TABLE_REF matches quoted identifiers only. Both fixes land in both copies of the rewriter. 7 tests each: the two destructive cases, the wrong-reason case finding 4 also produced (`source-unknown` for a column declared two files up), and four guards for the ways a naive fix breaks — the first conversion in a chain, a chain across different columns, a commented-out earlier ALTER, and a real non-public schema.
…ning for comments Quote parity is a whole-file property. Anything that makes the scanner disagree with Postgres about where a literal ENDS shifts every token after it — so a commented-out ALTER downstream reads as live and is rewritten into a live DROP COLUMN. `isInsideCommentOrString` left two such gaps. A `$$ … $$` body was untracked on the stated grounds that mis-reading one can only make us SKIP a rewrite; that holds for an unterminated literal but not for an over-terminated one, and an odd apostrophe count inside the body (`$$ don't $$` — a function body, a comment, any prose) ends a literal EARLY. And `endOfQuoted` knew only the doubled-delimiter escape, so `E'a\'b'` read as closing at the escaped quote. Skip dollar-quoted bodies whole, and give `endOfQuoted` a backslashEscapes mode selected when the opening quote is an `E''` prefix (guarded so an identifier merely ending in `e` does not trigger it). Both copies. Four tests each. The two destructive shapes, a tagged `$fn$` variant, and a guard that a live ALTER *following* a dollar-quoted body is still rewritten — that one failed before this change too, in the over-skip direction the old docblock had assumed was the only risk.
…ssing it CREATE_TABLE_RE carried a lazy `([\s\S]*?)\)\s*;` body, so it stopped at the first `);` anywhere in the file. That can sit inside a `--` comment or a string DEFAULT — `"id" text, -- pk (uuid);` is legal and unremarkable — and the body was then truncated at that point. Every column declared after it disappeared from BOTH indexes. For an encrypted column that means the already-encrypted guard never sees it. On its own that yields a wrong reason (`source-unknown` for a column declared two lines up). Composed with a `declared` record from elsewhere in the corpus — a table dropped and recreated in a later migration — the fail-closed rule is satisfied too, and the ADD+DROP+RENAME drops a column full of ciphertext. Match only `CREATE TABLE … (` and locate the closing `)` by scanning candidate `)\s*;` positions for the first one `isInsideCommentOrString` says is live. Nested parens (`numeric(10,2)`, `CHECK (x > 0)`) are not followed by `;`, so requiring the terminator still excludes them. An unclosed statement is skipped rather than indexed to end-of-file. Three tests each: the comment truncation, the DEFAULT-string truncation, and the composed case that actually destroys data.
…ls CI The rewriter lives twice — packages/wizard/src/lib and packages/cli/src/commands/db — at ~99% similarity, and neither package depends on the other. Four #772 review findings each needed the identical fix in both places; a fix landing in one still passes that package's own suite, and this code emits DROP COLUMN. Extraction is not available this cycle: packages/utils has no package.json, @cipherstash/test-kit is private and build-less, and both bin entries set skipNodeModulesBundle, so a shared workspace package would survive into dist as an unresolvable bare specifier. That leaves publishing a new package into a fixed mid-RC release group or adding noExternal to two CLI bundles — both worse than the drift they would prevent, for now. So compare instead. Everything outside an explicit `#region wizard-only` marker must match byte-for-byte modulo comments, imports and the tool name in the emitted header. Verified the guard actually fires: perturbing either copy fails with a message naming both files and pointing at the region marker.
The wizard cannot discover a project's configured drizzle-kit `out`, so it tries drizzle/, migrations/ and src/db/migrations/. Two of those are generic names — Knex, node-pg-migrate, Flyway and hand-rolled psql all use them — and this sweep emits DROP COLUMN. So a project whose out is drizzle/ but which also keeps a hand-maintained migrations/ had that second directory rewritten into ADD+DROP+RENAME, in a directory the wizard was never pointed at. #783's fail-closed rule does not help: it indexes per directory, and a real migration history declares its own columns, so the guard passes and the rewrite proceeds. Worse, the emitted `--> statement-breakpoint` is a valid SQL line comment, so the mangled file stays runnable by whichever tool actually owns it. Gate on meta/_journal.json, which drizzle-kit writes on first generate and maintains after, and which none of the others produce. A directory holding .sql files but no journal is reported rather than silently passed over — a genuine drizzle output whose meta/ went missing must not just look clean. Both CLI call sites are unaffected; they pass one explicit --out and never sweep. The multi-directory tests added by the previous commit keep their intent — their fixtures are now journal-bearing, so shrinking DRIZZLE_OUT_DIRS or short-circuiting the aggregation loop still fails them — and a new pair covers the foreign-directory case at both the sweep and command layers. Also adds the afterEach that post-agent.test.ts never had: all five tests in that describe leaked a temp tree per run.
…base
extractEncryptionSchema keys the encrypted-table column map by the Drizzle
table's JS PROPERTY (encText), not the DB column name (enc_text) — so
buildColumnKeyMap() is {encText: 'enc_text', ...} and resolveEncryptColumnMap
matches models on the property names. The v2 -> v3 port left the seed keyed by
DB name, so no field matched: bulkEncryptModels returned every row untouched,
with no failure, and the insert then carried PLAINTEXT into eql_v3_* columns.
The remap at seed.ts:66 was moving plaintext, not ciphertext, and its comment
had become false.
Not a data-safety bug — verified against a real Postgres with the pinned EQL
3.0.2 bundle, all three domains reject the insert with 23514. It is a bench
that cannot run at all for anyone with credentials.
Nothing caught it because nothing could. `tsc --noEmit` passes: BenchPlaintextRow
was hand-written and agreed with itself. harness.test.ts asserts a row count and
never that a column is encrypted. And CI runs only `test:local db-only`, which
never seeds — every suite that touches the seed needs credentials.
So the check that fails on this is deliberately cheap: __unit__/seed-keys.test.ts
compares the row's keys against buildColumnKeyMap()'s, under a second vitest
config with no globalSetup, so it needs neither a database nor credentials and
cannot be skipped for want of either. BenchPlaintextRow is now the property
space, and the identity remap is gone.
The adapter's v2 read path calls decryptModel(item) with NO table, on purpose:
a v2 table means nothing to a v3 client's reconstructor map, and the native
clients derive the table from the payloads. WasmEncryptionClient cannot — its
decrypt requires the table and resolves date fields from a per-table map, so
the omitted argument reached requireTable(undefined) and threw
`TypeError: Cannot read properties of undefined (reading 'tableName')` from
deep inside the client.
That client ships in this package, is the documented entry for Deno, Workers
and Supabase Edge Functions, and satisfies DynamoDBEncryptionClient
structurally — so encryptedDynamoDB accepted it with no cast and the pairing
only failed at the first read, with a message pointing nowhere near the cause.
Reject it where the message can name the combination. The signal is a declared
capability on the client (`requiresTableForDecrypt`) rather than sniffing arity
or constructor name. v3 tables are always passed the table, so they still work.
Also corrects three comments this package carried claiming audit metadata is
forwarded "regardless of client shape" and that "every client this package
ships carries .audit() on decrypt". The wasm-inline client's decrypt is a plain
async method; its audit metadata is dropped, and the debug message that fires
told the user to build a client with Encryption({ schemas }) — which is exactly
what the wasm entry's own factory is.
…te (#788 review) Addresses the three review findings on #788, plus a real defect found while verifying the second one. The wasm+v2 guard message is now operation-neutral. `assertClientTableVersionMatch` runs on all four operations, so a plain-JS caller reaching `encryptModel` with a v2 table got "cannot read legacy EQL v2 items ... would fail at the first read" — naming an operation that never ran. The pairing is wrong in both directions anyway (`Encryption()` on that entry rejects a v2 schema), so the message now says so instead of describing a read. Verifying that finding surfaced the larger one: `encryptModel` / `bulkEncryptModels` chained `.audit()` onto the client's result unconditionally. The native clients return a thenable operation carrying it; the wasm-inline client's encrypt returns a bare `Promise<WasmResult>` and has no `.audit()` anywhere. So EVERY EQL v3 write through this adapter on that entry failed with `client.encryptModel(...).audit is not a function` — surfaced as a `DYNAMODB_ENCRYPTION_ERROR`, indistinguishable from a genuine encryption fault. This PR's own changeset claimed the opposite ("EQL v3 tables are unaffected ... the wasm-inline client keeps working there"); it was true of decrypt only. `resolveEncryptResult` mirrors the existing `resolveDecryptResult`: tolerate the bare promise, drop audit metadata observably rather than crashing, and fail closed on a malformed result so an unencrypted item can never pass through as a success. The changeset and the `types.ts` docblock are corrected to match. Regression tests are credential-free. The chainable half matters most — the native clients' encrypt audit trail had no coverage outside a live-ZeroKMS suite, so nothing would have caught breaking it. skills/stash-dynamodb documented v2 decrypt as unconditionally supported and never mentioned that wasm-inline refuses v2 tables, on the documented entry for the runtimes most often paired with DynamoDB. It also claimed audit metadata forwards "regardless of client shape" — the exact phrase this PR removed from the source as untrue. Both corrected, plus the wasm-inline row in skills/stash-encryption, which never said the entry is v3-only. packages/bench/package.json is back to 2-space indent, keeping only the substantive `test:unit` addition: 53 changed lines down to 1. The repo's Biome config is `indentStyle: "space"` and the file was spaces on main, so the churn was neither required nor conformant — Biome ignores `**/package.json` entirely, verified by direct invocation.
… disproved (#788 review) Follow-up to the #788 review of this branch. The largest item is that the fix landed at runtime but the TYPE still declared the invariant it disproves. `CallableEncryptionClient.encryptModel` / `bulkEncryptModels` returned `ChainableEncryptOperation`, declaring an `.audit()` that the wasm-inline entry does not have — precisely the assumption that let the write path chain it unconditionally and fail every EQL v3 write there. The decrypt members already returned `unknown` with a docblock explaining why; encrypt never got the same treatment. Both are now `unknown`, `ChainableEncryptOperation` is deleted (it had no other reference), and the docblock covers all four members. This is not cosmetic: re-introducing `client.encryptModel(...).audit(...)` now fails to compile with TS2571, verified by reverting the call site under tsc. The regression is statically unreachable rather than merely fixed. The type is internal — absent from every emitted .d.ts — so there is no API change and no changeset. The test I had described as load-bearing was the weakest one. It stubbed `audit()` returning a promise; the native contract is `audit(): this` on a thenable whose `then()` calls `execute()`, so metadata is read back off the operation at execution time rather than passed forward. That stub passes even if `audit()` stops recording entirely. It now subclasses the real `EncryptionOperation`, mirroring what the decrypt half of the file already did, and asserts the metadata reaches `execute()` and that the operation runs exactly once. A native-failure case covers the other arm. Also strengthened, all verified by mutation (each kills exactly 2 tests): - the audit-drop test asserted only that the result succeeded — it now asserts the drop is logged and names the entry to switch to, plus a mirror proving a chainable client does NOT trip the drop path - both malformed-result loops gained `null` and `[]`. `null` makes the `resolved === null` clause load-bearing (without it `'data' in null` throws); `[]` is `typeof 'object'` so it must be rejected on the key checks `DecryptFailure` is renamed `ResultFailure` — it was already the encrypt helper's failure type, structurally identical, wrong name. bench: the `BenchPlaintextRow` docblock claimed it was "derived from the table so the two cannot drift again". It is hand-written. Deriving it was investigated and is actively worse: `InferInsertModel` describes the ENCRYPTED column and degrades to optional `any`, and `extractEncryptionSchema` returns the widened `AnyV3Table` whose index-signature column map admits both `encTxt: 'x'` and `encText: 12345`, which the literal rejects. The comment now says so and names the unit test as the real guard. The credential-free bench step moved ahead of `docker compose up`: leaving it last meant a database failure would skip the one check whose entire rationale is that it cannot be skipped for want of a database. Its `server.deps.inline` for test-kit was inert — nothing on that path imports it — and is gone; verified still passing with DATABASE_URL and all four CS_* vars unset.
On a mixed table — a legacy v2 pair the classifier no longer sees, plus one unrelated EQL v3 column — pickEncryptedColumn's sole-EQL-column rule claims the v3 column for the v2 plaintext. Three separate defects turned that guess into wrong outcomes. cutover had no `via` gate at all. Its v3 branch returns from inside try with exitCode untouched, so the finally never fires process.exit: it printed "point your application at email_enc" and exited 0 while the v2 rename never ran. drop.ts:106 already gated on `via === 'sole'` for exactly this reason. The manifest hint was discarded. backfill records the true pairing, so the answer was on disk, but resolveColumnLifecycle dropped a hint that failed to resolve and re-picked without it — reaching the sole rule. Recording the pairing changed nothing. Split the two reasons a hint fails: a column that is GONE is stale and still falls through to convention; a column that EXISTS but is not EQL v3 (the legacy eql_v2_encrypted case) is reported by name. drop's remedy prescribed the guess — `--encrypted-column <guess>`. Recording it makes the next resolution `via: 'hint'`, which walks past the gate, and the coverage check passes vacuously because an unrelated backfilled column is non-NULL on every row. The message was the instruction manual for generating a live DROP COLUMN on the plaintext at exit 0. Tested at the layer that had none: resolve-eql.test.ts covered only explainUnresolved, and encrypt-v3.test.ts stubs resolveColumnLifecycle outright, so neither could see the hint discard. The new tests keep pickEncryptedColumn real and replace only the two I/O boundaries.
Both placeholder templates emitted `await Encryption({ schemas: [] })`. An
empty schema set is a hard TS2769 against both overloads — deliberately, per
S-6 — so every `stash init` left a project failing its first tsc, in the one
file the CLI tells the user not to hand-edit. The old scaffold called
EncryptionV3, whose `readonly AnyV3Table[]` bound accepted `[]`; collapsing it
into an alias of Encryption tightened that away.
Relaxing the constraint is not an option (it exists to catch a real mistake),
and `stash init` has no table names in scope by design — it stopped
introspecting, and `build-schema.ts` sets `schemas: []` on its own state. So
the scaffold declares a sentinel table instead, which keeps the file compiling
and keeps the "you haven't declared anything yet" signal: loadEncryptConfig
exits 1 when `__stash_placeholder__` is the only table left, naming the file.
The gap that let this ship is the more important half. packages/cli has no
typecheck step (21 pre-existing errors), utils-codegen*.test.ts only
`toContain`-matches fragments, and build-schema.test.ts mocks
generatePlaceholderClient to '// placeholder' — so nothing anywhere compiled,
parsed or executed the generated output. Both templates are now committed as
`.generated.ts` fixtures compiled by a scoped tsconfig in CI, and pinned
byte-for-byte to the generator by a unit test. Verified the gate reproduces the
original TS2769 when the fixture is reverted.
The `.generated.ts` suffix is load-bearing: biome.json already excludes it, so
formatting cannot rewrite template output and break the byte comparison.
The `unresolvedHint` fail-closed added for #772 finding 7 had no `candidates.length > 0` guard, so it fired on pure EQL v2 tables too. `encrypt backfill` records `encryptedColumn` in migrations.json unconditionally, v2 included. On a pure-v2 table `listEncryptedColumns` returns [] (the classifier recognises `eql_v3_*` only), so the hint failed to resolve, `columnExists` found the real `eql_v2_encrypted` column, and `cutover` / `drop` exited 1 — telling users to downgrade for a lifecycle this same build still fully implements in cutover.ts / drop.ts. Gate the fail-closed on a non-empty candidate list. Finding 7's protection is unchanged: the mixed table it targets always has candidates. Order `explainUnresolved`'s empty-candidates fall-through ahead of the hint branch so both agree for direct callers. Also drop the "this release no longer manages that lifecycle" claim from the cutover/drop `via: 'sole'` messages — the build does still implement it; the command simply resolves EQL v3 counterparts only. Tests cover the previously untested shape: candidates [] + a recorded hint. Review follow-ups: - Extend the placeholder-table guard to `encrypt backfill` via loadEncryptionContext; correct the changeset/skill wording to name the commands that actually refuse (cutover/drop never read the client file). - Alias `@cipherstash/migrate` to source in the CLI unit vitest config, so `pnpm --filter stash test` no longer needs a prior workspace build (verified: 888 tests pass with packages/migrate/dist removed). - Revert the unrelated em-dash re-encoding in packages/cli/package.json.
Follow-up to 1d14412, from an adversarial cross-check of that commit. resolve-eql's private `columnExists` used a bare `to_regclass($1)`. That form parses and case-folds unquoted identifiers, so on a Prisma-style "User" table the probe reported the column missing, the recorded pairing was treated as stale, and the #772 fail-closed silently did not fire — falling through to the sole/convention rules and resolving the guess it exists to prevent. migrate already documents this exact anti-pattern (REGCLASS_SQL) and its test asserts `not.toMatch(/to_regclass\(\$1\)/)`. Moved to a shared, case-exact `columnExists` export and deleted the CLI copy; the test double is now case-exact too, so it cannot hide a regression. The placeholder guard read the harvested export map while the `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. Those disagree in both directions on one file: `schemas: [placeholderTable]` minus the `export` keyword fell through to "table not found … Available: (none)" — the error the guard replaces — and a stale placeholder export beside real tables wrongly fired it. Now reads the same source. cutover's `via:'sole'` refusal was nested inside `version === 3`; drop's is top-level. Equivalent today, but cutover's v2 ladder does an irreversible rename plus config promotion, so a restored v2 classification or a v4 family would let cutover rename on a guess drop refuses. Hoisted to match. Text that was false: - the scaffold `stash init` writes into every customer project (and both fixtures) claimed `stash encrypt` commands refuse to run; only backfill does - skills/stash-cli listed the client as loaded by `schema build` and `encrypt *`, and omitted `db push` - skills/stash-cli and skills/stash-encryption still said cutover on a backfilled v3 column exits 0. Since `.cipherstash/` is gitignored, a clone or CI runner hits the new `sole` refusal on a pure-v3 table with an unconventional column name — the changeset's "pure-v3 unaffected" was wrong too - both `sole` messages said "the table's only EQL column"; pickEncryptedColumn excludes the plaintext column first, so it fires with two - the remedy said to drive the v2 lifecycle "directly"; there is no CLI route, so it now says to run the eql_v2 SQL yourself - vitest.config.ts and packages/cli/AGENTS.md claimed the unit suite is self-contained. It is not: @cipherstash/stack is still reached via migrate/src/backfill.ts. The alias removed one of two couplings typecheck:scaffold ran `tsc` directly, bypassing turbo, so it needed a prior build and passed only by accident of step ordering behind steps that read as independently droppable. Now a turbo task with dependsOn ^build. Added scripts/__tests__/cli-vitest-alias.test.mjs — the CLI's alias map sits outside the vitest.shared.ts guard and must, since stackSourceAlias's '@/' would clobber the CLI's own. Runs in test:scripts, ahead of the build-dependent suite. Verified: frozen-lockfile install clean; 891 CLI unit, 41 migrate, 93 scripts, 76 pty e2e; code:check 0 errors; scaffold gate green through turbo with packages/stack/dist absent.
…exposed The reviewer's two findings were both already fixed at bbade2c — this adds the guards that would have caught them, plus two accuracy fixes. `typecheck:scaffold` declares `dependsOn: ["^build"]`, so turbo has to be the thing invoking it. #787 first added it as a bare `pnpm --filter stash run typecheck:scaffold` and CI went green anyway: an earlier step in the same job happened to build the workspace via its own `^build`. Those earlier steps read as independent guards for other packages, so they look freely removable — delete one and the scaffold step fails `TS2307`, which reads as "the scaffold is broken" rather than "you skipped the build". Nothing caught that. scripts/__tests__/workflow-turbo-build-deps.test.mjs derives the protected set from turbo.json (every task declaring `^build`) rather than hardcoding names, and asserts tests.yml routes them through turbo. Mutation-tested against three regressions: reverting the step to the bare form, deleting the step outright, and adding a new bare build-dependent step — each fails with a message naming the step and the fix. Two pre-existing bare `typecheck` invocations (prisma-next, wizard) carry the same latent trap; they are recorded in KNOWN_BARE rather than silently skipped, and a fourth test fails if an entry goes stale so the list gets worked down instead of accumulating. turbo.json is JSONC, so reading it needs the string-aware comment stripper #782 wrote for `turbo-skills-inputs.test.mjs` — a naive regex eats the `//` in `"$schema": "https://turbo.build/schema.json"`. Rather than hand-copy 30 lines of parser into an adjacent file, that function moves to `scripts/__tests__/lib/read-jsonc.mjs` and both guards import it. Two accuracy fixes: - vitest.config.ts claimed the suite's residual `@cipherstash/stack` coupling runs through `migrate/src/backfill.ts`. That is only one of two routes: `init/lib/__tests__/introspect.test.ts:1` imports `@cipherstash/stack/eql/v3` directly, never touching migrate. As written, someone could decouple backfill.ts and expect the suite to go standalone. It would not. - cli-vitest-alias.test.mjs called `.endsWith()` on every alias value, so Vite's array form (`[{find, replacement}]`) would throw an opaque TypeError from a helper — reading as "the guard is broken" rather than "the config changed shape". It now fails cleanly, naming the offender, and says to update the guard rather than delete it. No changeset: repo tooling and a comment, no published surface touched.
Follow-up to the #787 review. Five gaps, none raised by the reviewer, all found by auditing what the PR's own tests actually pin. A guard clause that no test held. `context-placeholder.test.ts` named a case it did not construct: its fixture returned `getEncryptConfig: () => ({})`, so `configuredTables` was empty and the guard short-circuited on the arity check before the sentinel-name comparison ever ran. Deleting `configuredTables.length === 1` left all 891 tests green — measured, not argued. The fixture now declares the sentinel FIRST alongside a real table, because the guard indexes `[0]`, and both tests spy `process.exit` so a regression fails as an assertion instead of killing the worker. One guard, not two. The same refusal was hand-copied into `loadEncryptConfig` and `loadEncryptionContext`, sharing only the constant — and it had already drifted: on a client whose `getEncryptConfig()` returns nothing, `db push` named the cause while `encrypt backfill` fell through to `Table "..." was not found`, the symptom-not-cause message the guard exists to replace. Both now call `requireUsableEncryptConfig`, pinned by a parity test asserting the two seams emit byte-identical text. The resolver/command seam, tested. `encrypt-v3.test.ts` stubs `resolveColumnLifecycle` and hand-writes its return value; `resolve-eql.test.ts` proves the pure-v2 shape produces that value. Nothing ran the real producer into the real consumer. The new composition test mocks no resolution at all — only pg, fs, prompts and the effectful migrate helpers — so the v2 verdict is derived from a real manifest hint and a real catalog read. Mutation-checked: the two #787 defences are independent, so removing either alone is masked by the other; removing both fails this test and nothing else. Its sibling types are now imported rather than re-declared, so the shape cannot drift silently. The workflow guard, applied to all workflows. It scanned only `tests.yml` while enforcing a rule its own docblock generalises. Widened to all 13, which surfaced six real bare invocations of `^build`-dependent tasks — five `test:integration`, plus a `test:e2e` that `rootScriptDelegatesToTurbo` was wrongly exempting (a `--filter` invocation runs the PACKAGE's script, so the root script's delegation says nothing about it). All six routed through turbo with `--env-mode=loose`: turbo defaults to strict and would otherwise withhold the step env these suites need, which `passThroughEnv` could only fix by enumerating ten variables across workflows this change cannot execute. Also corrects `AGENTS.md`'s path to `introspect.test.ts` and states the `stackSourceAlias` collision symmetrically — spreading it either way breaks one package's `'@/'`. Its "fails 10 files" figure was re-measured by deleting `packages/stack/dist`: exactly 10. `stash` unit suite 896/896, scripts 112/112, typecheck:scaffold clean, Biome 0 errors.
… the package-path linter sound
Finding 14 — decryptModel/bulkDecryptModels return an
AuditableDecryptModelOperation: thenable, with .withLockContext() and .audit().
Three sites in skills/stash-encryption and four in packages/stack/README.md
said the opposite ("a plain Promise<Result<...>>", "no .withLockContext() to
chain"), steering agents away from the very .audit() chain the audit-on-decrypt
changeset advertises. The skill contradicted its own reference table, which was
already right — 8b74430 fixed that table and nothing else, despite a commit
message claiming otherwise.
Both files ship: the skill inside the stash tarball and thence into customer
repos via installSkills(), the README inside the @cipherstash/stack tarball.
The equivalent statement about the WASM entry is CORRECT and deliberately
untouched — that client really does return a bare promise.
Also `protectOps.eq` in the setup prompt stash init writes for coding agents.
One occurrence repo-wide, and no such export exists; the real API is
createEncryptionOperators(client), conventionally `ops`.
Finding 15 — the package-path linter had a false positive and a false negative,
both fixed with the fixture-driven self-tests the suite already uses:
- The name capture had no right anchor, so a sentence-final `packages/stack.`
swallowed the period and reported a LIVE package as dead. Uppercase was also
excluded from the class, so a capitalised name was never checked at all.
- livePackages came from readdirSync, i.e. the working tree. Deleting a package
leaves dist/ and node_modules/ behind, so every reference to it kept passing —
the exact case the linter was commissioned to catch, silently unenforced on
any checkout that had built the package. Now derived from `git ls-files`.
Deliberately not "has a package.json": packages/utils has none and is live.
- scripts/ was not scanned, so the linters never checked themselves. Adding it
immediately surfaced a dead allowlist entry in lint-no-hardcoded-runners for
a path that never existed in git history. Self-test fixtures stay exempt —
they must name dead packages.
Severity note for the record: CI was never affected (fresh checkout, caching
disabled), and main carries no required status checks — this was a local
developer papercut plus a real soundness hole, not a broken build.
Finally, resolves two changesets that contradicted each other in the same
release: dynamodb-eql-v3 claimed "EQL v2 tables continue to work unchanged" and
"no existing caller needs to change" while stack-dynamodb-v2-write-removal
announced the v2 encrypt overloads as removed. The code sides with removal;
the claims are now scoped to the decrypt path.
…tch their own rot Follow-up to the #772 review findings, addressing what the review of that review turned up. `stash init`'s setup prompt named `createEncryptionOperators` unconditionally. That symbol is exported by `@cipherstash/stack-drizzle` alone, so a Supabase or Prisma Next project was sent after a package that is not in its dependency tree — the previous `protectOps.eq` was wrong for everyone, this was wrong for three integrations out of four. Step 5 now branches through `queryOperatorGuidance()`, following the `migrationCommands()` pattern already in the file: `ops.eq` for Drizzle, the `encryptedSupabase` wrapper's own filters for Supabase, the `eql*` column operators for Prisma Next, and `client.encryptQuery(...)` for plain Postgres — which is also pointed at `stash-encryption`, since it is the default integration and installs no integration skill, making "see the integration skill" a dangling pointer. The package-path linter reported an untracked-but-present package as "does not exist" — finding 15's false alarm pointed the other way, at a directory sitting right there on disk. The live set now unions `git ls-files --others --exclude-standard`. `--directory` is deliberately not passed: it collapses an all-ignored directory to one entry and would resurrect exactly the `dist/`-and- `node_modules/` shells the linter exists to catch (verified both ways). git failing now exits 2 with an actionable message instead of a raw ENOENT stack trace and exit 1, which was indistinguishable from a genuine lint failure, and an empty live set refuses to run rather than flagging every reference at once. The `scans scripts/ but not its fixtures` test asserted the repo was clean — byte-for-byte what the suite's first test already asserted, and passing whether or not `scripts` was in TARGETS. It now plants an offender in the scanned directory. Verified by mutation: it dies when `scripts` is dropped, as the untracked-package test dies without the union and the git-failure test dies when the exit code is flipped. The runners linter now requires each allowlist entry to exist and to still contain an unexcused `npx` literal. It immediately found a second stale entry the sibling linter structurally cannot see, since it only matches the `packages/<name>` shape: `setup-prompt.ts` has had no `npx` since its switch moved to `utils.ts`. Removed. Also corrects two claims: the test comment saying `packages/drizzle` never existed in git history (it was added speculatively by c671560, became load-bearing 31 minutes later with 9d259e6, and rotted when 413ca39 deleted the package), and the `AnyEncryptedTable` doc comment still promising v3 is "purely additive and no existing caller has to change" — true of decrypt, false of encrypt since the v2 write overloads were removed.
… repeats Remediation for the two review rounds on #789, plus what verifying them turned up in the same code. **The linter fix the review asked for twice.** `livePackages` moving from `readdirSync` to `git ls-files` was the highest-value change in the PR and had no test that pinned it. It still doesn't fail under the revert anyone would actually write: keeping the git call and unioning the filesystem back in passes every existing test while fully restoring the false negative. The new fixture builds the discriminating case — a package deleted from git whose gitignored `dist/` and `node_modules/` shells survive on disk — and is the only test that fails under both a naive `readdirSync` revert and that hybrid. The `git status` guard is scoped to the probe, not to `packages/`, so an unrelated uncommitted file doesn't turn it into an assertion about nothing. **A missing target no longer passes in silence.** A linter whose entire job is catching dead paths in configuration skipped dead paths in its own: rename a target and it dropped out of coverage forever, green. Its sibling already exits 2 for a stale allowlist entry. Both now do, and both report a target outside the repo by name instead of as a `../../../../..` chain climbing out of the root — the review's cosmetic nit, which turned out to sit in both files. **Every step that names an integration-specific API now branches.** Step 5 was fixed for this; steps 1, 2 and the read-path step were not. A Prisma Next project was sent at `types.*` / `encryptedTable` — the client `stash schema build` explicitly refuses to scaffold for it — and a plain-Postgres project was pointed three times at "the integration skill" it never gets installed. `encryptQuery` is shown taking the schema objects rather than an object-shorthand that read as three required strings. `queryOperatorGuidance` is a switch with a neutral default rather than an if-chain ending in Drizzle's answer: tsup transpiles without type-checking and this package has no typecheck script, so nothing would have caught a fifth integration inheriting it. One test asserted nothing: it scoped to a `#### Encryption cutover` heading that does not exist, and `substring(-1)` returns the whole document.
…default `stash plan` writes one of three templates. Two of them — the cutover plan and the `--complete-rollout` plan — described the EQL v2 rename swap as the only way to switch reads: "a single transaction renames `<col>` → `<col>_plaintext` and `<col>_encrypted` → `<col>`", with the steps after it referring to `<col>_plaintext` as though the rename had certainly happened. On EQL v3, which is the default, none of that is true. `stash encrypt cutover` refuses a v3 column outright — "Cut-over is not applicable to EQL v3 columns … there is no rename step" (`encrypt/cutover.ts:119`) — and refuses entirely on a v3-only database, where the `eql_v2_configuration` relation it needs does not exist (`:142`). So on a default install the agent was being asked to draft a plan around a command that will decline to run, and to name a `<col>_plaintext` column that will never exist. The implement prompt in this same file already splits the two correctly. This copies that split into the templates that were missing it. The EQL version is per-column — one database can hold both — so the templates tell the agent to establish it per column (`stash encrypt status`) rather than deciding once for the whole plan; a context-level flag would have been wrong for a mixed database. Also: the "this column is already encrypted" stop-and-ask keyed on the `eql_v2_encrypted` udt alone. v3 columns carry `eql_v3_*` domains, so on the default path the check could never fire. The agent reads the schema itself, so naming both is sufficient here. `introspect.ts:88` has the same v2-only assumption in code (`isEqlEncrypted: row.udt_name === 'eql_v2_encrypted'`). Left alone deliberately: it decides which columns `stash init` pre-selects as already-managed, so widening it changes setup behaviour and wants its own tests rather than riding along here.
`introspectDatabase` marked a column as CipherStash-managed only when its udt
was exactly `eql_v2_encrypted`:
isEqlEncrypted: row.udt_name === 'eql_v2_encrypted'
v3 is the default generation and its columns carry per-domain types —
`eql_v3_text_search`, `eql_v3_integer_ord`, and so on — so on the default path
this was false for every encrypted column. The consequences are all in the
column picker: the table hint never said "N already encrypted", the
pre-selection notice never fired, and each encrypted column was displayed with
its plaintext `dataType` and left unticked. An encrypted column presented as
plaintext invites the user to encrypt it a second time, which is the direction
of wrongness that costs data rather than time.
`packages/wizard` already had the correct predicate, with the reasoning
written down (`isEqlEncryptedDomain`, `wizard-tools.ts:167`). This mirrors it
rather than inventing a second answer, and exports it so it is testable — the
old inline comparison sat inside a function that needs a live pg connection,
which is why nothing covered it.
Deliberately not `classifyEqlDomain` from `@cipherstash/migrate`, despite the
dependency already being present: that answers "which generation authors this"
and returns null for `eql_v2_encrypted`, since v2 is no longer authorable. The
question here is "is this encrypted at all", and v2 answers yes.
The per-column hint was also the hardcoded string `eql_v2_encrypted` for any
encrypted column, which mislabels a v3 column with a domain it does not have.
It now reports the column's actual udt, and the pre-selection notice lists the
domains it found instead of naming one it may not have seen.
Checked the rest of the CLI for the same assumption: every other site either
already handles both generations (`rewrite-migrations.ts` matches
`eql_v2_encrypted|eql_v3_[a-z0-9_]+`, `db-readers.ts`, `backfill.ts`) or is
legitimately v2-only (`cutover.ts`, the installer's CREATE TYPE permission
messages). Introspection was the sole outlier.
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.
…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.
…dicates and the WASM entry (#777) * feat(cli): add the stash-sql and stash-edge skills — raw-SQL predicates and the WASM entry Closes #754. No shipped skill covered the integrations that don't use an ORM: hand-written SQL over `pg` / `postgres-js`, and the WASM entry on Deno / Supabase Edge Functions / Workers. Grepping the skills `stash init` installs for `postgres-js|::jsonb::eql|sql.json|query_text_search` returned one hit, in an unrelated code comment — so an integration on that path had to recover the binding surface from `dist/*.d.ts`, the Postgres catalog, and experiment. Two skills rather than one, per the scope question in the issue: the raw-SQL predicate cookbook serves a supported plain-Node path (`hono-pg`) with no edge or WASM involvement, so scoping it under an edge-named skill would leave that surface uncovered. skills/stash-sql — the predicate matrix (which of `=`, `<>`, `<`, `>=`, `@@`, `@>` each column domain accepts, against which `eql_v3.query_*` operand), storage-vs-query payload shapes, per-driver parameter binding, and recipes for equality / free-text / range / ORDER BY / JSON containment / field selectors. skills/stash-edge — import specifier per runtime, the four mandatory `CS_*` variables and minting them with `stash env`, how the WASM client surface differs from the native typed client, and why a schema module can't be shared across the two entries. Both carry the credential-identity rule, also folded into stash-cli (under `env` and `encrypt backfill`, where it bites) and stash-supabase: EQL index terms derive from the ZeroKMS client key, so rows written under one credential and queried under another decrypt correctly and never match a query. Three claims were corrected against a live EQL v3 3.0.2 install rather than carried over from the issue: - The binding rule is driver-specific. On postgres-js a bare object fails to INSERT and `JSON.stringify(...)::jsonb` fails the domain CHECK; only `sql.json()` works in both positions. On `pg` all three forms work. - A bare-`jsonb` operand does not fall through to native jsonb semantics — EQL defines `jsonb` overloads that coerce to the *storage* domain, which requires the ciphertext key `c`, so a query term without the domain cast raises a CHECK violation rather than silently matching nothing. - The schema type incompatibility reproduces in both directions, not one. Also fixes the wasm-inline module JSDoc, which passed `OidcFederationStrategy.create(...)`'s Result straight to `config.authStrategy` without unwrapping — the same JSDoc the raw-SQL surface was being reverse-engineered from. SKILL_MAP (CLI + wizard) installs both for `postgresql` and `supabase`; Drizzle and Prisma Next get cross-links from their own skills instead. * refactor(skills): rename stash-sql to stash-postgres The skill is Postgres-specific throughout — encrypted columns are Postgres domains over jsonb, the driver rules cover pg and postgres-js, and the drift check reads information_schema. Nothing in it generalises to another SQL dialect, so the name overclaimed. Renames the directory and frontmatter name, updates the SKILL_MAP entries in both the CLI and wizard installers, the setup-prompt purpose line, the cross-links in the five sibling skills, and the AGENTS.md skill list and change→skill map. The skill has not shipped yet (no CHANGELOG entry — only the pending changeset), so no installed .claude/skills/ directory carries the old name and the existing changeset is amended in place rather than adding a second one. The new name also matches the CLI's `postgresql` integration key, which is the no-ORM path that installs it. * docs(skills): scope stash-postgres against Proxy and cite EQL upstream The skill covered client-side encryption over a direct connection without ever saying so. That is precisely the fork `stash init` asks about and stores as `usesProxy`: a CipherStash Proxy user writes plaintext SQL and Proxy encrypts on the wire, so every rule in the skill — mint a term with encryptQuery, cast to eql_v3.query_*, bind with sql.json — is wrong for them, and nothing said which world they were in. Adds a scope callout up front, and notes that Proxy's config lifecycle (stash db push into eql_v2_configuration) is the EQL v2 one, while this skill is v3 where there is nothing to push. The operator matrix also read as if the client library defined it. It does not: the domains, operators, CHECKs, and extractors all come from the EQL bundle developed at cipherstash/encrypt-query-language and shipped as @cipherstash/eql. A reader hitting an operator the matrix does not list had nowhere authoritative to check and no idea where to file it. Adds a provenance section with the version check (SELECT eql_v3.version()), ties the "operator does not exist" troubleshooting entry to it, and adds upstream links for EQL and Proxy to the Reference list. Also fixes the frontmatter description, which now states the direct-connection assumption so skill selection reflects it. * docs(skills): drop the usesProxy pointer from the stash-postgres Proxy callout The callout told readers to check `usesProxy` in `.cipherstash/context.json` to learn which path they were on. That flag and field are being removed (the CLI never used the value for anything beyond gating a wizard `stash db push` step), so the pointer would have shipped stale. The Proxy scoping itself stands — it is the substance of the callout. * docs(skills): separate the database domain from its language mappings "Assume `users.email` is a `types.TextEq` column" named the schema builder as though it were the column's type. It isn't: `types.TextEq` is a TypeScript factory in this repo, while the column is `public.eql_v3_text_eq`, a Postgres domain over jsonb. The distinction is the one the whole section turns on, so opening by conflating them was the wrong footing. Now opens with the database type and states that the domain is the authority — what the CHECK enforces and what decides the operator set — then lists the three things that map onto it: the `types.*` schema factory, the `TextEq` / `TextEqQuery` wire types from `@cipherstash/eql`, and the `eql_v3.query_*` operand domain. Records where the TypeScript types come from: generated with the JSON Schemas from the Rust `eql-bindings` crate, with the SQL bundle built from the same commit, so the wire shape and the domain CHECK cannot drift. Verified against @cipherstash/eql 3.0.2 — `TextEq` is `{ v, i, c, hm }` and `TextEqQuery` is `{ v, i, hm }`, so the generated pair is exactly the storage-vs-query split this section goes on to explain. * docs(skills): send agents to EQL for the current type surface The domain and operator tables read as authoritative. They are not — they are a snapshot of a versioned surface defined in `encrypt-query-language`, and nothing in CI catches them drifting when the `@cipherstash/eql` pin moves. Marks them as a snapshot at the point of use, and adds a ranked list of places to confirm current types: the EQL skill first (it ships beside the bundle it documents, so it tracks the installed version), then the generated `@cipherstash/eql` TypeScript types, then the install SQL's CREATE OPERATOR statements, then `SELECT eql_v3.version()` against the database. The middle two need only `node_modules` — the `stash` CLI depends on `@cipherstash/eql` at an exact pin — so the check costs nothing and needs no connection. The EQL skill does not exist yet (cipherstash/encrypt-query-language#422), so it is referenced by role rather than by a name that may not survive review, and every other rung of the ladder works today without it. * docs(skills): stop teaching native-entry bundling in the edge skill `stash-edge` named `@cipherstash/protect-ffi` five times. Three of those were configuration for the runtime this skill exists to steer readers away from: the entry-choice table's Node row explained how to externalise it in Next, and the Workers section named it again to say the config was irrelevant. A reader here is not configuring a Node server, so that is the wrong skill to carry it — the bundling guide already owns it, and is now linked instead. Two mentions stay, for different reasons: - The `When to Use` trigger keeps the name. Deploying the default entry to an edge runtime fails with `Cannot find module '@cipherstash/protect-ffi'`, and that string in a deploy log is how an agent should route itself here. - The Deno note keeps `--allow-ffi`, which is Deno's own permission flag rather than the package — a diagnostic that the wrong entry resolved. The intro now says "a Node-API native module" without naming the package; the contrast it draws does not need it. * docs(skills): correct the lock-context explanation in stash-edge DEPENDS ON #797 — do not merge #777 until the wasm-inline `.withLockContext()` lands. This describes the post-#797 surface. The skill said "Identity-bound encryption is configured, not chained" and presented `config.authStrategy` as the replacement for `.withLockContext()`. That conflates two orthogonal mechanisms. An auth strategy decides who the client is; a lock context decides which key the value is encrypted under. You need both, and a strategy alone writes data that is not identity-bound. What actually changed on the native entry was authentication: per-operation CTS tokens were removed in protect-ffi 0.25, so `LockContext.identify()` is deprecated and the strategy handles token acquisition. `.withLockContext()` never went anywhere. The WASM entry picked up the authentication half and not the key-binding half, and the skill then described the gap as though the first subsumed the second. Rewrites the section as two numbered steps, adds the encrypt-side example, and states the symmetry rule — the same claim must be supplied on decrypt, and a mismatch surfaces as a failed decrypt rather than a key error. The code example assumes the chainable form, matching native. If #797 lands a per-call option instead, this example must change before #777 merges. * docs(skills): address CodeRabbit review on #777 Five of six findings applied; the sixth skipped with a reason. - `stash-drizzle`: `db.execute(sql\`…\`)` used backslash-escaped backticks inside an inline code span. Escapes do not work inside code spans, so the backslashes rendered literally. Switched to a double-backtick span, which is the actual mechanism for embedding a backtick. - `stash-edge`, `stash-postgres`: language tags on three unlabelled fences — a TypeScript error, the domain-name mapping, and a Postgres CHECK-violation message. All `text`. - `stash-postgres` frontmatter: the operator list omitted `<=` and `>`, which the predicate matrix includes. The description is the skill-selection surface, so an incomplete list there is worse than verbose. - `stash-postgres` Query Recipes: the recipes dereference `.data` without guarding `.failure`, and agents copy from them. Rather than adding the guard to six short recipes, the section now states the omission up front and shows the guard once — unguarded, the failure surfaces as a domain CHECK violation rather than as the encryption error it is, which is the confusing outcome this skill exists to prevent. Skipped: adding a "the docs site needs a corresponding update" note to the changeset. Changesets become CHANGELOG entries, which are customer-facing; an internal follow-up action does not belong there. Tracked separately. Two other unlabelled fences exist in `stash-encryption` and `stash-cli` but predate this branch and are untouched by it. * docs(skills): reconcile the WASM entry surface after the rebase Rebasing onto current `remove-v2` pulled in a `stash-encryption` subpath row asserting the WASM entry has "no `.audit()` or `.withLockContext()` chaining", which contradicts `stash-edge` — the two ship in the same tarball, so an agent reading both gets opposite answers about the same entry. `.withLockContext()` stays described as chainable (this branch documents the post-#797 surface, per 9a57509), so the `stash-encryption` row now states only the `.audit()` difference and defers identity-bound encryption to `stash-edge`. The `.audit()` claim is the one that survives independently: the WASM operations return plain Results rather than thenable operations (`wasm-inline.ts:664`). The changeset carried the same stale "no `.withLockContext()`" phrasing and is corrected with it. Also documents a real native/WASM divergence the difference table was missing, raised in review: the model decrypt helpers. Both entries take the table as the second argument, but the native typed client *also* carries a one-arg `decryptModel(model)` overload — the read path for rows whose table isn't in the schema set, legacy EQL v2 above all (`encryption/v3.ts:121-134`). The WASM entry has no such overload; `requiresTableForDecrypt` is declared true and the call throws without a table rather than returning a `{ failure }` (`wasm-inline.ts:674-685`). A wrapper written against the one-arg form therefore compiles on one entry and breaks on the other. Still blocked on #797 — `.withLockContext()` does not exist on the wasm-inline client at any published `protect-ffi` (0.30.0 is the pin and npm `latest`; its WASM typings carry no lock-context declarations). protectjs-ffi#143 has since landed the FFI half, as a per-call `lockContext` option field rather than a chainable method, so the chainable form here remains a bet on how #797 wraps it — the review condition 9a57509 set out. * docs(skills,stack): document the wasm entry as having no lock context Unblocks this PR from #797 by describing the surface that exists today rather than the one #797 will create. `.withLockContext()` is absent from the wasm-inline client on every branch in this repo, and no published protect-ffi adds it — the FFI exposes `lockContext` as an option field, never a chainable, so the chainable form was always going to be a stack-side wrapper that has not been written. What is kept from 9a57509 is the part that was right and is the reason #793 exists: an auth strategy and a lock context are orthogonal. A strategy decides who the client is; a lock context decides which key the value is encrypted under. The earlier text presented the first as a replacement for the second. That claim is now stated as the mistake it is, in both the skill and the source comment it came from, instead of being silently deleted. The two consequences are silent, so both are now written down rather than left to surface as a failed decrypt: - Values written from the edge entry are encrypted under the workspace key even when the client is authenticated as an end user. - The edge entry cannot read anything the native entry wrote under a lock context, since decrypt requires the same context. A read split between the two entries, on top of the schema nominal-typing incompatibility. `wasm-inline.ts` also records that the binding already accepts a lock context on both paths, so closing #797 is plumbing rather than a new capability — and that it wants a live round-trip test first, since a wrong or missing claim surfaces as a failed decrypt rather than a key error. The `stash-encryption` subpath row regains the `.withLockContext()` fact the rebase reconciliation had removed, now that the two skills agree again.
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.
Umbrella PR for #707 — the integration branch collecting the EQL v2 removal stack. Still a draft: seven of the planned PRs have landed,
packages/cliandpackages/stackstill carry v2, and the branch is one commit behindmain.Goal
Make EQL v3 the sole generation the workspace authors and emits, while preserving the ability to decrypt existing v2 payloads. "Remove v2" means remove the v2 emission and authoring surface, not the read path.
End state: no
eql_v2_*emission or authoring path, one naming convention (v3 takes the unsuffixed name), green suite, v2 ciphertext still readable.Decisions
Referenced by number from the sub-PRs. Stated here so they resolve.
test/live/side-by-side-clients-live-pg.test.tsruns a v2 and a v3 client in one process against one database. No shipped data-migration tooling.*V3name stays as a@deprecatedalias, so people already on v3 don't rewrite imports twice. Exception:stack-drizzle's./v3subpath is a hard break, no alias — post-collapse the root names match v2's, so a stale import would type-check while silently binding v3 semantics. Full rationale in Remove EQL v2 repo-wide (umbrella) #707's "Naming — decided".Landed
PRs 1–7. All green on
a16302fc.protect,schema,protect-dynamodb(~19k lines)migrate— drop EQL v2 from the domain-type classifierstackcore — v3 audit-on-decrypt, collapseEncryptionV3intoEncryptionstack-supabase— remove v2 authoring surface, de-suffix v3, deletequery-builder-v3.tsstack-drizzle— remove v2 root, collapse./v3to root (hard break, no alias)wizard— port the migration rewriter to the v3 domain familycli/examples/meta — de-suffix the init scaffold, rewriteexamples/basicto v3, sweep the docsVerified:
examples/basic,examples/prismaande2etypecheck clean; Biome reports 0 errors (196 warnings, CI gates on errors only); all eightSKILL.mdfiles are free of stale references, with every*V3mention explicitly marked deprecated;EncryptionErrorTypesand the EQL payload keys are untouched; everyexportsentry keeps bothimportandrequire.@cipherstash/stack/v3,EncryptionV3andencryptedSupabaseV3still exist as@deprecatedaliases per Decision 5 — they are not stale.Branch state
One commit behind
main, one conflict:packages/stack-drizzle/README.mdagainst #775. The sixstack-drizzleconflicts from #763 are already resolved —encryptedIndexessits atsrc/indexes.tsand is exported from the root barrel.Remaining
PR 8 —
cli+migrate, the v2 SQL leaf≈ −25,200 / +300 across ~40 files; ~2,700 lines excluding the vendored SQL.
Delete:
src/sql/*.sql(22,512 lines, read only byinstaller/index.tsand copied bytsup.config.ts:83);encrypt/cutover.ts(306);db/push.ts(177);db/activate.ts(73);db/supabase-migration.ts(133);migrate/src/eql.ts(166 — all eight functions confirmedeql_v2.*mutations, exported atmigrate/src/index.ts:44-53); the--eql-version/--latestflags atregistry.ts:353-364,426-437; the v2 routing indb/install.ts:269-316,697-727,760-816.eql upgradestays —db/upgrade.ts:29already defaults to v3. Only its flags go.Deleting
cutover.tsleaves no hole. The v3 ladder is complete:backfill.ts:561→drop.ts:113-123→dropped, already rendered byquest.ts:95-104. The identically-named plan step (parse-plan.ts:14,rollout-state.ts:26) is v3-valid and survives.Must survive:
db/status.ts:32-62,104-131(legacy v2 detection — without it a v2 customer is told "EQL is not installed"); the manifesteqlVersionfallbacks inencrypt/status.ts:127-129,154-183andstatus/index.ts:41,55,136-147(the only remaining v2 signal since #761);eql_v2_encryptedinrewrite-migrations.ts:11(that sweep runs on the v3 path over repos whose history carries v2 tokens);encrypt/lib/db-readers.ts:46-82; the empty-candidate guard inencrypt/lib/resolve-eql.ts.Decrypt-path risk, must be mitigated in the same PR: deleting the vendored SQL removes the only offline way to recreate
public.eql_v2_encryptedwhen restoring a v2 dump into a fresh Postgres. Surfaceinstaller/index.ts:38-40's upstreameql-2.3.1release URL in the--eql-version 2rejection message.Sequencing:
tsup.config.ts:83'scpSync('src/sql', …)throws on a missing directory, so it moves in the same commit. Confirmeql/migration.ts:229's ALTER COLUMN sweep is complete before deletinginstall.ts:638-680, or the CLI ships broken Drizzle migrations.Open: whether
--eql-version 3stays accepted (8 doc/skill sites plustest-kit/src/install.ts:64pass it;parseArgsis lenient, so leaving it silently ignored is dishonest).skills/stash-encryption/SKILL.md:1057becomes false on merge.PRs 9–12 —
packages/stack, the v2 authoring and emission surfaceNot previously tracked as outstanding, and the largest remaining piece.
src/schema/index.tsis 754 lines, of which ~511 (68%) are v2-only authoring:EncryptedColumn(269-452),EncryptedTable(459-548),EncryptedField(214-267),encryptedTable/encryptedColumn/encryptedField(599-716). All three root re-exports atsrc/index.ts:9are v2-only — the package root exposes no v3 authoring.The emission path is live.
resolveEqlVersion(encryption/index.ts:97-131) detects v3 byhasBuildColumnKeyMap, and:130returnsundefinedwhen no v3 table is present. protect-ffi 0.30.0 documentsundefinedas defaulting to 2. SoEncryption({ schemas: [v2Table] })still writes v2 wire, and there is no runtime rejection of a v2 schema (encryption/index.ts:879-947checks only non-empty schemas, keyset UUID, mixed sets and v2ste_vec). Only the WASM entry rejects, atwasm-inline.ts:1359-1366— the pattern to lift.Removing the DSL does not remove the
undefinedbranch: the looseBuildableTablepath (types.ts:271) would keep writing v2 silently.Test-coverage gap — Decision 6 is currently unmet. No test anywhere has a v3-configured client decrypt v2 ciphertext, which is the actual invariant customers depend on. All v2 coverage is same-client v2→v2 round-trip. Worse,
integration/shared/v2-decrypt-compat.integration.test.ts:29-45mints its fixture through the emission path being removed — it dies with the code it guards, and passes falsely until then. A purely static fixture is not viable as the sole replacement: ZeroKMS ciphertexts are keyset-bound, andtest-kit/src/env.ts:30-67lets credentials come from a developer's own profile, so a fixture minted against CI's workspace won't decrypt locally.Suggested split:
EncryptConfigrather than the DSL.buildEncryptConfig(encryption/index.ts:910currently uses the v2 copy for both generations); de-coupledynamodb/types.ts:11,29,266,280. No public surface change.encryptedTable/Infer*name collisions, port or drop ~30 v2-authored test files.Keep the
eqlVersion: 2hatch — it outlives the builders and the read-compat test needs it.Fix before merge
.changeset/carries 29 stale references across 12 files, and these render into the shipped 1.0.0 CHANGELOG.adapter-package-split.md:14,15,22-24andeql-v3-drizzle.md:5,8,12,13nameencryptedType,@cipherstash/stack/eql/v3/drizzleand the*V3drizzle symbols as migration targets; six further files point at@cipherstash/stack-drizzle/v3;drizzle-encrypted-indexes.md:5advertises the deleted/v3entry;eql-v3-sole-docs.md:10-13contradictsstack-dynamodb-v2-write-removal.mdon DynamoDB.packages/stack/README.md:446-451headlines the Supabase section with the deprecatedencryptedSupabaseV3, contradicting:766of the same file and three other sources.init/lib/introspect.ts:88matches onlyeql_v2_encrypted, sostash initpre-selects nothing on a v3 database and:233announces the wrong domain. Pre-existing onmain, fixed in PR 8.examples/*ande2e/*typecheck in no CI job —turbofilters to./packages/*. That is whyexamples/basiccould sit broken here. Add a job.Release mechanics
Mid-RC, fixed group:
stash,stack,stack-drizzle,stack-supabase,prisma-next,wizard(all at1.0.0-rc.4). Six major-bearing changesets land inside it —stack-1-0-0-rc,stack-audit-on-decrypt,stack-dynamodb-v2-write-removal,remove-eql-v2-drizzle-root,remove-eql-v2-supabase-authoring,supabase-single-row-typing— so the whole group goes to1.0.0-rc.5.migrate→1.0.0-rc.2;nextjs→4.1.2.Two corrections wanted:
remove-eql-v2-packages.md's@cipherstash/nextjs: patchis a phantom bump.packages/nextjsdepends only onjoseand references no@cipherstash/*package; it would ship a changelog entry about three packages it never used. Drop the line.remove-eql-v2-migrate-classifier.mdatpatchunder-communicates.migrate/src/version.ts:47-51dropped theeql_v2_encrypted → 2arm, solistEncryptedColumnssilently omits v2 columns under an unchanged signature, andresolveEncryptedColumn/detectColumnEqlVersioninherit it.minorat minimum (moot inside migrate's own major, but the changelog should say it).Out of scope
#708 (Supabase v3 in Workers/browser), #654 (v3 Supabase full-ciphertext scalar operand), #637, and customer data-migration tooling.
Closes
Closes #707
Closes #752
Closes #198
#198 closes as obsoleted, not implemented —
encryptedTypeno longer exists in the Drizzle public API, so the duplicative-generic complaint has no referent. #752's last item was thestash initscaffold, delivered by #776.Already closed as
NOT_PLANNEDon 2026-07-21, carried by this work: #409, #599, #423, #426, #421, #422.