diff --git a/.changeset/encrypt-client-guard-parity.md b/.changeset/encrypt-client-guard-parity.md new file mode 100644 index 000000000..63c3a2664 --- /dev/null +++ b/.changeset/encrypt-client-guard-parity.md @@ -0,0 +1,20 @@ +--- +'stash': patch +--- + +`stash encrypt backfill` now names the cause when the encryption client has no +initialized encrypt config, instead of reporting a missing table. + +The guard that refuses an unusable client file existed twice — once in +`loadEncryptConfig` (`stash db push` / `db validate`) and once, hand-copied, in +`loadEncryptionContext` (`stash encrypt backfill`). The copies had already +drifted: for a client whose `getEncryptConfig()` returns nothing, `db push` +exited 1 with `Encryption client in has no initialized encrypt config`, +while `encrypt backfill` fell through to `Table "users" was not found in the +encryption client exports. Available: (none)` — naming the symptom rather than +the cause, which is precisely the failure the guard was added to eliminate. + +Both loaders now call one shared guard, so a single file cannot produce two +different diagnoses, and the refusals are pinned at both public entry points. +The un-replaced `stash init` scaffold is unaffected — it was already refused by +both, with the same message. diff --git a/.changeset/encrypt-lifecycle-mixed-table.md b/.changeset/encrypt-lifecycle-mixed-table.md new file mode 100644 index 000000000..3b3dab41a --- /dev/null +++ b/.changeset/encrypt-lifecycle-mixed-table.md @@ -0,0 +1,59 @@ +--- +'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. + +The `unresolvedHint` refusal is scoped to tables that actually hold EQL v3 +columns a guess could wrongly claim. A **pure-v2** table has none, so it still +falls through to the EQL v2 lifecycle exactly as before — including when +`encrypt backfill` recorded an `encryptedColumn` for it, which it does for v2 +columns too. + +Two cases DO newly exit 1, both deliberately: + +- Any table with at least one EQL v3 column where the manifest records an + `encryptedColumn` that exists but is not one of them — not only the + v2-pair-plus-one-v3 shape. The recorded pairing is authoritative and + disagrees with every candidate, so guessing past it is the bug. + +- A column whose encrypted counterpart could only be identified **by + elimination** — no recorded `encryptedColumn`, no `_encrypted` name + match, one EQL column left once the plaintext column itself is excluded. + `cutover` now refuses this as `drop` already did. Note `.cipherstash/` is + gitignored, so `migrations.json` is machine-local: a fresh clone or CI runner + can hit this on a **pure-v3** table whose encrypted column is named + unconventionally, where `cutover` previously exited 0 with "not applicable". + Re-run `stash encrypt backfill --table T --column C --encrypted-column ` + to record the pairing. diff --git a/.changeset/init-scaffold-compiles.md b/.changeset/init-scaffold-compiles.md new file mode 100644 index 000000000..f20ae5d4d --- /dev/null +++ b/.changeset/init-scaffold-compiles.md @@ -0,0 +1,27 @@ +--- +'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. Every command that reads the encryption client +— `stash db push`, `stash db validate`, and `stash encrypt backfill` — refuses +to run while that table is still the only one declared, and names it, rather +than failing later with a confusing "table not found". (`stash encrypt cutover` +and `stash encrypt drop` do not read the client file at all; they resolve +against the database.) + +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/.changeset/migrate-column-exists.md b/.changeset/migrate-column-exists.md new file mode 100644 index 000000000..d88867430 --- /dev/null +++ b/.changeset/migrate-column-exists.md @@ -0,0 +1,21 @@ +--- +'@cipherstash/migrate': minor +'stash': patch +--- + +Add `columnExists(client, tableName, columnName)` — a case-exact "does this +column exist at all?" catalog probe, distinct from `detectColumnEqlVersion`'s +"and is it an EQL column?". + +Callers need that difference to tell a STALE column reference (it is gone) from +a live one the domain classifier simply does not recognise — most often a legacy +`eql_v2_encrypted` counterpart. + +`stash encrypt cutover` / `drop` had a private copy of this probe built on a bare +`to_regclass($1)`. That form *parses* its argument and case-folds unquoted +identifiers, so on a Prisma-style `"User"` table it resolved `user`, reported the +column missing, and treated a valid recorded pairing as stale — silently skipping +the fail-closed that stops those commands acting on a guessed encrypted column. +The shared implementation quotes with `format('%I')` first, like every other +catalog probe in this package, so the lookup is case-exact while still honouring +`search_path` for unqualified names. diff --git a/.github/workflows/integration-drizzle.yml b/.github/workflows/integration-drizzle.yml index 7dd8310ba..c4b3e7669 100644 --- a/.github/workflows/integration-drizzle.yml +++ b/.github/workflows/integration-drizzle.yml @@ -153,8 +153,14 @@ jobs: # # Step env, not a `.env` file: `dotenv/config` does not override an # already-set `process.env`, so these win and no secret is written to disk. + # Through turbo, not a bare `pnpm --filter`: `test:integration` declares + # `dependsOn: ["^build", "build"]`, and the only build in this job is the + # setup action's `--filter stash`, which reaches these packages purely by + # coincidence of stash's own dependency graph. `--env-mode=loose` because + # turbo defaults to strict and would otherwise withhold the step env below + # from the test process (#787 review follow-up). - name: Drizzle v3 integration suites - run: pnpm --filter @cipherstash/stack-drizzle run test:integration + run: pnpm exec turbo run test:integration --filter @cipherstash/stack-drizzle --env-mode=loose env: DATABASE_URL: ${{ steps.db.outputs.database-url }} PGRST_URL: ${{ steps.db.outputs.pgrest-url }} @@ -164,7 +170,7 @@ jobs: # `isInstalled` short-circuits against the DB the first invocation already # provisioned — a fast no-op check, not a second schema apply. - name: Shared core + identity + wasm integration suites - run: pnpm --filter @cipherstash/stack run test:integration + run: pnpm exec turbo run test:integration --filter @cipherstash/stack --env-mode=loose env: DATABASE_URL: ${{ steps.db.outputs.database-url }} PGRST_URL: ${{ steps.db.outputs.pgrest-url }} diff --git a/.github/workflows/integration-prisma-next.yml b/.github/workflows/integration-prisma-next.yml index acfb99b64..6b7e19134 100644 --- a/.github/workflows/integration-prisma-next.yml +++ b/.github/workflows/integration-prisma-next.yml @@ -114,7 +114,7 @@ jobs: # `stash eql install --eql-version 3`, so an installer regression fails # here rather than hiding behind a test-only SQL apply. - name: prisma-next v3 family suites - run: pnpm --filter @cipherstash/prisma-next run test:integration + run: pnpm exec turbo run test:integration --filter @cipherstash/prisma-next --env-mode=loose env: # Step env, not a `.env` file: `dotenv/config` does not override an # already-set `process.env`, so these win and no secret hits disk. diff --git a/.github/workflows/integration-supabase.yml b/.github/workflows/integration-supabase.yml index 98b281655..f5eff3093 100644 --- a/.github/workflows/integration-supabase.yml +++ b/.github/workflows/integration-supabase.yml @@ -129,7 +129,7 @@ jobs: # Step env, not a `.env` file: `dotenv/config` does not override an # already-set `process.env`, so these win and no secret is written to disk. - name: Supabase v3 integration suites - run: pnpm --filter @cipherstash/stack-supabase run test:integration + run: pnpm exec turbo run test:integration --filter @cipherstash/stack-supabase --env-mode=loose env: DATABASE_URL: ${{ steps.db.outputs.database-url }} PGRST_URL: ${{ steps.db.outputs.pgrest-url }} @@ -139,7 +139,7 @@ jobs: # `isInstalled` short-circuits against the DB the first invocation already # provisioned — so this is a fast no-op check, not a second schema apply. - name: Shared core integration suites (against Supabase Postgres) - run: pnpm --filter @cipherstash/stack run test:integration + run: pnpm exec turbo run test:integration --filter @cipherstash/stack --env-mode=loose env: DATABASE_URL: ${{ steps.db.outputs.database-url }} PGRST_URL: ${{ steps.db.outputs.pgrest-url }} diff --git a/.github/workflows/prisma-next-e2e.yml b/.github/workflows/prisma-next-e2e.yml index 8c2e260ae..1455cad60 100644 --- a/.github/workflows/prisma-next-e2e.yml +++ b/.github/workflows/prisma-next-e2e.yml @@ -121,7 +121,7 @@ jobs: done - name: Run E2E suite - run: pnpm --filter @cipherstash/prisma-next-example test:e2e + run: pnpm exec turbo run test:e2e --filter @cipherstash/prisma-next-example --env-mode=loose - name: Stop E2E Postgres container if: always() diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 369eecd61..aadfe5aa5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -193,6 +193,21 @@ jobs: - name: Typecheck (stack — emitted declarations, not source) run: pnpm exec turbo run test:types:dist --filter @cipherstash/stack + # `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`. + # Through turbo, not `pnpm run` — the fixtures import `@cipherstash/stack/v3`, + # so this needs that package BUILT. Invoked directly it passed only because + # earlier steps in this job happen to build it via their own `^build`; drop + # or reorder those (they read as guards for other packages, so they look + # independently removable) and this fails `TS2307`, which reads as "the + # scaffold is broken" rather than "you forgot to build" (#787 review). + - name: Typecheck (stash init's scaffolded client) + run: pnpm exec turbo run typecheck:scaffold --filter stash + - name: Lint — no hardcoded package-manager runners run: pnpm run lint:runners diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 73795c3bc..aa65e4d00 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -6,11 +6,24 @@ This package has **two** Vitest configs. Run the right one for the change. | Command | Config | Scope | Needs build? | | --- | --- | --- | --- | -| `pnpm --filter stash test` | `vitest.config.ts` | Unit tests under `src/__tests__/**` and `src/**/__tests__/**` | No | +| `pnpm --filter stash test` | `vitest.config.ts` | Unit tests under `src/__tests__/**` and `src/**/__tests__/**` | **Partly** — needs `@cipherstash/stack` built (see below). Turbo's `^build` supplies it in CI. | | `pnpm --filter stash test:e2e` | `vitest.integration.config.ts` | E2E tests under `tests/e2e/**.e2e.test.ts` driving the built `dist/bin/stash.js` through a real pty (`node-pty`) | **Yes** — run `pnpm --filter stash build` first, or use the turbo `test:e2e` task which depends on `build`. | The unit config explicitly excludes `tests/e2e/**` so the default `pnpm test` -stays fast and self-contained. +stays fast. + +It is **not** fully self-contained, despite running standalone in CI. Some `src` +modules import workspace packages that publish `./dist` only, so an unbuilt +workspace fails at collection with `Failed to resolve entry for package …` +rather than at an assertion. `vitest.config.ts` aliases `@cipherstash/migrate` +to its source to remove one such coupling; `@cipherstash/stack` remains, reached +via `packages/migrate/src/backfill.ts` and a direct import in +`init/lib/__tests__/introspect.test.ts`. Deleting `packages/stack/dist` fails 10 +files. Closing it needs `vitest.shared.ts`'s `stackSourceAlias`, which cannot be +spread into this config: its `'@/'` points at `packages/stack/src` while this +package's points at `packages/cli/src`, and a flat alias map admits only one — +spread it after and stack's entry clobbers the CLI's, spread it before and the +CLI's breaks stack's own source imports (#787 review). ## When to add or update an E2E test diff --git a/packages/cli/__fixtures__/scaffold/drizzle.generated.ts b/packages/cli/__fixtures__/scaffold/drizzle.generated.ts new file mode 100644 index 000000000..755815874 --- /dev/null +++ b/packages/cli/__fixtures__/scaffold/drizzle.generated.ts @@ -0,0 +1,67 @@ +/** + * 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 db push`, + * `stash db validate` and `stash encrypt backfill` refuse to run and point + * back here. (`stash encrypt cutover` / `drop` resolve against the database + * and never read this file.) + * + * 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 db push`, `stash db validate` +// and `stash encrypt backfill` refuse 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 000000000..9f0187adc --- /dev/null +++ b/packages/cli/__fixtures__/scaffold/generic.generated.ts @@ -0,0 +1,62 @@ +/** + * 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 db push`, + * `stash db validate` and `stash encrypt backfill` refuse to run and point + * back here. (`stash encrypt cutover` / `drop` resolve against the database + * and never read this file.) + * + * 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 db push`, `stash db validate` +// and `stash encrypt backfill` refuse 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 5b95ae381..afbf3e98b 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -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 ." }, diff --git a/packages/cli/src/__tests__/placeholder-guard-parity.test.ts b/packages/cli/src/__tests__/placeholder-guard-parity.test.ts new file mode 100644 index 000000000..80563e9c2 --- /dev/null +++ b/packages/cli/src/__tests__/placeholder-guard-parity.test.ts @@ -0,0 +1,121 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { PLACEHOLDER_TABLE_NAME } from '@/config/index.js' + +/** + * `stash db push` / `db validate` reach the user's encryption client through + * `loadEncryptConfig`; `stash encrypt backfill` reaches the same file through + * `loadEncryptionContext`. Both must refuse the un-replaced `stash init` + * scaffold, and — because it is one file and one mistake — both must say the + * same thing about it. + * + * The two guards were hand-copied rather than shared, so nothing held them + * together: the message text could drift on one side, and the nullish-config + * case HAD already drifted (#787 review follow-up). This pins the agreement at + * both public seams rather than testing the shared helper directly, which + * would prove only that a function calls itself. + * + * Real jiti, real temp project, real filesystem — the only doubles are + * `process.exit` and `console.error`. + */ +describe('the un-replaced init scaffold, refused identically by both loaders', () => { + let tmpDir: string + let originalCwd: () => string + + const writeProject = (clientBody: string) => { + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `export default { + databaseUrl: 'postgresql://u:p@127.0.0.1:5432/db', + client: './client.ts', + }`, + ) + fs.writeFileSync(path.join(tmpDir, 'client.ts'), clientBody) + process.cwd = () => tmpDir + } + + /** The duck-typed table shape `loadEncryptionContext` harvests. */ + const table = (name: string) => + `{ tableName: '${name}', build: () => ({ tableName: '${name}', columns: {} }) }` + + /** Run one loader, capturing whatever it wrote to stderr before exiting. */ + const captureRefusal = async (load: () => Promise) => { + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + const exit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + let exited = false + try { + await load() + } catch (err) { + exited = (err as Error).message === 'process.exit' + if (!exited) throw err + } + const message = error.mock.calls.flat().join('\n') + error.mockRestore() + exit.mockRestore() + return { exited, message } + } + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-guard-parity-')) + originalCwd = process.cwd + }) + + afterEach(() => { + process.cwd = originalCwd + vi.restoreAllMocks() + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('gives db push and encrypt backfill the same refusal for the same file', async () => { + writeProject( + `export const encryptionClient = { + getEncryptConfig: () => ({ tables: { '${PLACEHOLDER_TABLE_NAME}': {} } }), + } + export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)}`, + ) + + const { loadEncryptConfig } = await import('@/config/index.js') + const dbPush = await captureRefusal(() => loadEncryptConfig('./client.ts')) + + const { loadEncryptionContext } = await import( + '../commands/encrypt/context.js' + ) + const backfill = await captureRefusal(() => loadEncryptionContext()) + + expect(dbPush.exited).toBe(true) + expect(backfill.exited).toBe(true) + expect(dbPush.message).toContain('still contains the placeholder table') + expect(backfill.message).toEqual(dbPush.message) + }) + + it('names the cause, not the symptom, when the client has no encrypt config', async () => { + // A client whose `getEncryptConfig()` returns nothing is the same class of + // un-finished setup. `db push` already named it; `encrypt backfill` used to + // fall through to `requireTable`'s `Table "users" was not found … + // Available: (none)` — the symptom-not-cause message this guard exists to + // replace (#787 review follow-up). + writeProject( + `export const encryptionClient = { getEncryptConfig: () => undefined } + export const users = ${table('users')}`, + ) + + const { loadEncryptConfig } = await import('@/config/index.js') + const dbPush = await captureRefusal(() => loadEncryptConfig('./client.ts')) + + const { loadEncryptionContext } = await import( + '../commands/encrypt/context.js' + ) + const backfill = await captureRefusal(() => loadEncryptionContext()) + + expect(dbPush.exited).toBe(true) + expect(backfill.exited).toBe(true) + expect(backfill.message).toContain('no initialized encrypt config') + expect(backfill.message).not.toContain('was not found') + }) +}) diff --git a/packages/cli/src/commands/encrypt/__tests__/context-placeholder.test.ts b/packages/cli/src/commands/encrypt/__tests__/context-placeholder.test.ts new file mode 100644 index 000000000..9a0ad2440 --- /dev/null +++ b/packages/cli/src/commands/encrypt/__tests__/context-placeholder.test.ts @@ -0,0 +1,170 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { PLACEHOLDER_TABLE_NAME } from '@/config/index.js' + +/** + * `stash encrypt` does not load its client through `loadEncryptConfig`, so the + * placeholder guard that command group needs lives in `loadEncryptionContext`. + * Both must refuse the un-replaced scaffold; otherwise `requireTable` reports + * `Table "users" was not found … Available: __stash_placeholder__`, which names + * the symptom instead of the cause (#787 review). + * + * Runs against the real jiti runtime — the guard reads the tables harvested + * from an actually-evaluated client module, which is the part worth pinning. + */ +describe('loadEncryptionContext — the un-replaced init scaffold', () => { + let tmpDir: string + let originalCwd: () => string + + const writeProject = (clientBody: string) => { + fs.writeFileSync( + path.join(tmpDir, 'stash.config.ts'), + `export default { + databaseUrl: 'postgresql://u:p@127.0.0.1:5432/db', + client: './client.ts', + }`, + ) + fs.writeFileSync(path.join(tmpDir, 'client.ts'), clientBody) + process.cwd = () => tmpDir + } + + /** + * The duck-typed shapes `loadEncryptionContext` harvests: any export with a + * `getEncryptConfig()` method is the client, and any with `tableName` + + * `build()` is a table. Hand-rolled rather than imported from + * `@cipherstash/stack` so the test needs no native module. + */ + const table = (name: string) => + `{ tableName: '${name}', build: () => ({ tableName: '${name}', columns: {} }) }` + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-encrypt-ctx-')) + originalCwd = process.cwd + }) + + afterEach(() => { + process.cwd = originalCwd + vi.restoreAllMocks() + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + it('exits 1 naming the sentinel when it is the only table declared', async () => { + // The scaffold's real shape: the sentinel is both exported AND the sole + // entry in the built encrypt config, because it was passed to `Encryption`. + writeProject( + `export const encryptionClient = { + getEncryptConfig: () => ({ tables: { '${PLACEHOLDER_TABLE_NAME}': {} } }), + } + export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)}`, + ) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + + const { loadEncryptionContext } = await import('../context.js') + await expect(loadEncryptionContext()).rejects.toThrow('process.exit') + + const message = error.mock.calls.flat().join('\n') + expect(message).toContain(PLACEHOLDER_TABLE_NAME) + // Names the cause, not `requireTable`'s "table not found" symptom. + expect(message).toContain('still contains the placeholder table') + expect(message).not.toContain('was not found in the encryption client') + }) + + /** + * #787 review. The guard originally read the harvested EXPORT map, while the + * `db push` / `db validate` guard it mirrors reads `getEncryptConfig().tables`. + * Those disagree in both directions on the same client file, so the two + * commands gave different answers for identical input. + */ + it('fires when the placeholder is passed to Encryption but never exported', async () => { + // The false NEGATIVE. `schemas: [placeholderTable]` with a bare `const` — + // the scaffold minus one `export` keyword. Reading exports, the guard saw + // no tables and fell through to `requireTable`'s "table not found … + // Available: (none)" — the very error this guard exists to replace. + writeProject( + `export const encryptionClient = { + getEncryptConfig: () => ({ tables: { '${PLACEHOLDER_TABLE_NAME}': {} } }), + }`, + ) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + + const { loadEncryptionContext } = await import('../context.js') + await expect(loadEncryptionContext()).rejects.toThrow('process.exit') + + const message = error.mock.calls.flat().join('\n') + expect(message).toContain('still contains the placeholder table') + expect(message).not.toContain('was not found in the encryption client') + }) + + it('stays silent when a stale placeholder export sits beside real configured tables', async () => { + // The false POSITIVE, and the mirror of the case above: the user replaced + // the schema set but left the sentinel `export` behind (or imports their + // real tables without re-exporting them). Reading exports, the guard fired + // and told them to declare columns they had already declared. `db push` + // passes on this same file. + writeProject( + `export const encryptionClient = { + getEncryptConfig: () => ({ tables: { users: {} } }), + } + export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)}`, + ) + + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + const exit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + + const { loadEncryptionContext } = await import('../context.js') + const ctx = await loadEncryptionContext() + + // The silence this test is named for. Without the `exit` spy the guard + // firing here would kill the worker instead of failing an assertion. + expect(exit).not.toHaveBeenCalled() + expect(error.mock.calls.flat().join('\n')).not.toContain( + 'still contains the placeholder table', + ) + expect(ctx.tables.has(PLACEHOLDER_TABLE_NAME)).toBe(true) + }) + + it('allows the sentinel through once a real table sits alongside it', async () => { + // Only the SOLE-placeholder case is the un-replaced scaffold. A user who + // has added real tables must not be blocked by a leftover sentinel. + // + // The sentinel is declared FIRST in the config deliberately: the guard + // reads `configuredTables[0]`, so a placeholder-last fixture passes even + // with the arity check deleted. Ordered this way, dropping + // `configuredTables.length === 1` makes this test fail — which is the only + // thing that pins that clause anywhere in the suite (#787 review follow-up). + writeProject( + `export const encryptionClient = { + getEncryptConfig: () => ({ + tables: { '${PLACEHOLDER_TABLE_NAME}': {}, users: {} }, + }), + } + export const placeholderTable = ${table(PLACEHOLDER_TABLE_NAME)} + export const users = ${table('users')}`, + ) + const error = vi.spyOn(console, 'error').mockImplementation(() => {}) + const exit = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit') + }) + + const { loadEncryptionContext } = await import('../context.js') + const ctx = await loadEncryptionContext() + + expect(exit).not.toHaveBeenCalled() + expect(error.mock.calls.flat().join('\n')).not.toContain( + 'still contains the placeholder table', + ) + expect(ctx.tables.has('users')).toBe(true) + }) +}) 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 45def1ef2..2b8255e79 100644 --- a/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts +++ b/packages/cli/src/commands/encrypt/__tests__/encrypt-v3.test.ts @@ -1,4 +1,9 @@ +import type { + EncryptedColumnInfo, + ResolvedEncryptedColumn, +} from '@cipherstash/migrate' import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { ResolvedLifecycle } from '../lib/resolve-eql.js' // The EQL-v3 lifecycle branches (#648/#649): v3 has no cut-over rename — // `encrypt cutover` must short-circuit with guidance instead of running the @@ -23,9 +28,14 @@ vi.mock('pg', () => ({ }, })) -type ColumnInfo = { column: string; domain: string; version: 2 | 3 } -type ResolvedInfo = ColumnInfo & { via: 'hint' | 'convention' | 'sole' } -type Lifecycle = { info: ResolvedInfo | null; candidates: ColumnInfo[] } +// Imported, never re-declared. These stubs stand in for the real +// `resolveColumnLifecycle`, so a structural copy would let a renamed or added +// field compile on both sides while the commands read something the resolver no +// longer returns — the exact seam `v2-lifecycle-composition.integration.test.ts` +// exercises at runtime, pinned here at compile time (#787 review follow-up). +type ColumnInfo = EncryptedColumnInfo +type ResolvedInfo = ResolvedEncryptedColumn +type Lifecycle = ResolvedLifecycle const migrateMocks = vi.hoisted(() => ({ listEncryptedColumns: vi.fn(async (): Promise => []), @@ -216,6 +226,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 +412,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/__tests__/v2-lifecycle-composition.integration.test.ts b/packages/cli/src/commands/encrypt/__tests__/v2-lifecycle-composition.integration.test.ts new file mode 100644 index 000000000..15ea2b01d --- /dev/null +++ b/packages/cli/src/commands/encrypt/__tests__/v2-lifecycle-composition.integration.test.ts @@ -0,0 +1,202 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +/** + * The seam between lifecycle RESOLUTION and the commands that act on it. + * + * `encrypt-v3.test.ts` stubs `resolveColumnLifecycle` and hand-writes the value + * it returns; `resolve-eql.test.ts` separately proves a pure-v2 table produces + * that value. Both can stay green while the shape passed between them changes, + * because neither runs the real producer into the real consumer — and the + * commands' v2/v3 branch is chosen entirely from that shape. + * + * So here `resolve-eql.js` is NOT mocked at all: `resolveColumnLifecycle`, + * `explainUnresolved`, `pickEncryptedColumn`, `listEncryptedColumns` and + * `columnExists` all run for real, and the pure-v2 verdict is derived from a + * real manifest hint plus a real catalog read rather than asserted into + * existence. Only genuine boundaries are faked: `pg`, the filesystem, prompts, + * and the effectful migrate helpers that write to the database (#787 review + * follow-up). + */ + +const queryMock = vi.hoisted(() => + vi.fn(async (_sql: string, _params?: unknown[]) => ({ + rows: [] as unknown[], + })), +) +vi.mock('pg', () => ({ + default: { + Client: class { + connect = vi.fn(async () => {}) + end = vi.fn(async () => {}) + query = queryMock + }, + }, +})) + +/** + * Spread the real module and override only the functions that TOUCH something — + * the manifest file and the database. Everything resolution actually reasons + * with (`listEncryptedColumns`, `columnExists`, `pickEncryptedColumn`, + * `classifyEqlDomain`) stays real and runs against `queryMock` below. + */ +const migrateMocks = vi.hoisted(() => ({ + readManifest: vi.fn(async () => null as unknown), + countUnencrypted: vi.fn(async () => 0), + progress: vi.fn( + async () => ({ phase: 'cut-over' }) as { phase: string } | null, + ), + appendEvent: vi.fn(async () => {}), + setManifestTargetPhase: vi.fn(async () => {}), + renameEncryptedColumns: vi.fn(async () => {}), + migrateConfig: vi.fn(async () => {}), + activateConfig: vi.fn(async () => {}), + reloadConfig: vi.fn(async () => {}), +})) +vi.mock('@cipherstash/migrate', async (importOriginal) => ({ + ...(await importOriginal()), + ...migrateMocks, +})) + +vi.mock('@clack/prompts', () => ({ + intro: vi.fn(), + outro: vi.fn(), + confirm: vi.fn(async () => true), + isCancel: vi.fn(() => false), + log: { + info: vi.fn(), + success: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + step: vi.fn(), + }, + note: vi.fn(), +})) +vi.mock('@/config/index.js', () => ({ + loadStashConfig: vi.fn(async () => ({ databaseUrl: 'postgres://test' })), +})) +vi.mock('@/commands/db/detect.js', () => ({ + detectDrizzle: vi.fn(() => false), +})) +vi.mock('@/commands/init/utils.js', () => ({ + detectPackageManager: vi.fn(() => 'npm'), + runnerCommand: vi.fn((_pm: string, ref: string) => `npx ${ref}`), +})) +vi.mock('../drizzle-helper.js', () => ({ + scaffoldDrizzleMigration: vi.fn(async ({ name }: { name: string }) => ({ + path: `drizzle/${name}.sql`, + })), +})) +const writeFileMock = vi.hoisted(() => vi.fn()) +vi.mock('node:fs', () => ({ + default: { mkdirSync: vi.fn(), writeFileSync: writeFileMock }, +})) + +import * as p from '@clack/prompts' +import { cutoverCommand } from '../cutover.js' +import { dropCommand } from '../drop.js' + +function spyExit() { + return vi + .spyOn(process, 'exit') + .mockImplementation((() => undefined) as never) +} + +/** Columns the faked catalog reports as EQL-domain typed. Empty = pure v2. */ +let eqlColumns: { column: string; domain_name: string }[] = [] +/** Columns the faked catalog reports as existing at all. */ +let existingColumns: string[] = [] + +/** + * Route on the catalog statements these paths issue. Each is matched on text + * unique to it, and ORDER MATTERS: three of them end in `AS exists`, and + * `columnExists` and `listEncryptedColumns` both embed the same shared + * `to_regclass` expression. Matching loosely silently answers the wrong probe — + * routing the v2 pending-config check into `columnExists` made cutover report + * "no pending EQL configuration" instead of running the ladder. + */ +function fakeCatalog() { + queryMock.mockImplementation(async (sql: string, params?: unknown[]) => { + if (typeof sql !== 'string') return { rows: [] } + // The v2 config machine exists… + if (sql.includes("to_regclass('public.eql_v2_configuration')")) { + return { rows: [{ exists: 'eql_v2_configuration' }] } + } + // …and holds a pending row to promote. + if (sql.includes("state = 'pending'")) return { rows: [{ exists: true }] } + if (sql.includes('typname AS domain_name')) return { rows: eqlColumns } + if (sql.includes('pg_attribute') && sql.includes('AS exists')) { + return { + rows: [{ exists: existingColumns.includes(String(params?.[2])) }], + } + } + return { rows: [] } + }) +} + +beforeEach(() => { + vi.clearAllMocks() + // The hint `encrypt backfill` records for EVERY column, v2 included. It is + // what used to be discarded into a guess, and what must now be ignored + // harmlessly on a table with no v3 columns to mis-claim. + migrateMocks.readManifest.mockResolvedValue({ + tables: { + users: [{ column: 'ssn', encryptedColumn: 'ssn_encrypted' }], + }, + }) + migrateMocks.progress.mockResolvedValue({ phase: 'cut-over' }) + eqlColumns = [] + existingColumns = ['ssn', 'ssn_encrypted'] + fakeCatalog() +}) + +describe('the pure-v2 lifecycle, resolved for real end to end', () => { + it('generates the v2 plaintext drop for a table whose encrypted column is legacy v2', async () => { + const exitSpy = spyExit() + + await dropCommand({ table: 'users', column: 'ssn' }) + + expect(exitSpy).not.toHaveBeenCalled() + + // Reached the v2 ladder: `_plaintext`, not the v3 target ``. + const sql = writeFileMock.mock.calls[0]?.[1] as string + expect(sql).toContain('DROP COLUMN "ssn_plaintext"') + // The v3-only coverage gate must not run on a v2 column. + expect(migrateMocks.countUnencrypted).not.toHaveBeenCalled() + }) + + it('runs the v2 rename and config promotion for a table with no EQL v3 columns', async () => { + migrateMocks.progress.mockResolvedValue({ phase: 'backfilled' }) + + const exitSpy = spyExit() + + await cutoverCommand({ table: 'users', column: 'ssn' }) + + expect(exitSpy).not.toHaveBeenCalled() + expect(migrateMocks.renameEncryptedColumns).toHaveBeenCalled() + expect(migrateMocks.migrateConfig).toHaveBeenCalled() + expect(migrateMocks.activateConfig).toHaveBeenCalled() + expect(p.log.error).not.toHaveBeenCalled() + }) + + it('refuses both commands when the table also holds an unidentifiable EQL v3 column', async () => { + // The mixed table from #772 finding 7. Same manifest hint, same v2 pair — + // the ONLY difference is an unrelated v3 column, which is what makes the + // recorded pairing meaningful and a fall-through a guess. Crossing this + // boundary is what neither existing half of the coverage can see. + eqlColumns = [{ column: 'email_enc', domain_name: 'eql_v3_text_search' }] + const exitSpy = spyExit() + + await dropCommand({ table: 'users', column: 'ssn' }) + await cutoverCommand({ table: 'users', column: 'ssn' }) + + // Refusing is the point: both must exit non-zero rather than act on + // `email_enc`, the unrelated v3 column the sole-EQL-column rule would + // otherwise claim. + expect(exitSpy).toHaveBeenCalledWith(1) + expect(writeFileMock).not.toHaveBeenCalled() + expect(migrateMocks.renameEncryptedColumns).not.toHaveBeenCalled() + const errors = vi.mocked(p.log.error).mock.calls.flat().join('\n') + expect(errors).toContain('is not an EQL v3 column') + expect(errors).toContain('ssn_encrypted') + }) +}) diff --git a/packages/cli/src/commands/encrypt/context.ts b/packages/cli/src/commands/encrypt/context.ts index 1975e2d96..682328eed 100644 --- a/packages/cli/src/commands/encrypt/context.ts +++ b/packages/cli/src/commands/encrypt/context.ts @@ -1,7 +1,11 @@ import fs from 'node:fs' import path from 'node:path' import type { EncryptionClient } from '@cipherstash/stack/encryption' -import { loadStashConfig, type ResolvedStashConfig } from '@/config/index.js' +import { + loadStashConfig, + type ResolvedStashConfig, + requireUsableEncryptConfig, +} from '@/config/index.js' /** * Structural shape of `@cipherstash/stack`'s `EncryptedTable` class. @@ -138,6 +142,14 @@ export async function loadEncryptionContext(): Promise { process.exit(1) } + // The same refusal `stash db push` / `db validate` get from + // `loadEncryptConfig`, applied here because `stash encrypt` does not go + // through that loader. Called, not re-implemented: it guards one file, so the + // two commands must say one thing about it. Without this, `requireTable` + // reported `Table "users" was not found … Available: __stash_placeholder__`, + // naming the symptom and not the cause (#787 review). + requireUsableEncryptConfig(client.getEncryptConfig(), stashConfig.client) + return { stashConfig, client, tables } } diff --git a/packages/cli/src/commands/encrypt/cutover.ts b/packages/cli/src/commands/encrypt/cutover.ts index 676c4e244..1080e9524 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) @@ -94,8 +95,32 @@ export async function cutoverCommand(options: CutoverCommandOptions) { } const state = await progress(client, options.table, options.column) + // `via: 'sole'` means only that this is the table's one remaining EQL + // candidate once the plaintext column itself is excluded — 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 + // (#772 review, finding 7). + // + // Deliberately at TOP LEVEL, not inside the `version === 3` branch, so it + // mirrors `drop.ts` exactly. Equivalent today — `classifyEqlDomain` + // recognises `eql_v3_*` only, so a non-null `info` is always version 3 — + // but the v2 ladder below performs an irreversible rename plus config + // promotion. Were v2 classification restored, or a v4 family added, a + // nested guard would let `cutover` rename on a guess that `drop` refuses + // (#787 review). + if (info?.via === 'sole') { + p.log.error( + `${options.table}.${info.column} (${info.domain}) is the only EQL column left on ${options.table} once "${options.column}" itself is excluded, 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, resolution cannot see it (this command resolves EQL v3 counterparts only): complete that column's v2 lifecycle yourself with the eql_v2 SQL — \`SELECT eql_v2.rename_encrypted_columns();\` plus the config promotion — since no stash command can drive it here. Otherwise record the pairing: re-run \`stash encrypt backfill --table ${options.table} --column ${options.column} --encrypted-column \`.`, + ) + exitCode = 1 + return + } + if (info?.version === 3) { const encryptedColumn = info.column + 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 9beb6953a..d016a0c63 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 only EQL column left on ${options.table} once "${options.column}" itself is excluded, 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, resolution cannot see it (this command resolves EQL v3 counterparts only): complete that column's v2 lifecycle yourself with the eql_v2 SQL, since no stash command can drive it here — and 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 1e5bc6cae..154c5e563 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 @@ -4,17 +4,59 @@ * * `listEncryptedColumns` can no longer emit `version: 2` — a legacy * `eql_v2_encrypted` column is not classified as an EQL column at all, so it - * never reaches this function as a candidate. The post-cutover v2 state (the - * ciphertext renamed onto the plaintext column's own name) therefore arrives - * here as an EMPTY candidate list, which the first guard already falls through - * on. These tests exist so removing the now-unreachable `version === 2` branch - * is provably behaviour-preserving, and so a future v2 sweep cannot delete the - * empty-list guard the v2 lifecycle actually depends on. + * never reaches this function as a candidate. Every pure-v2 table therefore + * arrives here as an EMPTY candidate list, both pre-cutover (`` / + * `_encrypted`) and post-cutover (the ciphertext renamed onto the + * plaintext column's own name), and the first guard falls through on it. + * + * These tests exist so removing the now-unreachable `version === 2` branch is + * provably behaviour-preserving, and so a future v2 sweep cannot delete the + * empty-list guard the v2 lifecycle actually depends on. That guard has to hold + * even when the manifest recorded an `encryptedColumn` — `backfill` records one + * for v2 columns too — which is the `candidates.length > 0` gate on the + * fail-closed path (#787 review). */ 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 => + ({ + // `columnExists` binds [table, schema, column] — it splits schema-qualified + // names and quotes with `format('%I')` so the lookup is case-EXACT. This + // double matches case-exactly for the same reason: a case-insensitive + // fixture would keep passing if the probe regressed to a bare + // `to_regclass($1)`, which case-folds (#787 review). + query: async (_sql: string, params: unknown[]) => ({ + rows: [{ exists: columns.includes(String(params[2])) }], + }), + }) as unknown as pg.ClientBase const v3 = ( column: string, @@ -32,6 +74,15 @@ describe('explainUnresolved', () => { expect(explainUnresolved('users', 'email', [])).toBeNull() }) + it('still falls through when no EQL v3 columns exist BUT a hint was recorded', () => { + // The pure-v2 table. `encrypt backfill` records `encryptedColumn` for v2 + // columns too, so a hint is present on every table backfilled with this + // release — it must not flip the empty-candidate fall-through into a + // refusal, because `cutover` / `drop` in this same build still implement + // the v2 ladder this falls through to (#787 review). + expect(explainUnresolved('users', 'ssn', [], 'ssn_encrypted')).toBeNull() + }) + it('fails closed, naming every candidate, when none is identifiable', () => { const message = explainUnresolved('users', 'email', [ v3('a_enc'), @@ -57,3 +108,169 @@ 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') + }) + + // The pure-v2 shape, and the regression the `candidates.length > 0` gate + // exists to prevent (#787 review). `backfill` records `encryptedColumn` + // unconditionally — v2 included — so EVERY pure-v2 table backfilled with this + // release carries a hint naming a real, existing, non-v3 column. Without the + // gate, `columnExists` returned true, `unresolvedHint` was set, and + // `cutover` / `drop` refused a lifecycle this same build still performs. + it('does not fail closed on a pure-v2 table, where no v3 column can be mis-claimed', async () => { + // No v3 columns at all: just the `ssn` / `ssn_encrypted` v2 pair, which the + // classifier does not see. + listEncryptedColumns.mockResolvedValue([]) + readManifest.mockResolvedValue({ + tables: { users: [{ column: 'ssn', encryptedColumn: 'ssn_encrypted' }] }, + }) + + const { info, candidates, unresolvedHint } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'ssn_encrypted'), + 'users', + 'ssn', + ) + + expect(info).toBeNull() + expect(candidates).toEqual([]) + // The fall-through signal: no hint reported, so `explainUnresolved` returns + // null and the caller reaches its own v2 preconditions. + expect(unresolvedHint).toBeUndefined() + expect( + explainUnresolved('users', 'ssn', candidates, unresolvedHint), + ).toBeNull() + }) + + // #787 review. `columnExists` used to be a local copy using a bare + // `to_regclass($1)`, which PARSES and case-folds its argument — so on a + // Prisma-style `"User"` table the probe reported "missing", the hint was + // treated as stale, and the fail-closed above silently did not fire. It fell + // through to the sole/convention rules and resolved a guess, which is exactly + // what #772 finding 7 exists to prevent. Now delegated to + // `@cipherstash/migrate`'s shared, case-exact probe. + it('fires the fail-closed on a mixed-case table name too', async () => { + listEncryptedColumns.mockResolvedValue([V3_OTHER]) + readManifest.mockResolvedValue({ + tables: { User: [{ column: 'ssn', encryptedColumn: 'ssn_encrypted' }] }, + }) + + const { info, unresolvedHint } = await resolveColumnLifecycle( + clientWithColumns('ssn', 'ssn_encrypted', 'email_enc'), + 'User', + '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 b4e595a10..d2b0ac889 100644 --- a/packages/cli/src/commands/encrypt/lib/resolve-eql.ts +++ b/packages/cli/src/commands/encrypt/lib/resolve-eql.ts @@ -1,4 +1,5 @@ import { + columnExists, type EncryptedColumnInfo, listEncryptedColumns, pickEncryptedColumn, @@ -23,6 +24,26 @@ export interface ResolvedLifecycle { * identifiable — callers name them instead of erroring blind. */ candidates: EncryptedColumnInfo[] + /** + * The manifest's recorded `encryptedColumn` when it named a real column that + * is NOT among `candidates`, AND `candidates` is non-empty — i.e. the pairing + * is on record, the column it names is not an EQL v3 column (typically legacy + * `eql_v2_encrypted`, which `classifyEqlDomain` no longer recognises), and + * there are v3 columns here that a guess could wrongly claim. + * + * 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). + * + * Deliberately NOT set when `candidates` is empty. That is the pure-v2 table + * — a `` / `_encrypted` pair and nothing else — where there is no + * v3 column to mis-claim and so nothing to protect against. The v2 lifecycle + * is still implemented in `cutover.ts` / `drop.ts`, and those commands must + * keep reaching it; failing closed here would refuse a lifecycle this same + * build still performs, and tell the user to downgrade to reach it (#787 + * review). + */ + unresolvedHint?: string } /** @@ -59,23 +80,52 @@ 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. Three 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, is not an EQL v3 column, and the table HAS v3 columns + // — the mixed table. The usual shape is 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: the sole-EQL-column rule 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). Fail closed; + // - the column exists, is not an EQL v3 column, and there are NO v3 columns + // on the table — the pure-v2 table. Nothing here can be mis-claimed, and + // `cutover` / `drop` still implement the v2 ladder, so fall through to it + // exactly as before. Gating on `candidates.length` is what keeps the + // protection above scoped to the mixed table it was written for (#787 + // review). + if ( + hint && + candidates.length > 0 && + (await columnExists(client, table, hint)) + ) { + return { info: null, candidates, unresolvedHint: hint } + } + + return { info: pickEncryptedColumn(candidates, column), candidates } } /** * Explain a failed resolution (`info === null`) to the user, or return * `null` when the failure is fine to fall through to the v2 lifecycle. * - * The one fall-through case is "no EQL columns at all", which the v2 + * The one fall-through case is "no EQL v3 columns at all", which the v2 * phase/config preconditions turn into an accurate error ("not backfilled", * "no pending config", …). Since `classifyEqlDomain` recognises `eql_v3_*` - * only, that case now also covers the post-cutover v2 state — `` was - * renamed onto the ciphertext, and its `eql_v2_encrypted` domain is no longer - * classified, so the column never appears as a candidate. + * only, that case covers every pure-v2 table — both the pre-cutover pair + * (`` / `_encrypted`) and the post-cutover state where `` was + * renamed onto the ciphertext. Neither column is ever a candidate, so a pure-v2 + * table reaches the v2 ladder here regardless of what the manifest recorded; + * a recorded `encryptedColumn` must NOT turn that into a refusal, because + * `cutover.ts` / `drop.ts` in this same build still implement that ladder + * (#787 review). * * A non-empty candidate list therefore means EQL v3 columns exist but none is * identifiable — the caller must fail closed with this message rather than @@ -87,8 +137,25 @@ export function explainUnresolved( table: string, column: string, candidates: readonly EncryptedColumnInfo[], + unresolvedHint?: string, ): string | null { + // "No EQL v3 columns at all" always falls through, even with a recorded hint. + // That is the pure-v2 table, and the v2 ladder in `cutover.ts` / `drop.ts` + // still handles it — the caller's own preconditions produce the accurate + // error. Ordered ahead of the hint branch deliberately: `resolveColumnLifecycle` + // already declines to set `unresolvedHint` on an empty candidate list, and this + // keeps the two agreeing for direct callers of this function (#787 review). if (candidates.length === 0) return null + + // The recorded pairing points at a real column that is not an EQL v3 column, + // on a table that does have v3 columns — almost always a legacy + // `eql_v2_encrypted` counterpart alongside an unrelated v3 one. 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. ${table} also holds EQL v3 columns, and none of them is a confirmed counterpart for ${column}, so this command cannot tell which lifecycle applies and will not guess.\n\nIf that pairing is wrong, correct or remove "encryptedColumn" for ${column} in .cipherstash/migrations.json and re-run. If it is right, ${column} is on the EQL v2 lifecycle, which no stash command can drive on this table — complete it yourself against ${unresolvedHint} with the eql_v2 SQL.` + } + const listed = candidates .map((c) => ` - ${c.column} (${c.domain})`) .join('\n') 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 000000000..c3ddbf548 --- /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 fc0e6aec1..9750af055 100644 --- a/packages/cli/src/commands/init/utils.ts +++ b/packages/cli/src/commands/init/utils.ts @@ -384,9 +384,11 @@ 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 db push\`, + * \`stash db validate\` and \`stash encrypt backfill\` refuse to run and point + * back here. (\`stash encrypt cutover\` / \`drop\` resolve against the database + * and never read this file.) * * This project uses EQL v3. Encrypted columns are concrete Postgres domains * built with the \`types.*\` factories from \`@cipherstash/stack-drizzle\`. @@ -429,9 +431,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 db push\`, \`stash db validate\` +// and \`stash encrypt backfill\` refuse 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 +452,11 @@ 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 db push\`, + * \`stash db validate\` and \`stash encrypt backfill\` refuse to run and point + * back here. (\`stash encrypt cutover\` / \`drop\` resolve against the database + * and never read this file.) * * This project uses EQL v3. Encrypted columns are concrete Postgres domains * built with the \`types.*\` factories from \`@cipherstash/stack/eql/v3\` @@ -483,7 +495,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 db push\`, \`stash db validate\` +// and \`stash encrypt backfill\` refuse 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 7103d7baa..4442c43e0 100644 --- a/packages/cli/src/config/index.ts +++ b/packages/cli/src/config/index.ts @@ -231,12 +231,62 @@ export async function loadEncryptConfig( process.exit(1) } - const config = encryptClient.getEncryptConfig() + return requireUsableEncryptConfig( + encryptClient.getEncryptConfig(), + encryptClientPath, + ) +} + +/** + * Refuse an encryption client that cannot drive any command yet, naming the + * cause. + * + * Shared rather than duplicated because it guards ONE file reached by two + * loaders — `loadEncryptConfig` for `stash db push` / `db validate`, and + * `loadEncryptionContext` for `stash encrypt backfill`. When the copies were + * separate they had already drifted on the nullish-config case, so one command + * named the cause while the other fell through to `requireTable`'s `Table + * "users" was not found … Available: (none)` — the symptom-not-cause message + * this guard exists to replace (#787 review follow-up). + * + * Both refusals are hard exits: there is no partially-usable state here, and + * every caller would otherwise have to re-derive that. + */ +export function requireUsableEncryptConfig( + config: EncryptConfig | undefined, + encryptClientPath: string, +): EncryptConfig { if (!config) { console.error( `Error: Encryption client in ${encryptClientPath} has no initialized encrypt config.`, ) 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. + // + // Read from the built encrypt config, never from the module's export map: a + // scaffold whose `export` keyword was dropped is still un-replaced, while a + // stale `export const placeholderTable` beside real tables that are imported + // rather than re-exported is not (#787 review). + 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 000000000..1a11f6286 --- /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/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 53df05899..76b3903f4 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -9,6 +9,30 @@ export default defineConfig({ resolve: { alias: { '@/': `${resolve(__dirname, './src')}/`, + // `@cipherstash/migrate` publishes `./dist` only, so importing it — or + // spreading `importOriginal()` inside a partial `vi.mock` — makes the + // UNIT suite require a prior workspace build. CI only hides that because + // turbo runs `^build` first; without this alias and without a build, 10 + // files / 177 tests fail to collect with `Failed to resolve entry for + // package "@cipherstash/migrate"` — the transitive `src` importers + // (`status/index.ts`, `db/install.ts`, `encrypt/lib/db-readers.ts`), not + // just the one mocked test. A full-factory `vi.mock` does not help: + // Vite's import analysis fails on the source module's import statement + // before mocking runs. + // + // This removes the `@cipherstash/migrate` build coupling ONLY. The suite + // is still not self-contained: removing `packages/stack/dist` still + // fails 10 files, via TWO independent routes — + // 1. `packages/migrate/src/backfill.ts` imports `@cipherstash/stack`, + // so this alias reaches it transitively; and + // 2. `init/lib/__tests__/introspect.test.ts` imports + // `@cipherstash/stack/eql/v3` directly, never touching migrate. + // Route 2 means decoupling `backfill.ts` would NOT make the suite + // standalone. Closing both needs `stackSourceAlias`, which cannot be + // spread here — its `'@/'` entry (pointing at `packages/stack/src`) + // would clobber this package's own `'@/'`. That is why + // `vitest.shared.ts` is not imported by this config (#787 review). + '@cipherstash/migrate': resolve(__dirname, '../migrate/src/index.ts'), }, }, }) diff --git a/packages/migrate/src/__tests__/version.test.ts b/packages/migrate/src/__tests__/version.test.ts index 96e5cfbb6..140b12738 100644 --- a/packages/migrate/src/__tests__/version.test.ts +++ b/packages/migrate/src/__tests__/version.test.ts @@ -2,6 +2,7 @@ import type { ClientBase } from 'pg' import { describe, expect, it, vi } from 'vitest' import { classifyEqlDomain, + columnExists, detectColumnEqlVersion, type EncryptedColumnInfo, listEncryptedColumns, @@ -202,3 +203,36 @@ describe('resolveEncryptedColumn', () => { }) }) }) + +describe('columnExists', () => { + it('returns true when the catalog reports the column', async () => { + const { client } = mockClient([{ exists: true }]) + expect(await columnExists(client, 'users', 'ssn_encrypted')).toBe(true) + }) + + it('returns false when it does not', async () => { + const { client } = mockClient([{ exists: false }]) + expect(await columnExists(client, 'users', 'gone')).toBe(false) + }) + + it('preserves identifier case — no bare to_regclass($1)', async () => { + // Same guard as `detectColumnEqlVersion` above. A bare `to_regclass($1)` + // parses its argument and case-folds it, so a Prisma-style `"User"` table + // silently resolves to `user` and the probe reports "column missing" for a + // column that plainly exists. In `resolveColumnLifecycle` that turns the + // #772 fail-closed into a silent no-op (#787 review). + const { client, query } = mockClient([{ exists: true }]) + await columnExists(client, 'User', 'ssn_encrypted') + const [sql, values] = query.mock.calls[0] as [string, unknown[]] + expect(sql).toContain("format('%I'") + expect(sql).not.toMatch(/to_regclass\(\$1\)/) + expect(values).toEqual(['User', null, 'ssn_encrypted']) + }) + + it('splits schema-qualified names on the first dot, like qualifyTable', async () => { + const { client, query } = mockClient([{ exists: true }]) + await columnExists(client, 'app.users', 'ssn_encrypted') + const [, values] = query.mock.calls[0] as [string, unknown[]] + expect(values).toEqual(['users', 'app', 'ssn_encrypted']) + }) +}) diff --git a/packages/migrate/src/index.ts b/packages/migrate/src/index.ts index 65bd0db15..026b5f6e5 100644 --- a/packages/migrate/src/index.ts +++ b/packages/migrate/src/index.ts @@ -73,6 +73,7 @@ export { } from './state.js' export { classifyEqlDomain, + columnExists, detectColumnEqlVersion, type EncryptedColumnInfo, type EncryptedColumnResolution, diff --git a/packages/migrate/src/version.ts b/packages/migrate/src/version.ts index 60e1d9d41..88c496170 100644 --- a/packages/migrate/src/version.ts +++ b/packages/migrate/src/version.ts @@ -105,6 +105,38 @@ export async function detectColumnEqlVersion( return domain === undefined ? null : classifyEqlDomain(domain) } +/** + * Whether `columnName` exists on `tableName` at all, whatever its type. + * + * Distinct from {@link detectColumnEqlVersion}, which answers "and is it an EQL + * column?". Callers need the difference to tell a STALE reference (the column + * is gone) from a live one the classifier simply does not recognise — most + * often a legacy `eql_v2_encrypted` counterpart. + * + * Lives here rather than in the CLI so it shares {@link REGCLASS_SQL} with the + * other catalog probes: a hand-rolled `to_regclass($1)` case-folds unquoted + * identifiers and silently reports "missing" for a Prisma-style `"User"` table + * (#787 review). + */ +export async function columnExists( + client: ClientBase, + tableName: string, + columnName: string, +): Promise { + const { schema, table } = splitTableName(tableName) + const { rows } = await client.query<{ exists: boolean }>( + `SELECT EXISTS ( + SELECT 1 FROM pg_attribute + WHERE attrelid = ${REGCLASS_SQL} + AND attname = $3 + AND attnum > 0 + AND NOT attisdropped + ) AS exists`, + [table, schema, columnName], + ) + return rows[0]?.exists === true +} + /** * Every EQL-domain column on a table, classified. The EQL types are * self-describing, so this is the ground truth for "which columns on this diff --git a/scripts/__tests__/cli-vitest-alias.test.mjs b/scripts/__tests__/cli-vitest-alias.test.mjs new file mode 100644 index 000000000..5d93db5e7 --- /dev/null +++ b/scripts/__tests__/cli-vitest-alias.test.mjs @@ -0,0 +1,65 @@ +import { existsSync } from 'node:fs' +import { describe, expect, it } from 'vitest' +import cliVitestConfig from '../../packages/cli/vitest.config.ts' + +/** + * `packages/cli/vitest.config.ts` carries its own alias map, outside the one + * `vitest-shared-alias.test.mjs` guards. It has to: closing the CLI suite's + * remaining build coupling would need `stackSourceAlias`, whose `'@/'` entry + * points at `packages/stack/src` and would clobber the CLI's own `'@/'`. So the + * map is package-local and needs its own guard, for the same reason — an alias + * pointing at a moved file fails LATE, with a "failed to resolve import" naming + * a path nobody wrote. + * + * `@cipherstash/migrate` is pinned by name because it is load-bearing, not + * cosmetic: without it, 10 files / 177 tests fail to COLLECT on an unbuilt + * workspace (the transitive `src` importers, not just the one mocked test). + * `pnpm run test:scripts` runs ahead of the build-dependent suite in CI, so + * this fires before that failure would (#787 review). + */ + +const aliases = cliVitestConfig.resolve.alias + +/** + * Vite also accepts the array form (`[{ find, replacement }]`). This guard + * reads the object-of-strings form; on the array form every check below would + * throw `TypeError: target.endsWith is not a function` from a helper, which + * reads as "the guard is broken" rather than "the config changed shape". + * Fail here instead, naming the offender. + */ +const nonStringTargets = Object.entries(aliases).filter( + ([, target]) => typeof target !== 'string', +) + +/** Directory aliases are written with a trailing slash (`'@/'`). */ +const targets = Object.entries(aliases) + .filter(([, target]) => typeof target === 'string') + .map(([specifier, target]) => [ + specifier, + target.endsWith('/') ? target.slice(0, -1) : target, + ]) + +describe('packages/cli vitest alias map', () => { + it('is the object-of-strings form this guard can read', () => { + expect( + nonStringTargets.map(([specifier]) => specifier), + 'packages/cli/vitest.config.ts switched away from the object-of-strings alias form. Update this guard to walk the new shape — do not delete it.', + ).toEqual([]) + }) + + it('still aliases @cipherstash/migrate to source', () => { + // migrate publishes `./dist` only. Drop this entry — or re-introduce an + // `importOriginal()` spread that needs the built package — and the unit + // suite silently regains a dependency on a prior `turbo run build`. + expect(Object.keys(aliases)).toContain('@cipherstash/migrate') + expect(aliases['@cipherstash/migrate']).toMatch( + /packages[/\\]migrate[/\\]src[/\\]index\.ts$/, + ) + }) + + it.each( + targets, + )('%s resolves to a file that exists', (_specifier, target) => { + expect(existsSync(target), `missing alias target: ${target}`).toBe(true) + }) +}) diff --git a/scripts/__tests__/lib/read-jsonc.mjs b/scripts/__tests__/lib/read-jsonc.mjs new file mode 100644 index 000000000..3ecff8b80 --- /dev/null +++ b/scripts/__tests__/lib/read-jsonc.mjs @@ -0,0 +1,49 @@ +import { readFileSync } from 'node:fs' + +/** + * Parse a JSONC file (`turbo.json` and friends allow comments and trailing + * commas; `JSON.parse` does not). + * + * String-aware by necessity, not fussiness: `turbo.json` opens with + * `"$schema": "https://turbo.build/schema.json"`, so a regex that strips `//` + * without tracking string state eats the URL and yields invalid JSON — or, + * worse, silently valid JSON with the wrong contents. + * + * Extracted from `turbo-skills-inputs.test.mjs` when a second guard needed it. + * Two hand-rolled copies of this would drift, and a subtly wrong copy fails as + * a confusing `SyntaxError` at a byte offset rather than as a clear defect. + */ +export function readJsonc(path) { + const raw = readFileSync(path, 'utf8') + let out = '' + let inString = false + let escaped = false + for (let i = 0; i < raw.length; i++) { + const ch = raw[i] + if (inString) { + out += ch + if (escaped) escaped = false + else if (ch === '\\') escaped = true + else if (ch === '"') inString = false + continue + } + if (ch === '"') { + inString = true + out += ch + continue + } + if (ch === '/' && raw[i + 1] === '/') { + while (i < raw.length && raw[i] !== '\n') i++ + out += '\n' + continue + } + if (ch === '/' && raw[i + 1] === '*') { + i += 2 + while (i < raw.length && !(raw[i] === '*' && raw[i + 1] === '/')) i++ + i++ + continue + } + out += ch + } + return JSON.parse(out.replace(/,(\s*[}\]])/g, '$1')) +} diff --git a/scripts/__tests__/turbo-skills-inputs.test.mjs b/scripts/__tests__/turbo-skills-inputs.test.mjs index 7706ca767..a2413a121 100644 --- a/scripts/__tests__/turbo-skills-inputs.test.mjs +++ b/scripts/__tests__/turbo-skills-inputs.test.mjs @@ -21,45 +21,10 @@ import { existsSync, readdirSync, readFileSync } from 'node:fs' import { join, resolve } from 'node:path' import { fileURLToPath } from 'node:url' import { describe, expect, it } from 'vitest' +import { readJsonc } from './lib/read-jsonc.mjs' const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') -/** Strip comments so `JSON.parse` accepts turbo.json (it is JSONC). */ -function readJsonc(path) { - const raw = readFileSync(path, 'utf8') - let out = '' - let inString = false - let escaped = false - for (let i = 0; i < raw.length; i++) { - const ch = raw[i] - if (inString) { - out += ch - if (escaped) escaped = false - else if (ch === '\\') escaped = true - else if (ch === '"') inString = false - continue - } - if (ch === '"') { - inString = true - out += ch - continue - } - if (ch === '/' && raw[i + 1] === '/') { - while (i < raw.length && raw[i] !== '\n') i++ - out += '\n' - continue - } - if (ch === '/' && raw[i + 1] === '*') { - i += 2 - while (i < raw.length && !(raw[i] === '*' && raw[i + 1] === '/')) i++ - i++ - continue - } - out += ch - } - return JSON.parse(out.replace(/,(\s*[}\]])/g, '$1')) -} - /** Packages whose tsup config copies the repo-root `skills/` into `dist/`. */ function packagesCopyingSkills() { const pkgsDir = join(REPO_ROOT, 'packages') diff --git a/scripts/__tests__/workflow-turbo-build-deps.test.mjs b/scripts/__tests__/workflow-turbo-build-deps.test.mjs new file mode 100644 index 000000000..82386574d --- /dev/null +++ b/scripts/__tests__/workflow-turbo-build-deps.test.mjs @@ -0,0 +1,209 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import yaml from 'js-yaml' +import { describe, expect, it } from 'vitest' +import { readJsonc } from './lib/read-jsonc.mjs' + +/** + * A turbo task declaring `dependsOn: ["^build"]` gets its workspace + * dependencies built before it runs — but ONLY when turbo is the one invoking + * it. Run the same task as a bare package script (`pnpm --filter run + * `) and the dependency graph is skipped silently. + * + * That failure mode is invisible in CI, because a bare step usually still + * passes: some earlier step in the same job built the workspace via its own + * `^build`. The step is then load-bearing on an ordering nobody declared. + * Reorder or delete that earlier step — and they read as independent guards + * for other packages, so they look freely removable — and the bare step fails + * with a module-resolution error (`TS2307`, `Failed to resolve entry for + * package`) that reads as "the code is broken" rather than "you skipped the + * build". + * + * This is not hypothetical: #787 added `typecheck:scaffold` as a bare + * `pnpm --filter stash run typecheck:scaffold` and it went green for exactly + * that reason. The fixtures it typechecks import `@cipherstash/stack/v3`, so + * the step needs that package built; it was routed through turbo in review. + * This test pins that routing so it cannot be quietly "simplified" back. + */ + +const REPO_ROOT = resolve(fileURLToPath(import.meta.url), '../../..') + +/** + * The workflow that must carry the `typecheck:scaffold` step specifically. + * Scoped, because "that step exists" is a claim about this file, not about + * workflows in general. + */ +const SCAFFOLD_WORKFLOW = '.github/workflows/tests.yml' + +/** + * Every workflow, not just `tests.yml`. A bare build-dependent step is the same + * latent trap wherever it runs, and the integration workflows are where it is + * least visible: they build only `stash` (via the shared `integration-setup` + * action) and reach every other package through that one task's `^build`. + * Narrowing the guard to one file left five real bare invocations unchecked + * (#787 review follow-up). + */ +const WORKFLOWS = readdirSync(resolve(REPO_ROOT, '.github/workflows')) + .filter((file) => /\.ya?ml$/.test(file)) + .map((file) => `.github/workflows/${file}`) + .sort() + +/** + * Bare invocations that predate this guard. Each is the same latent trap: it + * passes today only because an earlier step in its job builds the workspace. + * They are recorded rather than ignored so the list can be worked down — do + * not add to it. Route new steps through `turbo run` instead. + */ +const KNOWN_BARE = new Set([ + 'pnpm --filter @cipherstash/prisma-next run typecheck', + 'pnpm --filter @cipherstash/wizard run typecheck', +]) + +const turboJson = readJsonc(resolve(REPO_ROOT, 'turbo.json')) +const rootScripts = + JSON.parse(readFileSync(resolve(REPO_ROOT, 'package.json'), 'utf8')) + .scripts ?? {} + +/** Turbo tasks that build their workspace dependencies first. */ +const buildDependentTasks = new Set( + Object.entries(turboJson.tasks ?? {}) + .filter(([, task]) => (task?.dependsOn ?? []).includes('^build')) + .map(([name]) => name), +) + +/** pnpm's own verbs — never package scripts. */ +const PNPM_VERBS = new Set([ + 'install', + 'exec', + 'dlx', + 'add', + 'why', + 'store', + 'run', +]) + +/** + * The task a command line runs, and whether turbo is the thing running it. + * Returns null when the line runs no recognisable task. + * + * Two shapes matter: + * `pnpm exec turbo run --filter ` / `pnpm turbo ` → routed + * `pnpm [--filter ] [run] ` → bare + */ +function invokedTask(line) { + const turbo = line.match(/\bturbo\b\s+(?:run\s+)?([\w:.-]+)/) + if (turbo) return { task: turbo[1], routed: true, filtered: false } + + const pnpm = line.match( + /\bpnpm\b(?:\s+(?:--filter|-F)\s+\S+|\s+--if-present)*\s+(?:run\s+)?([\w:.-]+)/, + ) + if (!pnpm) return null + const task = pnpm[1] + if (PNPM_VERBS.has(task)) return null + return { task, routed: false, filtered: /\s(?:--filter|-F)\s/.test(line) } +} + +/** + * Root scripts may delegate to turbo themselves (`"test": "turbo test ..."`). + * + * Only ever consult this for an UNFILTERED invocation. `pnpm --filter + * ` runs that package's script, so the root script's delegation says + * nothing about it — reading it either way exempted a genuinely bare + * `pnpm --filter @cipherstash/prisma-next-example test:e2e` purely because the + * root happens to define `"test:e2e": "turbo run test:e2e"` (#787 review + * follow-up). + */ +const rootScriptDelegatesToTurbo = (task) => + typeof rootScripts[task] === 'string' && /\bturbo\b/.test(rootScripts[task]) + +function workflowRunLines(path) { + const doc = yaml.load(readFileSync(resolve(REPO_ROOT, path), 'utf8')) + const lines = [] + for (const [jobName, job] of Object.entries(doc?.jobs ?? {})) { + for (const step of job?.steps ?? []) { + if (typeof step?.run !== 'string') continue + for (const raw of step.run.split('\n')) { + const line = raw.trim() + if (line) lines.push({ jobName, stepName: step.name, line }) + } + } + } + return lines +} + +describe('workflows route build-dependent turbo tasks through turbo', () => { + it('turbo.json declares the tasks this guard protects', () => { + // If `^build` is dropped from these, the guard below silently stops + // checking anything — assert the premise rather than trusting it. + expect(buildDependentTasks).toContain('typecheck:scaffold') + expect(buildDependentTasks).toContain('typecheck') + expect(buildDependentTasks).toContain('build') + }) + + it('typecheck:scaffold is invoked via turbo, never bare', () => { + const invocations = workflowRunLines(SCAFFOLD_WORKFLOW).filter( + ({ line }) => invokedTask(line)?.task === 'typecheck:scaffold', + ) + + // A bare `pnpm --filter stash run typecheck:scaffold` matches + // `invokedTask` but not `routedThroughTurbo`, so it would fail here. + // Deleting the step entirely is caught by this length assertion. + expect( + invocations.length, + `${SCAFFOLD_WORKFLOW} must run typecheck:scaffold — the scaffold fixtures are only typechecked there`, + ).toBeGreaterThan(0) + + for (const { jobName, stepName, line } of invocations) { + expect( + invokedTask(line).routed, + `${SCAFFOLD_WORKFLOW} job "${jobName}" step "${stepName}" runs typecheck:scaffold without turbo:\n ${line}\nThe scaffold fixtures import @cipherstash/stack/v3, so this needs that package BUILT. Use \`pnpm exec turbo run typecheck:scaffold --filter stash\`.`, + ).toBe(true) + } + }) + + it('no new bare invocation of a build-dependent turbo task in any workflow', () => { + const offenders = WORKFLOWS.flatMap((file) => + workflowRunLines(file) + .filter(({ line }) => { + const invoked = invokedTask(line) + if (!invoked || invoked.routed) return false + if (!buildDependentTasks.has(invoked.task)) return false + // A filtered invocation runs the package's own script, so the root + // script's turbo delegation is irrelevant to it. + if (!invoked.filtered && rootScriptDelegatesToTurbo(invoked.task)) + return false + return !KNOWN_BARE.has(line) + }) + .map( + ({ jobName, stepName, line }) => + `${file} / ${jobName} / ${stepName}: ${line}`, + ), + ) + + expect( + offenders, + `These steps run a turbo task declaring \`dependsOn: ["^build"]\` without turbo, so nothing guarantees their workspace dependencies are built. They pass only while an earlier step in the same job happens to build them. Route them through \`pnpm exec turbo run --filter \`.`, + ).toEqual([]) + }) + + it('the grandfathered bare invocations still exist as written', () => { + // KNOWN_BARE entries are matched by exact string. If a step is reworded or + // fixed, its entry goes stale and silently exempts nothing — or worse, + // masks a different line later. Fail so the list gets pruned. + // Searched across every workflow, not just tests.yml: the allowlist now + // exempts lines found anywhere, so a narrower staleness check would let an + // entry keep exempting a step it no longer describes. + const lines = new Set( + WORKFLOWS.flatMap((file) => workflowRunLines(file)).map( + ({ line }) => line, + ), + ) + for (const bare of KNOWN_BARE) { + expect( + lines.has(bare), + `KNOWN_BARE entry is stale — no workflow step runs:\n ${bare}\nRemove it from the allowlist.`, + ).toBe(true) + } + }) +}) diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index aa1c03c27..849763fb9 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -199,7 +199,7 @@ export default defineConfig({ | Option | Required | Default | Purpose | |---|---|---|---| | `databaseUrl` | yes | — | PostgreSQL connection string | -| `client` | no | `./src/encryption/index.ts` | Encryption client, loaded by `db validate`, `schema build`, `encrypt *` | +| `client` | no | `./src/encryption/index.ts` | Encryption client, loaded by `db push`, `db validate`, `encrypt backfill` (`schema build` only *writes* here; `encrypt cutover`/`drop` resolve against the database) | Resolved by walking up from `process.cwd()`, like `tsconfig.json`. `stash init` scaffolds it; `stash eql install` offers to. @@ -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, the commands that read the client file — `db push`, `db validate`, and `encrypt backfill` — exit 1 and point back at it. 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`. @@ -473,7 +473,7 @@ Backfill **detects a `public.eql_v3_*` target column as EQL v3** from its Postgr stash encrypt cutover --table users --column email ``` -**EQL v2 only** — v3 has no cut-over: the application switches to the encrypted column by name. Running this command on a **backfilled** v3 column reports "not applicable" (exit 0) with the next step; before backfill completes it exits 1 and says to finish the backfill (never "switch now" onto a half-populated column). For v2, the preconditions are: the column is in the `backfilled` phase, **and** a pending EQL configuration exists (on a v3-only database — no `eql_v2_configuration` table — it explains that and exits 1). +**EQL v2 only** — v3 has no cut-over: the application switches to the encrypted column by name. Running this command on a **backfilled** v3 column reports "not applicable" (exit 0) with the next step — *provided the encrypted column can be identified*. If it was resolved only by elimination (the table's one remaining EQL column, with no `encryptedColumn` recorded in `.cipherstash/migrations.json` and no `_encrypted` name match), it refuses and exits 1 rather than report an outcome for a column it guessed. `.cipherstash/` is gitignored, so a clone or CI runner without that manifest can hit this on a table whose encrypted column is named unconventionally; re-run `stash encrypt backfill … --encrypted-column ` to record the pairing; before backfill completes it exits 1 and says to finish the backfill (never "switch now" onto a half-populated column). For v2, the preconditions are: the column is in the `backfilled` phase, **and** a pending EQL configuration exists (on a v3-only database — no `eql_v2_configuration` table — it explains that and exits 1). In one transaction it renames `` → `_plaintext` and `_encrypted` → ``, advances the pending config to `encrypting`, activates it, and appends a `cut_over` event. With a Proxy URL configured (`--proxy-url` or `CIPHERSTASH_PROXY_URL`) it then calls `eql_v2.reload_config()` so Proxy picks up the new shape. diff --git a/skills/stash-encryption/SKILL.md b/skills/stash-encryption/SKILL.md index d766079df..da2f11581 100644 --- a/skills/stash-encryption/SKILL.md +++ b/skills/stash-encryption/SKILL.md @@ -822,7 +822,7 @@ try { ## Rolling Encryption Out to Production -> **EQL version note.** The rollout tooling (`stash encrypt *`, `@cipherstash/migrate`) works with **both EQL versions** and detects a column's generation from its Postgres domain type — there is no flag. Detection is one-sided: a `public.eql_v3_*` domain classifies as **v3**; anything else — a plaintext column, or a legacy `eql_v2_encrypted` one — classifies as *unknown* and falls through to the **v2** lifecycle, which is the correct default for a v2 column. Only a v3 column has its version recorded in the migration manifest. The lifecycles differ at the end: **v2** finishes with `stash encrypt cutover` (a rename swap plus a config promotion in `eql_v2_configuration`), then drops `_plaintext`. **v3 has no cut-over and no configuration table** — after backfill you point the application at `_encrypted` *by name*, verify reads, then `stash encrypt drop` generates the drop of the original plaintext ``. Running `encrypt cutover` on a v3 column safely reports "not applicable" with the next step. `stash db push`/`db activate` remain v2-only (they manage `eql_v2_configuration`). +> **EQL version note.** The rollout tooling (`stash encrypt *`, `@cipherstash/migrate`) works with **both EQL versions** and detects a column's generation from its Postgres domain type — there is no flag. Detection is one-sided: a `public.eql_v3_*` domain classifies as **v3**; anything else — a plaintext column, or a legacy `eql_v2_encrypted` one — classifies as *unknown* and falls through to the **v2** lifecycle, which is the correct default for a v2 column — *unless* the table also holds EQL v3 columns and `.cipherstash/migrations.json` records an `encryptedColumn` that is one of the unclassified ones. That combination is ambiguous, so `cutover`/`drop` fail closed and name the recorded column instead of guessing. Only a v3 column has its version recorded in the migration manifest. The lifecycles differ at the end: **v2** finishes with `stash encrypt cutover` (a rename swap plus a config promotion in `eql_v2_configuration`), then drops `_plaintext`. **v3 has no cut-over and no configuration table** — after backfill you point the application at `_encrypted` *by name*, verify reads, then `stash encrypt drop` generates the drop of the original plaintext ``. Running `encrypt cutover` on a v3 column safely reports "not applicable" with the next step. `stash db push`/`db activate` remain v2-only (they manage `eql_v2_configuration`). Adding a fresh encrypted column to a table you don't yet write to is the easy case — declare it in the schema, run the migration, start writing. The harder case is taking an **existing plaintext column with live data** and turning it into an encrypted one without dropping a write or returning the wrong value mid-cutover. @@ -873,7 +873,7 @@ Once dual-writes are recorded as live in `cs_migrations`: |---|---| | `stash encrypt backfill` | Walks the table in keyset-pagination order, encrypts each chunk, writes a single transactional `UPDATE` per chunk plus a `cs_migrations` checkpoint. SIGINT-safe; idempotent re-runs converge. | | Schema rename (**v2 only**) | Update the schema file: drop the `_encrypted` suffix; switch the original column declaration onto the encrypted type. **v3:** there is no rename — leave `_encrypted` under its own name and point the schema/queries at that name instead. | -| `stash encrypt cutover` (**v2 only**) | One transaction: renames `` → `_plaintext`, `_encrypted` → ``, and promotes the `eql_v2_configuration` row `pending` → `active`. Reads of `` through **CipherStash Proxy** are decrypted on the wire; SDK/ORM reads still return ciphertext and need the decrypt path in the next row. **v3:** cutover does not apply — on a backfilled v3 column it reports "not applicable" and exits 0 without changing anything; skip this row. | +| `stash encrypt cutover` (**v2 only**) | One transaction: renames `` → `_plaintext`, `_encrypted` → ``, and promotes the `eql_v2_configuration` row `pending` → `active`. Reads of `` through **CipherStash Proxy** are decrypted on the wire; SDK/ORM reads still return ciphertext and need the decrypt path in the next row. **v3:** cutover does not apply — on a backfilled v3 column it reports "not applicable" and exits 0 without changing anything; skip this row. Exception: if the encrypted column was identified only by elimination (no recorded `encryptedColumn`, no `_encrypted` name match), it exits 1 rather than report an outcome for a guess — record the pairing with `stash encrypt backfill … --encrypted-column `. | | Wire reads through the encryption client | Read paths must decrypt before returning the value to callers (`decryptModel(row, table)` for Drizzle; the Supabase wrapper for Supabase; `decrypt`/`bulkDecryptModels` otherwise). Without this step, reads return raw EQL payloads to end users (a `public.eql_v3_*` jsonb document on v3; an `eql_v2_encrypted` composite on a legacy v2 column). | | Remove dual-write code | The plaintext column is no longer authoritative — **v2:** it is now `_plaintext` (the cutover renamed it); **v3:** it is still the original ``, since nothing was renamed. Either way, delete the dual-write logic once reads are served from the encrypted column. | | `stash encrypt drop` | Emits a migration that drops the plaintext column — and *which* column that is depends on the generation. **v2** (precondition: phase `cut-over`): a plain `ALTER TABLE … DROP COLUMN "_plaintext"`. **v3** (precondition: phase `backfilled`): it drops the **original ``** — there is no `_plaintext` — and the generated SQL is a `DO` block that takes `ACCESS EXCLUSIVE` on the table, re-counts rows with `` set and `_encrypted` NULL *at apply time*, and raises instead of dropping if any remain. Apply with the project's normal migration tooling. | diff --git a/turbo.json b/turbo.json index f86fa0a04..0acfb8b38 100644 --- a/turbo.json +++ b/turbo.json @@ -55,6 +55,10 @@ "dependsOn": ["^build"], "inputs": ["$TURBO_DEFAULT$"] }, + "typecheck:scaffold": { + "dependsOn": ["^build"], + "inputs": ["$TURBO_DEFAULT$"] + }, "test:e2e": { "dependsOn": ["^build", "build"], "inputs": ["$TURBO_DEFAULT$", ".env*"],