From d8e6e7d2136fd991e2a6b573f014f4adff430821 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Sat, 25 Jul 2026 14:29:40 +1000 Subject: [PATCH 1/2] fix(cli): stop encrypt cutover/drop acting on a guessed encrypted column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `. 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. --- .changeset/encrypt-lifecycle-mixed-table.md | 39 +++++ .../encrypt/__tests__/encrypt-v3.test.ts | 44 +++++- packages/cli/src/commands/encrypt/cutover.ts | 18 ++- packages/cli/src/commands/encrypt/drop.ts | 12 +- .../encrypt/lib/__tests__/resolve-eql.test.ts | 149 +++++++++++++++++- .../src/commands/encrypt/lib/resolve-eql.ts | 64 +++++++- 6 files changed, 315 insertions(+), 11 deletions(-) create mode 100644 .changeset/encrypt-lifecycle-mixed-table.md diff --git a/.changeset/encrypt-lifecycle-mixed-table.md b/.changeset/encrypt-lifecycle-mixed-table.md new file mode 100644 index 00000000..78d41244 --- /dev/null +++ b/.changeset/encrypt-lifecycle-mixed-table.md @@ -0,0 +1,39 @@ +--- +'stash': patch +--- + +`stash encrypt cutover` and `stash encrypt drop` no longer act on — or report +success for — an encrypted column they only guessed at. + +On a table holding both a legacy EQL v2 pair (`ssn` / `ssn_encrypted`) and one +unrelated EQL v3 column, the v2 ciphertext column is not classified as an EQL +column at all, so column resolution fell through to the "this is the table's +only EQL column" rule and claimed the unrelated v3 column. Three consequences, +all now fixed: + +- **`cutover` reported success for work it never did.** Its EQL v3 branch had no + guard on how the column was resolved, and returned without setting an exit + code — so it printed "point your application at `email_enc`" and exited 0 + while the v2 rename never ran. A scripted rollout read that as complete. It + now refuses, and exits 1, exactly as `drop` already did. + +- **The recorded pairing was discarded.** `encrypt backfill` writes the true + `encryptedColumn` to `.cipherstash/migrations.json`, so the answer was already + on disk — but a hint that failed to resolve was dropped entirely and the + re-resolution reached the guess. A hint naming a column that still exists but + is not an EQL v3 column is now reported as what it is (most often a legacy + `eql_v2_encrypted` counterpart) instead of being replaced by a guess. A + genuinely stale hint — one naming a column that is gone — still falls back to + the naming convention as before. + +- **`drop`'s refusal message prescribed the guess.** It told the user to re-run + `backfill --encrypted-column `. Following it recorded the + guess as fact, so the next run resolved "by hint", walked past the refusal, + and passed the coverage check vacuously — an unrelated but legitimately + backfilled column is non-NULL on every row — then generated a live + `DROP COLUMN` on the plaintext and exited 0. The message now asks for the + column that actually encrypts the named one, and says explicitly not to record + the guess. + +Pure-v2 and pure-v3 tables are unaffected, as are tables with two or more EQL v3 +columns (resolution already failed closed there). diff --git a/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts b/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts index 45def1ef..a0d58329 100644 --- a/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts +++ b/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts @@ -216,6 +216,36 @@ describe('encrypt cutover — EQL version awareness', () => { ) }) + // #772 review, finding 7. `drop` gated on `via === 'sole'`; `cutover` did + // not, and its v3 branch `return`s without setting exitCode — so a mixed + // table (a v2 pair the classifier no longer sees, plus one unrelated v3 + // column) produced a success-shaped message and exit 0 while the v2 rename + // never ran. A scripted rollout read that as done. + it("refuses a by-elimination ('sole') match rather than reporting success", async () => { + lifecycleMock.mockResolvedValue( + resolved( + { column: 'email_enc', domain: 'eql_v3_text_search', version: 3 }, + 'sole', + ), + ) + migrateMocks.progress.mockResolvedValue({ phase: 'backfilled' }) + const exitSpy = spyExit() + + await cutoverCommand({ table: 'users', column: 'ssn' }) + + expect(p.log.error).toHaveBeenCalledWith( + expect.stringContaining('nothing confirms it encrypts "ssn"'), + ) + // The old message told the user to point their application at the guessed + // column, as though the lifecycle were complete. + expect(p.log.info).not.toHaveBeenCalledWith( + expect.stringContaining('point your application at email_enc'), + ) + expect(migrateMocks.renameEncryptedColumns).not.toHaveBeenCalled() + expect(exitSpy).toHaveBeenCalledWith(1) + exitSpy.mockRestore() + }) + it('fails closed when EQL columns exist but none is identifiable', async () => { lifecycleMock.mockResolvedValue({ info: null, @@ -372,9 +402,21 @@ describe('encrypt drop — EQL version awareness', () => { expect(p.log.error).toHaveBeenCalledWith( expect.stringContaining('nothing confirms it encrypts "email"'), ) - expect(p.log.error).toHaveBeenCalledWith( + // The remedy must not prescribe the GUESS. Recording `secret_blob` makes + // the next resolution `via: 'hint'`, which walks past this very gate — and + // the coverage check then passes vacuously, because an unrelated but + // legitimately-backfilled column is non-NULL on every row. Following that + // advice generated a live DROP COLUMN on the plaintext at exit 0 + // (#772 review, finding 7). + expect(p.log.error).not.toHaveBeenCalledWith( expect.stringContaining('--encrypted-column secret_blob'), ) + expect(p.log.error).toHaveBeenCalledWith( + expect.stringContaining('--encrypted-column '), + ) + expect(p.log.error).toHaveBeenCalledWith( + expect.stringContaining('do not record secret_blob'), + ) expect(migrateMocks.countUnencrypted).not.toHaveBeenCalled() expect(writeFileMock).not.toHaveBeenCalled() expect(exitSpy).toHaveBeenCalledWith(1) diff --git a/packages/cli/src/commands/encrypt/cutover.ts b/packages/cli/src/commands/encrypt/cutover.ts index 676c4e24..5b990613 100644 --- a/packages/cli/src/commands/encrypt/cutover.ts +++ b/packages/cli/src/commands/encrypt/cutover.ts @@ -70,7 +70,7 @@ export async function cutoverCommand(options: CutoverCommandOptions) { // DOMAIN TYPES (manifest name as a hint; the `_encrypted` naming is // a convention, never relied upon) before any phase/config checks so v3 // users get the real answer, not a confusing precondition error. - const { info, candidates } = await resolveColumnLifecycle( + const { info, candidates, unresolvedHint } = await resolveColumnLifecycle( client, options.table, options.column, @@ -86,6 +86,7 @@ export async function cutoverCommand(options: CutoverCommandOptions) { options.table, options.column, candidates, + unresolvedHint, ) if (!info && unresolved) { p.log.error(unresolved) @@ -96,6 +97,21 @@ export async function cutoverCommand(options: CutoverCommandOptions) { if (info?.version === 3) { const encryptedColumn = info.column + + // `via: 'sole'` means only that this is the table's ONE EQL v3 column — + // nothing ties it to the plaintext column the user named. On a mixed + // table (a v2 pair the classifier no longer sees, plus one unrelated v3 + // column) that guess is simply wrong, and reporting "nothing to do for + // EQL v3" for it told a scripted rollout the cut-over had succeeded when + // the v2 rename never ran. `drop.ts` already refuses a `'sole'` match for + // the same reason (#772 review, finding 7). + if (info.via === 'sole') { + p.log.error( + `${options.table}.${encryptedColumn} (${info.domain}) is the table's only EQL v3 column, but nothing confirms it encrypts "${options.column}" — refusing to report a cut-over outcome on that guess. If "${options.column}" pairs with a legacy eql_v2_encrypted column, this release no longer manages that lifecycle. Otherwise record the pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column \`.`, + ) + exitCode = 1 + return + } if (state?.phase === 'dropped') { // Terminal phase — the lifecycle already finished. Not an error and // not "finish the backfill": there is nothing left to backfill. diff --git a/packages/cli/src/commands/encrypt/drop.ts b/packages/cli/src/commands/encrypt/drop.ts index 9beb6953..ae5181e4 100644 --- a/packages/cli/src/commands/encrypt/drop.ts +++ b/packages/cli/src/commands/encrypt/drop.ts @@ -74,7 +74,7 @@ export async function dropCommand(options: DropCommandOptions) { // column, droppable straight after `backfilled`. The version and the // encrypted column's name are resolved from the DOMAIN TYPES (manifest // name as a hint) — the `_encrypted` naming is a convention only. - const { info, candidates } = await resolveColumnLifecycle( + const { info, candidates, unresolvedHint } = await resolveColumnLifecycle( client, options.table, options.column, @@ -91,6 +91,7 @@ export async function dropCommand(options: DropCommandOptions) { options.table, options.column, candidates, + unresolvedHint, ) if (!info && unresolved) { p.log.error(unresolved) @@ -103,9 +104,16 @@ export async function dropCommand(options: DropCommandOptions) { // that destroys the only copy of this data. Dropping is the single // irreversible step in the lifecycle, so it demands a positively // asserted pairing (manifest hint or the naming convention). + // + // The remedy must NOT name `info.column`. That is the guess itself, and + // recording it turns the next resolution into `via: 'hint'`, which walks + // straight past this gate — while the coverage check below passes + // vacuously, because a legitimately-backfilled unrelated column is + // non-NULL on every row. Following the old message verbatim generated a + // live `DROP COLUMN` on the plaintext at exit 0 (#772 review, finding 7). if (info?.via === 'sole') { p.log.error( - `${options.table}.${info.column} (${info.domain}) is the table's only encrypted column, but nothing confirms it encrypts "${options.column}" — refusing to generate an irreversible drop on that guess. Record the pairing and retry: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column ${info.column}\` (which writes it to the manifest), or set "encryptedColumn": "${info.column}" for this column in .cipherstash/migrations.json.`, + `${options.table}.${info.column} (${info.domain}) is the table's only encrypted column, but nothing confirms it encrypts "${options.column}" — refusing to generate an irreversible drop on that guess. Identify the column that actually encrypts "${options.column}" and record that pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column \` (which writes it to the manifest), or set "encryptedColumn" for this column in .cipherstash/migrations.json. If "${options.column}" pairs with a legacy eql_v2_encrypted column, this release no longer manages that lifecycle — do not record ${info.column}.`, ) exitCode = 1 return diff --git a/packages/cli/src/commands/encrypt/lib/__tests__/resolve-eql.test.ts b/packages/cli/src/commands/encrypt/lib/__tests__/resolve-eql.test.ts index 1e5bc6ca..0310891c 100644 --- a/packages/cli/src/commands/encrypt/lib/__tests__/resolve-eql.test.ts +++ b/packages/cli/src/commands/encrypt/lib/__tests__/resolve-eql.test.ts @@ -13,8 +13,40 @@ */ import type { EncryptedColumnInfo } from '@cipherstash/migrate' -import { describe, expect, it } from 'vitest' -import { explainUnresolved } from '../resolve-eql.js' +import type pg from 'pg' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// Only the two I/O boundaries are replaced. `pickEncryptedColumn` stays real — +// it IS the resolution rule under test, and stubbing it (as `encrypt-v3.test.ts` +// stubs `resolveColumnLifecycle`) is why the hint-discard below had no coverage. +const readManifest = vi.hoisted(() => + vi.fn(async () => null as { tables: Record } | null), +) +const listEncryptedColumns = vi.hoisted(() => + vi.fn(async () => [] as EncryptedColumnInfo[]), +) +vi.mock('@cipherstash/migrate', async (importOriginal) => ({ + ...(await importOriginal()), + readManifest, + listEncryptedColumns, +})) + +const { explainUnresolved, resolveColumnLifecycle } = await import( + '../resolve-eql.js' +) + +/** + * A `pg` double answering only the `pg_attribute` existence probe, from a set + * of column names the table is pretended to have. That probe is the whole + * point: it is what separates "the hint is stale" from "the hint names a real + * column that simply is not EQL v3". + */ +const clientWithColumns = (...columns: string[]): pg.ClientBase => + ({ + query: async (_sql: string, params: unknown[]) => ({ + rows: [{ exists: columns.includes(String(params[1])) }], + }), + }) as unknown as pg.ClientBase const v3 = ( column: string, @@ -57,3 +89,116 @@ describe('explainUnresolved', () => { expect(message).toContain('Cannot identify which encrypted column') }) }) + +/** + * #772 review, finding 7 — the mixed-table dead end. + * + * `classifyEqlDomain` recognises `eql_v3_*` only, so on a table holding a v2 + * pair (`ssn` / `ssn_encrypted`) plus one unrelated v3 column, the v2 ciphertext + * column is not a candidate at all. `pickEncryptedColumn`'s sole-EQL-column rule + * then claims the unrelated v3 column for `ssn`. + * + * `encrypt backfill` records the true pairing in the manifest, so the answer is + * on disk — but the hint was thrown away whenever it failed to resolve, and the + * re-pick without it reached the sole rule. Recording the pairing changed + * nothing, and `drop`'s remedy told the user to record the guess instead. + */ +describe('resolveColumnLifecycle — a recorded hint that is not a v3 candidate', () => { + const V3_OTHER: EncryptedColumnInfo = { + column: 'email_enc', + domain: 'eql_v3_text_search', + version: 3, + } + + beforeEach(() => { + vi.clearAllMocks() + readManifest.mockResolvedValue(null) + listEncryptedColumns.mockResolvedValue([]) + }) + + it('does not fall back to the sole-column guess when the manifest names a counterpart', async () => { + // The v2 pair is invisible to the classifier; only email_enc is a candidate. + listEncryptedColumns.mockResolvedValue([V3_OTHER]) + readManifest.mockResolvedValue({ + tables: { users: [{ column: 'ssn', encryptedColumn: 'ssn_encrypted' }] }, + }) + + const { info, unresolvedHint } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'ssn_encrypted', 'email_enc'), + 'users', + 'ssn', + ) + + expect(info).toBeNull() + expect(unresolvedHint).toBe('ssn_encrypted') + }) + + it('explains the recorded counterpart by name rather than listing candidates', async () => { + const message = explainUnresolved( + 'users', + 'ssn', + [V3_OTHER], + 'ssn_encrypted', + ) + + expect(message).toContain('ssn_encrypted') + expect(message).toContain('not an EQL v3 column') + // Naming the guess here is what sent users to `--encrypted-column email_enc`. + expect(message).not.toContain('--encrypted-column email_enc') + }) + + // The fallback the comment actually describes — a hint naming a column that + // is simply gone — must still resolve through convention. + it('still falls back to convention when the hint is genuinely stale', async () => { + listEncryptedColumns.mockResolvedValue([ + { column: 'ssn_encrypted', domain: 'eql_v3_text_eq', version: 3 }, + ]) + readManifest.mockResolvedValue({ + tables: { users: [{ column: 'ssn', encryptedColumn: 'ssn_old' }] }, + }) + + // `ssn_old` is gone from the table entirely — the genuinely stale case. + const { info } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'ssn_encrypted'), + 'users', + 'ssn', + ) + + expect(info?.column).toBe('ssn_encrypted') + expect(info?.via).toBe('convention') + }) + + it('resolves through the hint when it does name a candidate', async () => { + listEncryptedColumns.mockResolvedValue([ + { column: 'ssn_enc', domain: 'eql_v3_text_eq', version: 3 }, + ]) + readManifest.mockResolvedValue({ + tables: { users: [{ column: 'ssn', encryptedColumn: 'ssn_enc' }] }, + }) + + const { info, unresolvedHint } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'ssn_enc'), + 'users', + 'ssn', + ) + + expect(info?.column).toBe('ssn_enc') + expect(info?.via).toBe('hint') + expect(unresolvedHint).toBeUndefined() + }) + + // No manifest entry at all is the ordinary case; the sole rule still applies + // and `drop`'s own `via === 'sole'` gate is what refuses it. + it('leaves the sole-column rule alone when nothing was recorded', async () => { + listEncryptedColumns.mockResolvedValue([V3_OTHER]) + + const { info, unresolvedHint } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'email_enc'), + 'users', + 'ssn', + ) + + expect(info?.via).toBe('sole') + expect(unresolvedHint).toBeUndefined() + }) +}) diff --git a/packages/cli/src/commands/encrypt/lib/resolve-eql.ts b/packages/cli/src/commands/encrypt/lib/resolve-eql.ts index b4e595a1..df8f765b 100644 --- a/packages/cli/src/commands/encrypt/lib/resolve-eql.ts +++ b/packages/cli/src/commands/encrypt/lib/resolve-eql.ts @@ -23,6 +23,17 @@ export interface ResolvedLifecycle { * identifiable — callers name them instead of erroring blind. */ candidates: EncryptedColumnInfo[] + /** + * The manifest's recorded `encryptedColumn` when it named a column that is + * NOT among `candidates` — i.e. the pairing is on record but the column it + * names is not an EQL v3 column (typically legacy `eql_v2_encrypted`, which + * `classifyEqlDomain` no longer recognises). + * + * Distinct from a stale hint. It means the answer IS known and disagrees + * with anything resolution could otherwise guess, so callers must fail closed + * naming it rather than fall through to `via: 'sole'` (#772 review, finding 7). + */ + unresolvedHint?: string } /** @@ -59,11 +70,45 @@ export async function resolveColumnLifecycle( )?.encryptedColumn const candidates = await listEncryptedColumns(client, table) - let info = hint ? pickEncryptedColumn(candidates, column, hint) : null - // A stale hint (column since renamed/retyped) must not mask a resolvable - // counterpart — fall back to convention + sole-EQL-column resolution. - if (!info) info = pickEncryptedColumn(candidates, column) - return { info, candidates } + const hinted = hint ? pickEncryptedColumn(candidates, column, hint) : null + if (hinted) return { info: hinted, candidates } + + // The hint named a column that is not a candidate. Two very different + // reasons, and they must not share an outcome: + // + // - the column no longer exists (renamed, dropped) — a genuinely STALE hint, + // which must not mask a resolvable counterpart, so fall through; + // - the column exists but is not an EQL v3 column — the usual shape being a + // legacy `eql_v2_encrypted` counterpart, which `classifyEqlDomain` stopped + // recognising. Here the pairing IS known and falling through would discard + // it in favour of a guess: on a mixed table the sole-EQL-column rule then + // claims an unrelated v3 column, `cutover` reports success for a rename it + // never performed, and `drop`'s remedy tells the user to record the guess + // (#772 review, finding 7). + if (hint && (await columnExists(client, table, hint))) { + return { info: null, candidates, unresolvedHint: hint } + } + + return { info: pickEncryptedColumn(candidates, column), candidates } +} + +/** Whether `column` exists on `table` at all, whatever its type. */ +async function columnExists( + client: pg.ClientBase, + table: string, + column: string, +): Promise { + const { rows } = await client.query<{ exists: boolean }>( + `SELECT EXISTS ( + SELECT 1 FROM pg_attribute + WHERE attrelid = to_regclass($1) + AND attname = $2 + AND attnum > 0 + AND NOT attisdropped + ) AS exists`, + [table, column], + ) + return rows[0]?.exists === true } /** @@ -87,7 +132,16 @@ export function explainUnresolved( table: string, column: string, candidates: readonly EncryptedColumnInfo[], + unresolvedHint?: string, ): string | null { + // The recorded pairing points at a real column that is not an EQL v3 column + // — almost always a legacy `eql_v2_encrypted` counterpart. Say exactly that: + // listing the v3 candidates here would invite the user to record one of them, + // which is how the guess used to get laundered into a `via: 'hint'` match. + if (unresolvedHint !== undefined) { + return `${table}.${column} is recorded as pairing with "${unresolvedHint}", but ${unresolvedHint} is not an EQL v3 column — it is most likely a legacy eql_v2_encrypted column, which this command no longer manages. Finish the EQL v2 lifecycle for this column with a stash release that still supports it, or drop the recorded pairing from .cipherstash/migrations.json if it is wrong.` + } + if (candidates.length === 0) return null const listed = candidates .map((c) => ` - ${c.column} (${c.domain})`) From 7fc55b6c76aa504b4624759ac25ac71f7e697475 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Sat, 25 Jul 2026 14:34:07 +1000 Subject: [PATCH 2/2] fix(cli): make stash init's scaffolded client compile, and gate it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .changeset/init-scaffold-compiles.md | 24 ++++++ .github/workflows/tests.yml | 9 +++ .../scaffold/drizzle.generated.ts | 65 ++++++++++++++++ .../scaffold/generic.generated.ts | 60 +++++++++++++++ packages/cli/package.json | 5 +- .../placeholder-client-fixture.test.ts | 74 +++++++++++++++++++ packages/cli/src/commands/init/utils.ts | 36 ++++++--- packages/cli/src/config/index.ts | 22 ++++++ packages/cli/tsconfig.scaffold.json | 25 +++++++ skills/stash-cli/SKILL.md | 2 +- 10 files changed, 309 insertions(+), 13 deletions(-) create mode 100644 .changeset/init-scaffold-compiles.md create mode 100644 packages/cli/__fixtures__/scaffold/drizzle.generated.ts create mode 100644 packages/cli/__fixtures__/scaffold/generic.generated.ts create mode 100644 packages/cli/src/commands/init/__tests__/placeholder-client-fixture.test.ts create mode 100644 packages/cli/tsconfig.scaffold.json diff --git a/.changeset/init-scaffold-compiles.md b/.changeset/init-scaffold-compiles.md new file mode 100644 index 00000000..84587fa3 --- /dev/null +++ b/.changeset/init-scaffold-compiles.md @@ -0,0 +1,24 @@ +--- +'stash': patch +--- + +The client file `stash init` writes now compiles. + +Both placeholder templates emitted `await Encryption({ schemas: [] })`, and +`Encryption` requires at least one table — an empty schema set is a deliberate +compile error, so it cannot be relaxed. Every `stash init` therefore left a +project whose first `tsc` or `next build` failed, in a file the CLI had just +told the user not to hand-edit. (The previous scaffold called `EncryptionV3`, +whose looser bound accepted `[]`; collapsing that into an alias of `Encryption` +tightened it.) + +The scaffold now declares a single sentinel table, `__stash_placeholder__`, so +the file typechecks as written. `stash encrypt` commands refuse to run while +that table is still the only one declared, and say so — rather than failing +later with a confusing "table not found". + +Nothing in the repo compiled this output before: `packages/cli` has no +typecheck step, the codegen tests only string-match fragments of the template, +and the step test stubs the generator out entirely. Both templates are now +committed as fixtures that CI typechecks, pinned byte-for-byte to the generator +so they cannot drift. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 849feafb..7541cf41 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -160,6 +160,15 @@ jobs: - name: Typecheck (examples/basic — guards the v3 stack/stack-drizzle importers) run: pnpm exec turbo run typecheck --filter @cipherstash/basic-example + # `stash init` writes a client file into the user's project and tells them + # not to hand-edit it. Nothing compiled that file, so tightening + # `Encryption` to require a non-empty schema set left every `stash init` + # emitting a project that fails its first `tsc` — with CI green (#772 + # review). The fixtures are pinned byte-for-byte to the generator by + # `placeholder-client-fixture.test.ts`. + - name: Typecheck (stash init's scaffolded client) + run: pnpm --filter stash run typecheck:scaffold + - name: Lint — no hardcoded package-manager runners run: pnpm run lint:runners diff --git a/packages/cli/__fixtures__/scaffold/drizzle.generated.ts b/packages/cli/__fixtures__/scaffold/drizzle.generated.ts new file mode 100644 index 00000000..a0f9b77f --- /dev/null +++ b/packages/cli/__fixtures__/scaffold/drizzle.generated.ts @@ -0,0 +1,65 @@ +/** + * CipherStash encryption client — placeholder. + * + * `stash init` wrote this file. It is intentionally NOT a real Drizzle + * schema. Your existing schema files (typically under `src/db/`) remain + * authoritative — your agent will edit those directly when you encrypt a + * column, then update the `Encryption({ schemas: [...] })` call below + * to reference the encrypted tables you declared there. + * + * Until that happens, the encryption client is initialised with a single + * placeholder table so that this file compiles, and `stash encrypt` + * commands refuse to run and point back here. + * + * This project uses EQL v3. Encrypted columns are concrete Postgres domains + * built with the `types.*` factories from `@cipherstash/stack-drizzle`. + * Each domain's query capabilities are FIXED by the type you pick — there is + * no capability config object. Choose the factory whose capabilities you need: + * types.Text / types.Integer / … storage only (encrypt/decrypt, no queries) + * types.TextEq / types.IntegerEq equality (eq, inArray) + * types.IntegerOrd / types.DateOrd equality + order/range (gt/lt/between/sort) + * types.TextMatch free-text match only + * types.TextSearch equality + order/range + free-text + * types.Json encrypted-JSONB containment + selectors + * + * --- Pattern reference (copy into your real schema, do NOT use as-is) --- + * + * Encrypted twin column for an existing populated column (path 3 — lifecycle): + * + * import { pgTable, integer, text } from 'drizzle-orm/pg-core' + * import { types } from '@cipherstash/stack-drizzle' + * + * export const users = pgTable('users', { + * id: integer('id').primaryKey().generatedAlwaysAsIdentity(), + * email: text('email').notNull(), // existing plaintext, unchanged for now + * email_encrypted: types.TextSearch('email_encrypted'), // encrypted twin, NULLABLE — never .notNull() + * }) + * + * Net-new encrypted column (path 1 — declare encrypted from the start): + * + * export const orders = pgTable('orders', { + * id: integer('id').primaryKey().generatedAlwaysAsIdentity(), + * billing_address: types.TextEq('billing_address'), + * }) + * + * Once you have encrypted tables declared, harvest them and pass to Encryption(): + * + * import { extractEncryptionSchema } from '@cipherstash/stack-drizzle' + * import { Encryption } from '@cipherstash/stack/v3' + * import { users, orders } from './db/schema' + * + * export const encryptionClient = await Encryption({ + * schemas: [extractEncryptionSchema(users), extractEncryptionSchema(orders)], + * }) + */ +import { Encryption, encryptedTable, types } from '@cipherstash/stack/v3' + +// REPLACE THIS. It exists only so this file compiles before you have declared +// any encrypted tables — `Encryption` requires at least one. Swap it for your +// real tables (see the patterns above); `stash encrypt` refuses to run while +// the placeholder is still here. +export const placeholderTable = encryptedTable('__stash_placeholder__', { + replace_me: types.Text('replace_me'), +}) + +export const encryptionClient = await Encryption({ schemas: [placeholderTable] }) diff --git a/packages/cli/__fixtures__/scaffold/generic.generated.ts b/packages/cli/__fixtures__/scaffold/generic.generated.ts new file mode 100644 index 00000000..4f583184 --- /dev/null +++ b/packages/cli/__fixtures__/scaffold/generic.generated.ts @@ -0,0 +1,60 @@ +/** + * CipherStash encryption client — placeholder. + * + * `stash init` wrote this file. It is intentionally NOT a real schema + * definition. Your existing schema files remain authoritative — your + * agent will declare encrypted columns there and update the + * `Encryption({ schemas: [...] })` call below to reference them. + * + * Until that happens, the encryption client is initialised with a single + * placeholder table so that this file compiles, and `stash encrypt` + * commands refuse to run and point back here. + * + * This project uses EQL v3. Encrypted columns are concrete Postgres domains + * built with the `types.*` factories from `@cipherstash/stack/eql/v3` + * (also re-exported from `@cipherstash/stack/v3`). Each domain's query + * capabilities are FIXED by the type you pick — there is no chainable + * capability tuner. Choose the factory whose capabilities you need: + * types.Text / types.Integer / … storage only (encrypt/decrypt, no queries) + * types.TextEq / types.IntegerEq equality + * types.IntegerOrd / types.DateOrd equality + order/range + * types.TextMatch free-text match only + * types.TextSearch equality + order/range + free-text + * types.Json encrypted-JSONB containment + selectors + * + * --- Pattern reference (copy into your real schema, do NOT use as-is) --- + * + * Encrypted twin column for an existing populated column (path 3 — lifecycle): + * + * import { encryptedTable, types } from '@cipherstash/stack/v3' + * + * export const users = encryptedTable('users', { + * email_encrypted: types.TextSearch('email_encrypted'), + * }) + * + * Net-new encrypted column (path 1 — declare encrypted from the start): + * + * export const orders = encryptedTable('orders', { + * billing_address: types.TextEq('billing_address'), + * }) + * + * Once you have encrypted tables declared, pass them to Encryption(): + * + * import { Encryption } from '@cipherstash/stack/v3' + * import { users, orders } from './db/schema' + * + * export const encryptionClient = await Encryption({ + * schemas: [users, orders], + * }) + */ +import { Encryption, encryptedTable, types } from '@cipherstash/stack/v3' + +// REPLACE THIS. It exists only so this file compiles before you have declared +// any encrypted tables — `Encryption` requires at least one. Swap it for your +// real tables (see the patterns above); `stash encrypt` refuses to run while +// the placeholder is still here. +export const placeholderTable = encryptedTable('__stash_placeholder__', { + replace_me: types.Text('replace_me'), +}) + +export const encryptionClient = await Encryption({ schemas: [placeholderTable] }) diff --git a/packages/cli/package.json b/packages/cli/package.json index 5b95ae38..ea8ea97e 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,7 +1,7 @@ { "name": "stash", "version": "1.0.0-rc.4", - "description": "CipherStash CLI — the one stash command for auth, init, encryption schema, database setup, and secrets.", + "description": "CipherStash CLI \u2014 the one stash command for auth, init, encryption schema, database setup, and secrets.", "repository": { "type": "git", "url": "git+https://github.com/cipherstash/stack.git", @@ -41,6 +41,7 @@ "postbuild": "chmod +x ./dist/bin/stash.js", "dev": "tsup --watch", "test": "vitest run", + "typecheck:scaffold": "tsc -p tsconfig.scaffold.json", "test:e2e": "vitest run --config vitest.integration.config.ts", "lint": "biome check ." }, @@ -56,7 +57,7 @@ "posthog-node": "^5.40.0", "zod": "^3.25.76" }, - "//optionalDependencies": "@cipherstash/auth ships per-platform native bindings as optional peerDependencies. pnpm does not auto-install platform-matched optional peer deps, so we declare them here as optionalDependencies — pnpm then picks the binary matching the host's os/cpu (from each sub-package's own package.json) and ignores the rest. All seven names share a single catalog entry to keep them in lockstep.", + "//optionalDependencies": "@cipherstash/auth ships per-platform native bindings as optional peerDependencies. pnpm does not auto-install platform-matched optional peer deps, so we declare them here as optionalDependencies \u2014 pnpm then picks the binary matching the host's os/cpu (from each sub-package's own package.json) and ignores the rest. All seven names share a single catalog entry to keep them in lockstep.", "optionalDependencies": { "@cipherstash/auth-darwin-arm64": "catalog:repo", "@cipherstash/auth-darwin-x64": "catalog:repo", diff --git a/packages/cli/src/commands/init/__tests__/placeholder-client-fixture.test.ts b/packages/cli/src/commands/init/__tests__/placeholder-client-fixture.test.ts new file mode 100644 index 00000000..c3ddbf54 --- /dev/null +++ b/packages/cli/src/commands/init/__tests__/placeholder-client-fixture.test.ts @@ -0,0 +1,74 @@ +/** + * Binds `__fixtures__/scaffold/*.ts` to what `generatePlaceholderClient` + * actually emits. + * + * The fixtures are the only thing in the repo that COMPILES the file + * `stash init` writes into a user's project — `tsconfig.scaffold.json` and the + * CI step that runs it exist for that. But a typechecked fixture is worthless + * if the generator can drift away from it, and the existing codegen tests only + * `toContain`-match fragments while `build-schema.test.ts` stubs the generator + * out entirely. That combination is how `Encryption({ schemas: [] })` shipped: + * every test was green and no compiler ever saw the output (#772 review, + * finding 6). + * + * So: byte-for-byte, both directions. Change a template, regenerate the + * fixture; the compiler then has an opinion about it. + */ +import { readFileSync } from 'node:fs' +import path from 'node:path' +import { fileURLToPath } from 'node:url' +import { describe, expect, it } from 'vitest' +import { PLACEHOLDER_TABLE_NAME } from '@/config/index.js' +import { generatePlaceholderClient } from '../utils.js' + +const FIXTURE_DIR = path.resolve( + fileURLToPath(import.meta.url), + '../../../../../__fixtures__/scaffold', +) + +const fixture = (name: string) => + readFileSync(path.join(FIXTURE_DIR, name), 'utf-8') + +describe('the scaffolded client fixtures match the generator', () => { + // Named `.generated.ts` so biome.json's existing exclusion leaves them alone: + // they are template OUTPUT, and reformatting them would break the byte-for-byte + // comparison below (which is the only thing tying the compiler to the generator). + it.each([ + ['generic.generated.ts', 'postgresql'], + ['drizzle.generated.ts', 'drizzle'], + ] as const)('%s', (file, integration) => { + expect(fixture(file)).toBe(generatePlaceholderClient(integration)) + }) + + // `supabase` shares the generic template; pin that so a future split does not + // silently leave the supabase path ungated. + it('supabase reuses the generic template', () => { + expect(generatePlaceholderClient('supabase')).toBe( + fixture('generic.generated.ts'), + ) + }) +}) + +describe('the scaffold compiles because it declares a table', () => { + it.each([ + 'generic.generated.ts', + 'drizzle.generated.ts', + ])('%s passes a non-empty schema set', (file) => { + const body = fixture(file) + // The empty form is a hard TS2769 against both overloads — `Encryption` + // requires at least one table by design (S-6), so the scaffold cannot go + // back to `schemas: []` without breaking every project it is written into. + expect(body).not.toContain('Encryption({ schemas: [] })') + expect(body).toContain('schemas: [placeholderTable]') + }) + + it.each([ + 'generic.generated.ts', + 'drizzle.generated.ts', + ])('%s uses the sentinel name the config loader refuses', (file) => { + // `loadEncryptConfig` exits 1 when this is the only table left, so the + // two must agree — otherwise the user gets a confusing "table not found" + // from whichever command runs next instead of "you never replaced this". + expect(fixture(file)).toContain(`'${PLACEHOLDER_TABLE_NAME}'`) + }) +}) diff --git a/packages/cli/src/commands/init/utils.ts b/packages/cli/src/commands/init/utils.ts index fc0e6aec..282a0320 100644 --- a/packages/cli/src/commands/init/utils.ts +++ b/packages/cli/src/commands/init/utils.ts @@ -384,9 +384,9 @@ const DRIZZLE_PLACEHOLDER = `/** * column, then update the \`Encryption({ schemas: [...] })\` call below * to reference the encrypted tables you declared there. * - * Until that happens, the encryption client is initialised with no - * schemas, and \`stash encrypt\` commands will surface a clear error - * pointing at this file. + * Until that happens, the encryption client is initialised with a single + * placeholder table so that this file compiles, and \`stash encrypt\` + * commands refuse to run and point back here. * * This project uses EQL v3. Encrypted columns are concrete Postgres domains * built with the \`types.*\` factories from \`@cipherstash/stack-drizzle\`. @@ -429,9 +429,17 @@ const DRIZZLE_PLACEHOLDER = `/** * schemas: [extractEncryptionSchema(users), extractEncryptionSchema(orders)], * }) */ -import { Encryption } from '@cipherstash/stack/v3' +import { Encryption, encryptedTable, types } from '@cipherstash/stack/v3' + +// REPLACE THIS. It exists only so this file compiles before you have declared +// any encrypted tables — \`Encryption\` requires at least one. Swap it for your +// real tables (see the patterns above); \`stash encrypt\` refuses to run while +// the placeholder is still here. +export const placeholderTable = encryptedTable('__stash_placeholder__', { + replace_me: types.Text('replace_me'), +}) -export const encryptionClient = await Encryption({ schemas: [] }) +export const encryptionClient = await Encryption({ schemas: [placeholderTable] }) ` const GENERIC_PLACEHOLDER = `/** @@ -442,9 +450,9 @@ const GENERIC_PLACEHOLDER = `/** * agent will declare encrypted columns there and update the * \`Encryption({ schemas: [...] })\` call below to reference them. * - * Until that happens, the encryption client is initialised with no - * schemas, and \`stash encrypt\` commands will surface a clear error - * pointing at this file. + * Until that happens, the encryption client is initialised with a single + * placeholder table so that this file compiles, and \`stash encrypt\` + * commands refuse to run and point back here. * * This project uses EQL v3. Encrypted columns are concrete Postgres domains * built with the \`types.*\` factories from \`@cipherstash/stack/eql/v3\` @@ -483,7 +491,15 @@ const GENERIC_PLACEHOLDER = `/** * schemas: [users, orders], * }) */ -import { Encryption } from '@cipherstash/stack/v3' +import { Encryption, encryptedTable, types } from '@cipherstash/stack/v3' + +// REPLACE THIS. It exists only so this file compiles before you have declared +// any encrypted tables — \`Encryption\` requires at least one. Swap it for your +// real tables (see the patterns above); \`stash encrypt\` refuses to run while +// the placeholder is still here. +export const placeholderTable = encryptedTable('__stash_placeholder__', { + replace_me: types.Text('replace_me'), +}) -export const encryptionClient = await Encryption({ schemas: [] }) +export const encryptionClient = await Encryption({ schemas: [placeholderTable] }) ` diff --git a/packages/cli/src/config/index.ts b/packages/cli/src/config/index.ts index 7103d7ba..c304f7d5 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -238,5 +238,27 @@ export async function loadEncryptConfig( ) process.exit(1) } + + // `stash init` scaffolds a client holding one placeholder table, because + // `Encryption` requires a non-empty schema set and the scaffold has no real + // tables to name yet. Reaching here with only that table means the user never + // replaced it — which used to surface as a confusing "table not found" from + // whichever command ran next. + const tables = Object.keys(config.tables ?? {}) + if (tables.length === 1 && tables[0] === PLACEHOLDER_TABLE_NAME) { + console.error( + `Error: ${encryptClientPath} still contains the placeholder table \`${PLACEHOLDER_TABLE_NAME}\` that \`stash init\` wrote.\n\nDeclare your encrypted columns and pass those tables to Encryption({ schemas: [...] }) in that file, then re-run this command.`, + ) + process.exit(1) + } + return config } + +/** + * The table name `stash init`'s scaffold uses so the file it writes compiles. + * + * Kept in sync with the templates in `commands/init/utils.ts` by + * `__tests__/placeholder-client-fixture.test.ts`, which also typechecks them. + */ +export const PLACEHOLDER_TABLE_NAME = '__stash_placeholder__' diff --git a/packages/cli/tsconfig.scaffold.json b/packages/cli/tsconfig.scaffold.json new file mode 100644 index 00000000..1a11f628 --- /dev/null +++ b/packages/cli/tsconfig.scaffold.json @@ -0,0 +1,25 @@ +{ + // Compiles the exact files `stash init` writes into a user's project. + // + // Nothing else in the repo did. `packages/cli` has no typecheck script (21 + // pre-existing errors), the codegen tests only string-match the templates, + // and `build-schema.test.ts` stubs the generator out entirely — so when + // `Encryption` was tightened to require a non-empty schema set, both + // placeholders started emitting a file that fails `tsc` on first build, in a + // file the CLI had just told the user not to hand-edit, and CI stayed green + // (#772 review, finding 6). + // + // Deliberately scoped to the fixtures so it gates the scaffold without + // waiting on the rest of the package to compile. + "extends": "../../tsconfig.json", + "compilerOptions": { + "noEmit": true, + "module": "ESNext", + "moduleResolution": "bundler", + "target": "ESNext", + "lib": ["ESNext"], + "strict": true, + "skipLibCheck": true + }, + "include": ["__fixtures__/scaffold/*.ts"] +} diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index aa1c03c2..49873523 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -221,7 +221,7 @@ Seven mechanical steps, no agent handoff. It prompts only when it can't pick a s 1. **Authenticate** — silent when a valid token exists. 2. **Resolve database** — per the resolution order above; verifies the connection. 3. **Resolve proxy choice** — CipherStash Proxy or direct SDK access (the default). Stored as `usesProxy` in `context.json`. Set by `--proxy` / `--no-proxy`; non-TTY without a flag defaults to SDK. -4. **Build schema** — auto-detects Drizzle (`drizzle.config.*`, `drizzle-orm`/`drizzle-kit`), Supabase (from the `DATABASE_URL` host), and Prisma Next. Writes a placeholder encryption client; prompts only if a file already exists there. +4. **Build schema** — auto-detects Drizzle (`drizzle.config.*`, `drizzle-orm`/`drizzle-kit`), Supabase (from the `DATABASE_URL` host), and Prisma Next. Writes a placeholder encryption client; prompts only if a file already exists there. The placeholder declares one sentinel table, `__stash_placeholder__`, because `Encryption` requires at least one — replace it with the tables you actually encrypt. Until you do, `stash encrypt` commands exit 1 and point back at that file. 5. **Install dependencies** — one combined prompt for `@cipherstash/stack` and `stash`. Skipped when both are present. 6. **Install EQL** — always EQL v3. **Drizzle** projects generate a v3 install migration (the same output as `eql migration --drizzle`, including the `cs_migrations` tracking schema) so the install lands in your migration history — apply it with `drizzle-kit migrate`; requires `drizzle-kit` to be installed and configured. **Prisma Next** is skipped (it installs EQL via `prisma-next migrate`). Everything else runs `eql install` directly against the resolved database, and is skipped when EQL is already installed. 7. **Gather context** — detects available coding agents and writes `.cipherstash/context.json`.