diff --git a/.changeset/eql-repair-command.md b/.changeset/eql-repair-command.md new file mode 100644 index 000000000..a47ddd497 --- /dev/null +++ b/.changeset/eql-repair-command.md @@ -0,0 +1,53 @@ +--- +'stash': minor +'@cipherstash/wizard': patch +--- + +Add `stash eql repair --drizzle` — repair a migration directory that `drizzle-kit +generate` filled with an un-runnable in-place `ALTER COLUMN … SET DATA TYPE +`, without generating anything (cipherstash/stack#710). + +```bash +stash eql repair --drizzle # sweep drizzle/ +stash eql repair --drizzle --dry-run # preview; writes nothing +stash eql repair --drizzle --database-url … # leave applied migrations alone +``` + +Until now the only way to run that sweep was `stash eql migration --drizzle`, +which generates a redundant EQL install migration as a side effect purely to +trigger it — the sweep runs before `drizzle-kit generate` has emitted the broken +statement, so recovery meant creating a migration you did not want. `eql repair` +runs the same rewriter and prints the same report (both commands now share one +reporting path, so the two surfaces cannot drift). + +**New: applied-migration awareness.** The sweep has always been unfiltered. That +is harmless for almost every match, because an ALTER to an EQL domain cannot run +— so the migration failed and was never applied. The exception is a `jsonb` +column changed to an EQL domain on an empty table, which applies successfully; +rewriting it afterwards leaves the `.sql` describing a shape the database never +got from it, and a fresh CI or staging database replaying the rewritten file +diverges from the original, silently. + +`eql repair` therefore reads `meta/_journal.json` offline and, given +`--database-url` (or `DATABASE_URL`), the latest `created_at` in +`drizzle.__drizzle_migrations`. A migration is applied when its journal `when` is +at or below that watermark — the same timestamp comparison `drizzle-kit migrate` +makes, hashes being written but never compared. Applied migrations are reported +as their own outcome, left untouched, and the command exits non-zero. Without a +database URL the repair proceeds and warns that applied state could not be +verified; if the check is requested but cannot run, nothing is rewritten. + +A ledger that is not where the probe looked is reported as **unverified**, not as +"nothing applied" — it means either `drizzle-kit migrate` never ran, or +`drizzle.config.ts` overrode `migrations.table` / `migrations.schema` and the +query went to the wrong relation. `--migrations-table <[schema.]table>` names the +ledger for that case; the value must be a plain (optionally schema-qualified) +identifier and is rejected before connecting otherwise. + +An applied migration whose statement the sweep would have skipped regardless — an +undeclared source column, an already-encrypted one, an existing twin — is +reported with that skip reason rather than as an applied-migration refusal. + +`rewriteEncryptedAlterColumns` gained `dryRun`, and its `skip` option now accepts +several paths as well as one. The wizard's copy of the rewriter carries the same +change so the two stay in sync; its own sweep is unaffected. diff --git a/packages/cli/README.md b/packages/cli/README.md index 7bb470e2b..a34d07111 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -308,6 +308,26 @@ npx drizzle-kit migrate --- +### `npx stash eql repair --drizzle` + +Repairs migrations `drizzle-kit generate` emitted with an in-place `ALTER COLUMN … SET DATA TYPE `, which Postgres cannot run (there is no cast from `text`/`numeric` to an EQL domain). Each is rewritten into an additive `ADD COLUMN "_encrypted"` that preserves the source column. + +```bash +npx stash eql repair --drizzle +npx drizzle-kit migrate +``` + +| Flag | Description | +|------|-------------| +| `--drizzle` | Required. Repair a Drizzle migration directory | +| `--out ` | Directory to sweep. Default `drizzle` | +| `--dry-run` | Report what would be rewritten without writing anything | +| `--database-url ` | Leave migrations the database has already applied untouched | + +This is the same sweep `eql migration --drizzle` performs, without generating an install migration you do not need. With `--database-url` it reads `drizzle.__drizzle_migrations` and refuses to rewrite an already-applied migration — doing so would leave the file describing a shape that database never got from it, and a fresh CI or staging database replaying it would silently diverge. Without a URL it proceeds and warns that applied state could not be verified. + +--- + ## Required database permissions Before installing EQL, the CLI verifies that the connected role has: diff --git a/packages/cli/src/bin/main.ts b/packages/cli/src/bin/main.ts index 773e41c3a..16f26f639 100644 --- a/packages/cli/src/bin/main.ts +++ b/packages/cli/src/bin/main.ts @@ -110,6 +110,7 @@ Commands: eql install Scaffold stash.config.ts (if missing) and install EQL extensions eql migration Generate an EQL v3 install migration for your ORM (Drizzle) + eql repair Repair migrations with an un-runnable ALTER COLUMN to an encrypted type eql upgrade Upgrade EQL extensions to the latest version eql status Show EQL installation status @@ -264,6 +265,17 @@ async function runEqlCommand( }) break } + case 'repair': { + const { eqlRepairCommand } = await import('../commands/eql/repair.js') + await eqlRepairCommand({ + drizzle: flags.drizzle, + out: values.out, + dryRun: flags['dry-run'], + databaseUrl: values['database-url'], + migrationsTable: values['migrations-table'], + }) + break + } case 'upgrade': await runUpgrade(flags, values) break diff --git a/packages/cli/src/cli/__tests__/help.test.ts b/packages/cli/src/cli/__tests__/help.test.ts index de0c48906..d3d9df596 100644 --- a/packages/cli/src/cli/__tests__/help.test.ts +++ b/packages/cli/src/cli/__tests__/help.test.ts @@ -53,6 +53,19 @@ describe('renderCommandHelp', () => { expect(out).toContain('auth regions') }) + // `eql repair` is the only command that can consult applied state, so its + // --database-url is load-bearing rather than decorative — help must show it + // alongside the flags that shape the sweep. + it('renders every flag of the standalone repair command', () => { + const out = renderCommandHelp('eql repair', RUNNER) + expect(out).not.toBeNull() + expect(out).toContain('Usage: npx stash eql repair [options]') + expect(out).toContain('--drizzle') + expect(out).toContain('--out ') + expect(out).toContain('--dry-run') + expect(out).toContain('--database-url ') + }) + it('renders a summary-only command without empty Options/Examples', () => { const out = renderCommandHelp('wizard', RUNNER) expect(out).toContain('Usage: npx stash wizard [options]') diff --git a/packages/cli/src/cli/registry.ts b/packages/cli/src/cli/registry.ts index 6eb715609..0e9e52a8a 100644 --- a/packages/cli/src/cli/registry.ts +++ b/packages/cli/src/cli/registry.ts @@ -360,6 +360,54 @@ export const registry: CommandGroup[] = [ DRY_RUN_FLAG, ], }, + { + name: 'eql repair', + summary: + 'Repair migrations drizzle-kit generated with an un-runnable ALTER COLUMN to an encrypted type', + long: [ + 'Sweep an existing Drizzle output directory for in-place', + '`ALTER COLUMN ... SET DATA TYPE ` statements — which cannot run,', + 'because Postgres has no cast from text/numeric to an EQL domain — and rewrite', + 'each into an additive encrypted column that preserves the source column.', + '', + 'This is the same sweep `eql migration --drizzle` runs, without having to', + 'generate an EQL install migration you do not want just to trigger it.', + '', + 'Migrations the database has already applied are reported and left alone:', + 'rewriting one would leave its .sql describing a shape that database never got', + 'from it, so a fresh CI or staging database replaying the file would silently', + 'diverge. Pass --database-url so that check can run; without it the repair', + 'proceeds and warns that applied state could not be verified. If your', + 'drizzle.config.ts overrides `migrations.table` / `migrations.schema`, name', + 'the ledger with --migrations-table — otherwise the check queries the default', + 'relation, finds nothing, and reports applied state as unverified.', + ].join('\n'), + examples: [ + 'eql repair --drizzle', + 'eql repair --drizzle --dry-run', + 'eql repair --drizzle --out db/migrations --database-url postgres://…', + ], + flags: [ + { + name: '--drizzle', + description: 'Repair a Drizzle migration directory.', + }, + { + name: '--out', + value: '', + description: + 'Directory holding the migrations to sweep. Defaults to `drizzle`; set it to match your drizzle.config.ts.', + }, + { + name: '--migrations-table', + value: '<[schema.]table>', + description: + "Drizzle's migration ledger, when drizzle.config.ts overrides `migrations.table` / `migrations.schema`. Defaults to `drizzle.__drizzle_migrations`. Only read with --database-url.", + }, + DRY_RUN_FLAG, + DATABASE_URL_FLAG, + ], + }, { name: 'eql upgrade', summary: 'Upgrade EQL extensions to the latest version', diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index c8c60ca1f..adac6487f 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -1038,10 +1038,19 @@ interface RewriteSweepError extends Error, PartialRewriteResult {} * shipping broken SQL. Statements sitting inside a * SQL comment — or inside a single-quoted string literal, where they are data * rather than SQL — are inert and are neither rewritten nor reported. + * + * `options.skip` names files to leave on disk — they are still READ, because a + * column's current type comes from the whole corpus, but never written. One + * path or many: `eql migration` passes the install migration it just generated, + * `eql repair` passes every migration already applied to the database. + * + * `options.dryRun` computes the identical result without writing anything, so a + * caller can preview a repair — or ask which files WOULD be rewritten — before + * touching migrations that are about to be applied. */ export async function rewriteEncryptedAlterColumns( outDir: string, - options: { skip?: string } = {}, + options: { skip?: string | readonly string[]; dryRun?: boolean } = {}, ): Promise { const entries = await readdir(outDir).catch( (error: NodeJS.ErrnoException) => { @@ -1057,6 +1066,9 @@ export async function rewriteEncryptedAlterColumns( const staged: StagedColumn[] = [] const seen = new Set() const stagedTargets = new Set() + const skipFiles = new Set( + typeof options.skip === 'string' ? [options.skip] : (options.skip ?? []), + ) /** Record a skip once — the strict pass and the broad scan can both find it. */ const skip = (file: string, statement: string, reason: SkipReason): void => { @@ -1084,7 +1096,7 @@ export async function rewriteEncryptedAlterColumns( try { for (const [filePath, original] of contents) { - if (options.skip && filePath === options.skip) continue + if (skipFiles.has(filePath)) continue // Reset the regex's lastIndex — it's stateful on /g ALTER_COLUMN_TO_ENCRYPTED_RE.lastIndex = 0 @@ -1165,7 +1177,7 @@ export async function rewriteEncryptedAlterColumns( ) if (updated !== original) { - await writeFile(filePath, updated, 'utf-8') + if (!options.dryRun) await writeFile(filePath, updated, 'utf-8') rewritten.push(filePath) staged.push(...fileStaged) } diff --git a/packages/cli/src/commands/eql/__tests__/repair.test.ts b/packages/cli/src/commands/eql/__tests__/repair.test.ts new file mode 100644 index 000000000..a3eda6313 --- /dev/null +++ b/packages/cli/src/commands/eql/__tests__/repair.test.ts @@ -0,0 +1,737 @@ +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { CliExit } from '../../../cli/exit.js' +import { messages } from '../../../messages.js' +import { describeSkipReason } from '../../db/rewrite-migrations.js' +import { eqlRepairCommand } from '../repair.js' + +// clack is chrome — silence it and spy on the channels the command reports +// through. `step` is on the real clack `log`; omitting it makes the +// per-statement sweep report throw. +const clack = vi.hoisted(() => ({ + spinnerInstance: { start: vi.fn(), stop: vi.fn() }, + log: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + success: vi.fn(), + step: vi.fn(), + }, + intro: vi.fn(), + note: vi.fn(), + outro: vi.fn(), +})) +vi.mock('@clack/prompts', () => ({ + spinner: vi.fn(() => clack.spinnerInstance), + log: clack.log, + intro: clack.intro, + note: clack.note, + outro: clack.outro, +})) + +// The applied-state probe is the one part of the command that talks to a +// database. Fake the driver rather than the module that uses it, so the real +// query text and the real timestamp comparison stay under test. +const pgMock = vi.hoisted(() => ({ + connect: vi.fn(), + query: vi.fn(), + end: vi.fn(), + connectionStrings: [] as (string | undefined)[], +})) +vi.mock('pg', () => ({ + default: { + Client: vi.fn((config: { connectionString?: string }) => { + pgMock.connectionStrings.push(config?.connectionString) + return { + connect: pgMock.connect, + query: pgMock.query, + end: pgMock.end, + } + }), + }, +})) + +// The sweep stays REAL by default — every other test drives it through actual +// SQL on disk. The spy exists only to reach the "sweep threw" branch, with a +// failure the sweep cannot be made to produce from a fixture (EACCES). +const rewriteMock = vi.hoisted(() => ({ + real: (() => { + throw new Error( + 'rewriteMock.real not initialised: rewrite-migrations mock factory did not run', + ) + }) as typeof import('../../db/rewrite-migrations.js').rewriteEncryptedAlterColumns, + spy: vi.fn(), +})) +vi.mock('../../db/rewrite-migrations.js', async (importOriginal) => { + const actual = + await importOriginal() + rewriteMock.real = actual.rewriteEncryptedAlterColumns + return { ...actual, rewriteEncryptedAlterColumns: rewriteMock.spy } +}) + +/** Stand in for `select max(created_at) from drizzle.__drizzle_migrations`. */ +function mockLatestAppliedMillis(millis: number | null): void { + // node-postgres returns bigint columns as strings; the command must cope. + pgMock.query.mockResolvedValue({ + rows: [{ max_created_at: millis === null ? null : String(millis) }], + }) +} + +beforeEach(() => { + // A stray DATABASE_URL in the developer's shell would silently turn the + // offline tests into online ones. + vi.stubEnv('DATABASE_URL', '') + pgMock.connectionStrings.length = 0 + rewriteMock.spy.mockImplementation(rewriteMock.real) + // `pg`'s Client.connect/end both return promises, and the probe chains off + // end() to swallow a teardown failure. A bare vi.fn() returns undefined, so + // the double has to resolve or that chain throws on a shape the real driver + // never produces. + pgMock.connect.mockResolvedValue(undefined) + pgMock.end.mockResolvedValue(undefined) +}) +afterEach(() => { + vi.unstubAllEnvs() + vi.clearAllMocks() +}) + +/** drizzle-kit's journal, with one entry per tag at the given `when`. */ +function writeJournal( + outDir: string, + entries: { tag: string; when: number }[], +): void { + mkdirSync(join(outDir, 'meta'), { recursive: true }) + writeFileSync( + join(outDir, 'meta', '_journal.json'), + JSON.stringify({ + version: '7', + dialect: 'postgresql', + entries: entries.map(({ tag, when }, idx) => ({ + idx, + version: '7', + when, + tag, + breakpoints: true, + })), + }), + ) +} + +const BROKEN_ALTER = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_text_search";\n' + +/** + * The corpus a real project has when drizzle-kit emits the broken ALTER: an + * earlier migration declaring the plaintext column (the sweep is fail-closed + * and rewrites nothing it cannot see declared), then the ALTER itself. + */ +function writeBrokenCorpus( + outDir: string, + opts: { declaredWhen?: number; brokenWhen?: number } = {}, +): string { + mkdirSync(outDir, { recursive: true }) + writeFileSync( + join(outDir, '0000_declare.sql'), + 'CREATE TABLE "users" ("email" text);\n', + ) + const broken = join(outDir, '0001_encrypt-email.sql') + writeFileSync(broken, BROKEN_ALTER) + writeJournal(outDir, [ + { tag: '0000_declare', when: opts.declaredWhen ?? 1_000 }, + { tag: '0001_encrypt-email', when: opts.brokenWhen ?? 2_000 }, + ]) + return broken +} + +describe('eqlRepairCommand — target selection', () => { + it('exits 1 with an actionable message when no target is given', async () => { + await expect(eqlRepairCommand({})).rejects.toBeInstanceOf(CliExit) + expect(clack.log.error).toHaveBeenCalledWith(messages.eql.repairNeedsTarget) + }) +}) + +describe('eqlRepairCommand — Drizzle', () => { + let tmp: string + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'stash-eql-repair-')) + }) + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }) + }) + + // Fail closed rather than treat "nothing there" as "nothing to repair": a + // mistyped --out (or a drizzle.config.ts writing elsewhere) would otherwise + // report a clean sweep over a directory the command never looked at. + it('exits 1 naming --out when the output directory does not exist', async () => { + const out = join(tmp, 'nope') + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + expect(clack.log.error).toHaveBeenCalledWith(expect.stringContaining(out)) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('--out'), + ) + }) + + // The journal is the only offline record of which migrations exist and when + // they were generated — without it the applied-state check has no input at + // all, so repairing would be guessing. Refuse rather than sweep blind. + it('exits 1 when meta/_journal.json is missing', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + writeFileSync(join(out, '0000_x.sql'), 'SELECT 1;\n') + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('meta/_journal.json'), + ) + }) + + // Same refusal for a journal that is present but unusable — truncated by a + // botched merge, or carrying no `entries` array. Both leave the applied-state + // check with no input, and neither should surface as a raw SyntaxError. + it.each([ + ['unparseable JSON', '{ "entries": ['], + ['no entries array', '{ "version": "7" }'], + ])('exits 1 when the journal is malformed (%s)', async (_label, body) => { + const out = join(tmp, 'drizzle') + mkdirSync(join(out, 'meta'), { recursive: true }) + writeFileSync(join(out, 'meta', '_journal.json'), body) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('meta/_journal.json'), + ) + }) + + // The reason the command exists: repair a broken migration without having to + // generate a redundant EQL install migration to trigger the sweep. + it('rewrites an unapplied broken ALTER COLUMN and reports the file', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).resolves.toBeUndefined() + + const rewritten = readFileSync(broken, 'utf-8') + expect(rewritten).toContain( + 'ALTER TABLE "users" ADD COLUMN "email_encrypted" "public"."eql_v3_text_search";', + ) + expect(rewritten).not.toContain('SET DATA TYPE') + // Add-only: the source column survives for the staged backfill. + expect(rewritten).not.toMatch(/\b(?:DROP|RENAME)\s+COLUMN\b/i) + + const info = clack.log.info.mock.calls.map((c) => String(c[0])) + expect(info.some((msg) => msg.includes('Rewrote 1 migration file'))).toBe( + true, + ) + expect(clack.log.error).not.toHaveBeenCalled() + }) + + // A clean directory must say so out loud. Silence reads as "the command did + // not run", and sends the user back to `eql migration --drizzle` — the + // redundant-install-migration workaround this command exists to remove. + it('reports success and changes nothing when there is nothing to repair', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + const clean = join(out, '0000_clean.sql') + const contents = 'CREATE TABLE "users" ("email" text);\n' + writeFileSync(clean, contents) + writeJournal(out, [{ tag: '0000_clean', when: 1_000 }]) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).resolves.toBeUndefined() + + expect(readFileSync(clean, 'utf-8')).toBe(contents) + expect(clack.log.success).toHaveBeenCalledWith( + messages.eql.repairNothingToDo, + ) + expect(clack.log.error).not.toHaveBeenCalled() + }) + + // Same fail-closed posture as `eql migration --drizzle`: a statement the + // sweep could not rewrite still fails at migrate time, so exiting 0 would + // tell CI the repair had succeeded. + it('exits 1 and reports the guidance when the sweep leaves a statement behind', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + // Hand-authored `SET DATA TYPE ... USING`: outside the strict matcher, but + // the broad scan flags it as a near-miss. + const nearMiss = join(out, '0000_nearmiss.sql') + writeFileSync( + nearMiss, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;\n', + ) + writeJournal(out, [{ tag: '0000_nearmiss', when: 1_000 }]) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + + expect(readFileSync(nearMiss, 'utf-8')).toContain('SET DATA TYPE') + const stepped = clack.log.step.mock.calls.map((c) => String(c[0])) + expect( + stepped.some((msg) => msg.includes('falls outside the strict matcher')), + ).toBe(true) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('unsafe or unverified SQL'), + ) + }) + + // --dry-run must be a true preview: the whole point is inspecting the repair + // before letting it touch migrations that are about to be applied. + it('writes nothing under --dry-run but names the file it would rewrite', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + + await expect( + eqlRepairCommand({ drizzle: true, out, dryRun: true }), + ).resolves.toBeUndefined() + + expect(readFileSync(broken, 'utf-8')).toBe(BROKEN_ALTER) + const info = clack.log.info.mock.calls.map((c) => String(c[0])) + expect(info.some((msg) => msg.includes('Would rewrite 1 migration'))).toBe( + true, + ) + const stepped = clack.log.step.mock.calls.map((c) => String(c[0])) + expect(stepped.some((msg) => msg.includes(broken))).toBe(true) + }) + + /** + * A sweep that throws part way through has already written some rewrites, so + * the user needs the same partial-work report `eql migration --drizzle` gives + * (#786) — not an unhandled rejection. Both commands render it through the + * same helper. + */ + it('reports a sweep that threw, including the work it had already done', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + writeJournal(out, [{ tag: '0000_x', when: 1_000 }]) + rewriteMock.spy.mockRejectedValueOnce( + Object.assign(new Error('EACCES: permission denied'), { + rewritten: [join(out, '0000_x.sql')], + skipped: [], + staged: [], + }), + ) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + + const info = clack.log.info.mock.calls.map((c) => String(c[0])).join('\n') + expect(info).toContain('before the sweep stopped') + expect(clack.log.warn).toHaveBeenCalledWith( + expect.stringContaining('Could not sweep'), + ) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('unsafe or unverified SQL'), + ) + }) + + // `describeStagedReconciliation` is written in the past tense — "the database + // now has" — because a real sweep has already written the twin. Under + // --dry-run nothing was written, so printing it verbatim would send the user + // reconciling a schema against a column that does not exist. + it('does not claim a twin exists under --dry-run', async () => { + const out = join(tmp, 'drizzle') + writeBrokenCorpus(out) + + await eqlRepairCommand({ drizzle: true, out, dryRun: true }) + + const warnings = clack.log.warn.mock.calls + .map((c) => String(c[0])) + .join('\n') + expect(warnings).not.toContain('the database now has') + const info = clack.log.info.mock.calls.map((c) => String(c[0])).join('\n') + expect(info).toContain(messages.eql.repairDryRunStaged(1)) + }) +}) + +/** + * The capability `eql migration --drizzle` cannot offer, and the reason this + * command exists as its own entry point. + * + * A matching statement is un-runnable by construction, so the migration almost + * always failed on apply and is safe to rewrite. The exception is a `jsonb` + * column changed to a v3 domain on an EMPTY table: the base types are + * compatible and the envelope CHECK has nothing to reject, so it applies. Once + * it has, rewriting the .sql leaves it describing a shape the original database + * never got from it — a fresh CI or staging database replaying the rewritten + * file diverges from dev, silently. + */ +describe('eqlRepairCommand — applied migrations', () => { + let tmp: string + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'stash-eql-repair-')) + }) + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }) + }) + + it('refuses to rewrite a migration the database has already applied', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out, { brokenWhen: 2_000 }) + // Drizzle's applied-check is `folderMillis <= max(created_at)`, so a + // watermark at the broken migration's own `when` means it ran. + mockLatestAppliedMillis(2_000) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).rejects.toBeInstanceOf(CliExit) + + // Untouched on disk — the whole point. + expect(readFileSync(broken, 'utf-8')).toBe(BROKEN_ALTER) + + const reported = [ + ...clack.log.warn.mock.calls, + ...clack.log.error.mock.calls, + ...clack.log.step.mock.calls, + ] + .map((c) => String(c[0])) + .join('\n') + // A distinct outcome, not a silent skip: names the file, says it is + // applied, names the hazard, and points at the staged lifecycle. + expect(reported).toContain(broken) + expect(reported.toLowerCase()).toContain('already applied') + expect(reported).toContain('stash encrypt') + expect(reported).toContain(messages.eql.repairAppliedHazard) + }) + + // The discriminating half. Without this, an implementation that refused every + // migration whenever a database was reachable would pass the test above — and + // would be useless in the flow the command exists for. + it('still repairs a migration generated after the applied watermark', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out, { + declaredWhen: 1_000, + brokenWhen: 2_000, + }) + // Only the declaring migration ran. + mockLatestAppliedMillis(1_000) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).resolves.toBeUndefined() + + expect(readFileSync(broken, 'utf-8')).toContain( + 'ADD COLUMN "email_encrypted"', + ) + }) + + /** + * The offline default, stated as a test so it cannot drift: proceed, but say + * plainly that applied state was NOT verified. + * + * Refusing without a URL would be the fail-closed reading, and it is wrong + * here — the overwhelmingly common case is a broken migration that could not + * have applied, in a project whose database may not be reachable from where + * the repair runs, so a refusal would make the command useless in its own + * intended flow. The warning names the one genuinely unsafe case and how to + * get the check. + */ + it('warns that applied state is unverified without a database URL, and repairs anyway', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).resolves.toBeUndefined() + + expect(readFileSync(broken, 'utf-8')).toContain( + 'ADD COLUMN "email_encrypted"', + ) + expect(clack.log.warn).toHaveBeenCalledWith( + messages.eql.repairAppliedUnverified, + ) + // No connection attempt at all — the command stays offline. + expect(pgMock.connect).not.toHaveBeenCalled() + }) + + // DATABASE_URL is the documented second tier of the URL resolver, so the + // check must run off it too — a user with it exported gets the safety without + // repeating it on the command line. + it('runs the applied check from DATABASE_URL when no flag is passed', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out, { brokenWhen: 2_000 }) + mockLatestAppliedMillis(2_000) + vi.stubEnv('DATABASE_URL', 'postgres://user:pw@localhost:5432/from-env') + + await expect( + eqlRepairCommand({ drizzle: true, out }), + ).rejects.toBeInstanceOf(CliExit) + + expect(readFileSync(broken, 'utf-8')).toBe(BROKEN_ALTER) + expect(pgMock.connectionStrings).toEqual([ + 'postgres://user:pw@localhost:5432/from-env', + ]) + }) + + // Pins the mechanism, not just the outcome: drizzle decides on the timestamp + // watermark in its own ledger. A hash comparison would be modelling something + // drizzle does not do (`pg-core/dialect.js:62` writes the hash and never + // reads it), and would re-run migrations drizzle considers done. + it('reads the watermark from drizzle.__drizzle_migrations', async () => { + const out = join(tmp, 'drizzle') + writeBrokenCorpus(out) + mockLatestAppliedMillis(1_000) + + await eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }) + + const sql = String(pgMock.query.mock.calls[0]?.[0]) + // Quoted, because the relation is now parameterised by --migrations-table + // and reaches the query as text. + expect(sql).toContain('"drizzle"."__drizzle_migrations"') + expect(sql).toContain('created_at') + expect(sql).not.toContain('hash') + }) + + // An empty (or absent-but-readable) ledger means nothing has run yet, so + // everything is repairable. Reported rather than silent, because "no rows" + // and "check did not run" must not look the same to the user. + it('treats an empty ledger as nothing applied', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + mockLatestAppliedMillis(null) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).resolves.toBeUndefined() + + expect(readFileSync(broken, 'utf-8')).toContain( + 'ADD COLUMN "email_encrypted"', + ) + expect(clack.log.info).toHaveBeenCalledWith( + messages.eql.repairNothingApplied, + ) + }) + + /** + * An ABSENT ledger is ambiguous in a way an empty one is not. It means either + * `drizzle-kit migrate` never ran here, or the project set `migrations.table` + * / `migrations.schema` in drizzle.config.ts and the probe looked in the + * wrong place. The command cannot tell those apart, so it must not report the + * confident "nothing applied" — that is the fail-open case: a project with a + * custom ledger gets told every migration is repairable while its database + * has applied plenty. + */ + it.each([ + ['undefined_table', '42P01'], + ['invalid_schema_name', '3F000'], + ])('does not claim nothing is applied when the ledger (%s) is absent', async (_l, code) => { + const out = join(tmp, 'drizzle') + writeBrokenCorpus(out) + pgMock.query.mockRejectedValue( + Object.assign(new Error('relation does not exist'), { code }), + ) + + await eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }) + + expect(clack.log.info).not.toHaveBeenCalledWith( + messages.eql.repairNothingApplied, + ) + const warned = clack.log.warn.mock.calls.map((c) => String(c[0])).join('\n') + // Names where it looked, and the config key that would move it — a + // warning the user cannot act on is not much better than silence. + expect(warned).toContain('drizzle.__drizzle_migrations') + expect(warned).toContain('--migrations-table') + }) + + /** + * Applied files carry TWO kinds of statement, and they need different words. + * + * One the rewriter would have repaired — refusing it is about the applied + * hazard, and `repairAppliedHazard` is the right explanation. One it would + * have left alone regardless (here `source-unknown`: nothing in the corpus + * declares the column) — its applied-ness is irrelevant, and the actionable + * thing is the skip reason. Reporting the second under the hazard banner + * swaps guidance the user can act on for an explanation that does not apply. + */ + it('gives an applied near-miss its skip guidance, not the rewrite hazard', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + // No CREATE TABLE anywhere, so the sweep cannot prove the source type. + const undeclared = join(out, '0000_encrypt-orphan.sql') + writeFileSync( + undeclared, + 'ALTER TABLE "ghosts" ALTER COLUMN "note" SET DATA TYPE "undefined"."eql_v3_text_search";\n', + ) + writeJournal(out, [{ tag: '0000_encrypt-orphan', when: 1_000 }]) + mockLatestAppliedMillis(1_000) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).rejects.toBeInstanceOf(CliExit) + + const reported = [ + ...clack.log.warn.mock.calls, + ...clack.log.error.mock.calls, + ...clack.log.step.mock.calls, + ] + .map((c) => String(c[0])) + .join('\n') + expect(reported).toContain(describeSkipReason('source-unknown')) + expect(reported).not.toContain(messages.eql.repairAppliedHazard) + }) + + /** + * The actionable half of the absent-ledger warning. Without this the warning + * names a flag that does nothing, and a project with a custom ledger has no + * way to get the check at all. + */ + it('queries the ledger named by --migrations-table', async () => { + const out = join(tmp, 'drizzle') + writeBrokenCorpus(out) + mockLatestAppliedMillis(1_000) + + await eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + migrationsTable: 'audit.applied_migrations', + }) + + const sql = String(pgMock.query.mock.calls[0]?.[0]) + expect(sql).toContain('"audit"."applied_migrations"') + expect(sql).not.toContain('__drizzle_migrations') + }) + + /** + * The relation is the one part of the query built from user text. It is + * quoted, so it cannot break out — but a value that is not a plain + * `[schema.]table` becomes a quoted identifier no database has, and the + * resulting "relation does not exist" would be reported as an ABSENT ledger: + * a typo would silently downgrade the check the flag was passed to get. + * Reject it before connecting instead. + */ + it.each([ + ['a statement terminator', '__drizzle_migrations; drop table users --'], + ['too many parts', 'db.drizzle.__drizzle_migrations'], + ['an empty part', 'drizzle.'], + ['a quoted identifier', '"drizzle"."__drizzle_migrations"'], + ])('rejects a --migrations-table containing %s', async (_label, migrationsTable) => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + migrationsTable, + }), + ).rejects.toBeInstanceOf(CliExit) + + // Never reached the database, and never touched the migration. + expect(pgMock.query).not.toHaveBeenCalled() + expect(readFileSync(broken, 'utf-8')).toBe(BROKEN_ALTER) + }) + + // A bare table name is the common case — drizzle's `migrations.table` without + // a `migrations.schema`. It must resolve via search_path, not be forced into + // the `drizzle` schema. + it('accepts an unqualified --migrations-table', async () => { + const out = join(tmp, 'drizzle') + writeBrokenCorpus(out) + mockLatestAppliedMillis(1_000) + + await eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + migrationsTable: 'my_migrations', + }) + + expect(String(pgMock.query.mock.calls[0]?.[0])).toContain('"my_migrations"') + }) + + // An absent ledger still REPAIRS — the overwhelmingly common cause is that + // drizzle-kit migrate never ran, and refusing there would make the command + // useless in the flow it exists for. Only the claim of certainty is dropped. + it('still repairs when the ledger is absent', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + pgMock.query.mockRejectedValue( + Object.assign(new Error('relation does not exist'), { code: '42P01' }), + ) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).resolves.toBeUndefined() + + expect(readFileSync(broken, 'utf-8')).toContain( + 'ADD COLUMN "email_encrypted"', + ) + }) + + /** + * The asymmetry that matters: passing --database-url is a request to be + * protected from rewriting applied migrations. If the probe cannot answer, + * carrying on as if nothing were applied would hand the user the exact drift + * they asked to avoid, having told them the check was on. + */ + it('exits 1 without rewriting anything when the applied check itself fails', async () => { + const out = join(tmp, 'drizzle') + const broken = writeBrokenCorpus(out) + pgMock.connect.mockRejectedValue( + Object.assign(new Error('connection refused'), { code: 'ECONNREFUSED' }), + ) + + await expect( + eqlRepairCommand({ + drizzle: true, + out, + databaseUrl: 'postgres://user:pw@localhost:5432/app', + }), + ).rejects.toBeInstanceOf(CliExit) + + expect(readFileSync(broken, 'utf-8')).toBe(BROKEN_ALTER) + expect(clack.log.error).toHaveBeenCalledWith( + expect.stringContaining('connection refused'), + ) + // Every path out of the command closes the banner it opened. + expect(clack.outro).toHaveBeenCalledWith('Repair incomplete.') + }) +}) diff --git a/packages/cli/src/commands/eql/applied.ts b/packages/cli/src/commands/eql/applied.ts new file mode 100644 index 000000000..0cddf729e --- /dev/null +++ b/packages/cli/src/commands/eql/applied.ts @@ -0,0 +1,113 @@ +/** + * drizzle's own migration ledger: schema `drizzle`, table + * `__drizzle_migrations`, one row per applied migration carrying `hash` and + * `created_at`. These are drizzle-kit's defaults (`migrations.schema` / + * `migrations.table` in drizzle.config.ts); a project that overrides them must + * say so, because the probe cannot discover it — see {@link LEDGER_ABSENT}. + */ +export const DEFAULT_MIGRATIONS_RELATION = 'drizzle.__drizzle_migrations' + +/** Postgres SQLSTATEs meaning "the relation isn't there". */ +const NO_LEDGER = new Set([ + '42P01', // undefined_table + '3F000', // invalid_schema_name +]) + +/** The ledger exists and is empty — nothing has been applied. A real answer. */ +export const NOTHING_APPLIED = Symbol('nothing-applied') + +/** + * The ledger is not where we looked. NOT the same answer as {@link + * NOTHING_APPLIED}: it means either `drizzle-kit migrate` never ran here, or + * the project overrode `migrations.table` / `migrations.schema` and the probe + * queried the wrong relation. Those are indistinguishable from the SQLSTATE, so + * the caller must report the state as unverified rather than claim nothing has + * been applied — the second case would otherwise rewrite applied migrations + * while telling the user they were safe. + */ +export const LEDGER_ABSENT = Symbol('ledger-absent') + +/** + * Is `relation` a plain `table` or `schema.table` of unquoted identifiers? + * + * The relation is quoted before it reaches SQL, so this is not what stops + * injection — {@link quoteRelation} does. It stops something quieter: a value + * that is not a plain identifier pair becomes a quoted relation no database + * has, whose `undefined_table` would be read as {@link LEDGER_ABSENT}. A typo + * would then silently downgrade the very check the caller asked for. + */ +export function isValidRelation(relation: string): boolean { + const parts = relation.split('.') + if (parts.length < 1 || parts.length > 2) return false + return parts.every((part) => /^[A-Za-z_][A-Za-z0-9_$]*$/.test(part)) +} + +/** + * Render `[schema.]table` as quoted SQL identifiers. + * + * The relation reaches the query as text, so it is quoted rather than + * interpolated bare: each dot-separated part is wrapped in double quotes with + * any embedded quote doubled, which is injection-safe by construction. The + * caller validates the shape separately, so a malformed value is rejected with + * a useful message instead of becoming a quoted identifier that cannot exist. + */ +function quoteRelation(relation: string): string { + return relation + .split('.') + .map((part) => `"${part.replace(/"/g, '""')}"`) + .join('.') +} + +/** + * The `created_at` of the most recently applied migration, in epoch + * milliseconds; {@link NOTHING_APPLIED} when the ledger exists and is empty; or + * {@link LEDGER_ABSENT} when it is not where we looked. + * + * **Why a single watermark rather than a per-migration lookup.** drizzle's + * applied-check is timestamp-based, not hash-based: `drizzle-orm@0.45.2` + * `pg-core/dialect.js:62` is `if (!lastDbMigration || Number( + * lastDbMigration.created_at) < migration.folderMillis)`, with the hash written + * on insert at `:67` and never read for the decision, and `folderMillis` coming + * from the journal's `when` (`migrator.js:22`). `drizzle-kit migrate` delegates + * to that same code for every Postgres driver. So a migration is applied iff + * its `when` is at or below this watermark, and comparing hashes would be + * modelling a mechanism drizzle does not have. + * + * `pg` is imported lazily: repair is an offline command in every run that does + * not pass a database URL, and it should not pay for the driver to find that + * out. + */ +export async function latestAppliedMillis( + databaseUrl: string, + relation: string = DEFAULT_MIGRATIONS_RELATION, +): Promise { + const { default: pg } = await import('pg') + const client = new pg.Client({ connectionString: databaseUrl }) + try { + await client.connect() + const result = await client.query<{ max_created_at: string | null }>( + `select max(created_at) as max_created_at from ${quoteRelation(relation)}`, + ) + // `created_at` is a bigint, which node-postgres returns as a string to + // avoid a lossy Number conversion. The values are epoch milliseconds, far + // inside Number.MAX_SAFE_INTEGER, so converting here is safe. + const raw = result.rows[0]?.max_created_at + if (raw === null || raw === undefined) return NOTHING_APPLIED + return Number(raw) + } catch (error) { + // A missing relation is a partial answer — see LEDGER_ABSENT for why it is + // not "nothing applied". Every other error (auth, network, permissions) is + // a check that did NOT happen, and must not be quietly downgraded to "all + // clear" — it propagates to the caller. + if ( + typeof error === 'object' && + error !== null && + NO_LEDGER.has(String((error as { code?: unknown }).code)) + ) { + return LEDGER_ABSENT + } + throw error + } finally { + await client.end().catch(() => undefined) + } +} diff --git a/packages/cli/src/commands/eql/journal.ts b/packages/cli/src/commands/eql/journal.ts new file mode 100644 index 000000000..8d8f5f1d8 --- /dev/null +++ b/packages/cli/src/commands/eql/journal.ts @@ -0,0 +1,74 @@ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' + +/** + * One migration as drizzle-kit records it in `meta/_journal.json`. + * + * Only the two fields the applied-state check needs are modelled. `when` is the + * generation timestamp in epoch milliseconds — drizzle names it `folderMillis` + * once it reaches the migrator, and it is the value written into + * `drizzle.__drizzle_migrations.created_at` on a successful apply. + */ +export interface JournalEntry { + /** Migration tag, e.g. `0001_encrypt-email`. The `.sql` file is `.sql`. */ + tag: string + /** Generation timestamp in epoch milliseconds (`folderMillis`). */ + when: number +} + +/** Thrown when the journal is absent or unreadable as a drizzle journal. */ +export class JournalError extends Error {} + +/** `/meta/_journal.json` — drizzle-kit's fixed location. */ +export function journalPath(outDir: string): string { + return join(outDir, 'meta', '_journal.json') +} + +/** + * Read drizzle-kit's migration journal. + * + * Throws a {@link JournalError} when the file is missing, unparseable, or does + * not carry an `entries` array. Every one of those means the caller cannot tell + * which migrations exist or when they were generated, and a repair that cannot + * answer that must refuse rather than guess. + */ +export async function readJournal(outDir: string): Promise { + const path = journalPath(outDir) + let raw: string + try { + raw = await readFile(path, 'utf-8') + } catch (error) { + throw new JournalError( + `Could not read ${path}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch (error) { + throw new JournalError( + `Could not parse ${path}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + + if ( + typeof parsed !== 'object' || + parsed === null || + !Array.isArray((parsed as { entries?: unknown }).entries) + ) { + throw new JournalError(`${path} has no "entries" array.`) + } + + const entries: JournalEntry[] = [] + for (const entry of (parsed as { entries: unknown[] }).entries) { + if (typeof entry !== 'object' || entry === null) continue + const { tag, when } = entry as { tag?: unknown; when?: unknown } + // A malformed individual entry is not a malformed journal: skip it rather + // than abort, so one bad row cannot block repairing the rest. It simply + // never matches a file, and an unmatched file is treated as unapplied. + if (typeof tag !== 'string' || typeof when !== 'number') continue + entries.push({ tag, when }) + } + return entries +} diff --git a/packages/cli/src/commands/eql/migration.ts b/packages/cli/src/commands/eql/migration.ts index e7df41439..1a003c4b6 100644 --- a/packages/cli/src/commands/eql/migration.ts +++ b/packages/cli/src/commands/eql/migration.ts @@ -6,13 +6,11 @@ import { MIGRATIONS_SCHEMA_SQL } from '@cipherstash/migrate' import * as p from '@clack/prompts' import { CliExit } from '@/cli/exit.js' import { printNextSteps, SAFE_MIGRATION_NAME } from '@/commands/db/install.js' +import { rewriteEncryptedAlterColumns } from '@/commands/db/rewrite-migrations.js' import { - describeSkipReason, - describeStagedReconciliation, - isPartialRewriteResult, - type PartialRewriteResult, - rewriteEncryptedAlterColumns, -} from '@/commands/db/rewrite-migrations.js' + reportSweepFailure, + reportSweepResult, +} from '@/commands/eql/sweep-report.js' import { detectPackageManager, execArgv, @@ -267,64 +265,13 @@ async function generateDrizzleEqlMigration( // it again at the closing note (below) — not just inline here. let sweepIncomplete = false try { - const { rewritten, skipped, staged } = await rewriteEncryptedAlterColumns( - outDir, - { skip: migrationPath }, + sweepIncomplete = reportSweepResult( + await rewriteEncryptedAlterColumns(outDir, { skip: migrationPath }), ) - if (rewritten.length > 0) { - p.log.info( - `Rewrote ${rewritten.length} migration file(s) to add staged encrypted columns while preserving the source columns:`, - ) - for (const file of rewritten) p.log.step(` - ${file}`) - } - // The rewrite repaired SQL only, so schema.ts and the drizzle-kit snapshot - // now disagree with the database — and `drizzle-kit generate` cannot see it - // (#836, item 2). Warn, rather than exit non-zero: the swept SQL is valid - // and additive, and the reconciliation is the user's editorial call. - if (staged.length > 0) { - p.log.warn(describeStagedReconciliation(staged).join('\n')) - } - if (skipped.length > 0) { - sweepIncomplete = true - p.log.warn( - `Found ${skipped.length} ALTER-to-encrypted statement(s) the sweep left alone. Review and fix them before running your migrations:`, - ) - for (const { file, statement, reason } of skipped) { - p.log.step(` - ${file}: ${statement}`) - p.log.step(` ${describeSkipReason(reason)}`) - } - } } catch (error) { // Advisory: the install migration itself is already written and valid. sweepIncomplete = true - const partial: PartialRewriteResult = isPartialRewriteResult(error) - ? error - : {} - if (partial.rewritten && partial.rewritten.length > 0) { - p.log.info( - `Rewrote ${partial.rewritten.length} migration file(s) before the sweep stopped:`, - ) - for (const file of partial.rewritten) p.log.step(` - ${file}`) - } - // A partial sweep still staged real twins, so the same three-way divergence - // already exists for them. - if (partial.staged && partial.staged.length > 0) { - p.log.warn(describeStagedReconciliation(partial.staged).join('\n')) - } - if (partial.skipped && partial.skipped.length > 0) { - p.log.warn( - `Found ${partial.skipped.length} ALTER-to-encrypted statement(s) the sweep left alone. Review and fix them before running your migrations:`, - ) - for (const { file, statement, reason } of partial.skipped) { - p.log.step(` - ${file}: ${statement}`) - p.log.step(` ${describeSkipReason(reason)}`) - } - } - p.log.warn( - `Could not sweep ${outDir} for unsafe ALTER COLUMN statements: ${ - error instanceof Error ? error.message : String(error) - }`, - ) + reportSweepFailure(outDir, error) } if (sweepIncomplete) { diff --git a/packages/cli/src/commands/eql/repair.ts b/packages/cli/src/commands/eql/repair.ts new file mode 100644 index 000000000..29d8cee25 --- /dev/null +++ b/packages/cli/src/commands/eql/repair.ts @@ -0,0 +1,265 @@ +import { existsSync } from 'node:fs' +import { join, resolve } from 'node:path' +import * as p from '@clack/prompts' +import { CliExit } from '@/cli/exit.js' +import { + type RewriteResult, + rewriteEncryptedAlterColumns, +} from '@/commands/db/rewrite-migrations.js' +import { + DEFAULT_MIGRATIONS_RELATION, + isValidRelation, + LEDGER_ABSENT, + latestAppliedMillis, + NOTHING_APPLIED, +} from '@/commands/eql/applied.js' +import { + type JournalEntry, + JournalError, + readJournal, +} from '@/commands/eql/journal.js' +import { + reportSkipped, + reportSweepFailure, + reportSweepResult, +} from '@/commands/eql/sweep-report.js' +import { resolveDatabaseUrl } from '@/config/database-url.js' +import { messages } from '@/messages.js' + +/** Same default as `eql migration --drizzle`, and as drizzle-kit's own `out`. */ +const DEFAULT_DRIZZLE_OUT = 'drizzle' + +export interface EqlRepairOptions { + /** Repair a Drizzle output directory. Required — mirrors `eql migration`. */ + drizzle?: boolean + /** Directory holding the migrations. Defaults to `drizzle`. */ + out?: string + /** Report what would be rewritten without writing anything. */ + dryRun?: boolean + /** + * Database to check applied state against. Without it (and without + * `DATABASE_URL`) the repair proceeds, warning that it could not verify. + */ + databaseUrl?: string + /** + * The drizzle migration ledger to read applied state from, as + * `[schema.]table`. Defaults to drizzle-kit's own + * `drizzle.__drizzle_migrations`; set it to match `migrations.table` / + * `migrations.schema` in drizzle.config.ts when the project overrides them. + */ + migrationsTable?: string +} + +/** + * `stash eql repair` — sweep an existing migration directory for the un-runnable + * in-place `ALTER COLUMN ... SET DATA TYPE ` statements + * drizzle-kit emits, and rewrite them into a staged encrypted-column addition. + */ +export async function eqlRepairCommand( + options: EqlRepairOptions, +): Promise { + if (!options.drizzle) { + p.log.error(messages.eql.repairNeedsTarget) + throw new CliExit(1) + } + + // Before anything else: a malformed ledger name must not reach the probe, + // where its "relation does not exist" would masquerade as an absent ledger. + const relation = options.migrationsTable ?? DEFAULT_MIGRATIONS_RELATION + if (!isValidRelation(relation)) { + p.log.error(messages.eql.repairMigrationsTableInvalid(relation)) + throw new CliExit(1) + } + + const outDir = resolve(options.out ?? DEFAULT_DRIZZLE_OUT) + + // Fail closed on a directory that isn't there. `rewriteEncryptedAlterColumns` + // treats ENOENT as an empty sweep, which is right for a generate-time sweep + // but wrong here: a repair that reports "nothing to repair" for a mistyped + // --out sends the user to `drizzle-kit migrate` with the broken SQL intact. + if (!existsSync(outDir)) { + p.log.error(messages.eql.repairOutMissing(outDir)) + throw new CliExit(1) + } + + let journal: JournalEntry[] + try { + journal = await readJournal(outDir) + } catch (error) { + if (error instanceof JournalError) { + p.log.error(messages.eql.repairJournalUnreadable(error.message)) + throw new CliExit(1) + } + throw error + } + + p.intro('CipherStash EQL repair') + + const dryRun = options.dryRun ?? false + const applied = await appliedFiles( + outDir, + journal, + options.databaseUrl, + relation, + ) + + let appliedFindings: AppliedFindings + let result: RewriteResult + let sweepIncomplete: boolean + try { + appliedFindings = await refusedAppliedFiles(outDir, applied) + result = await rewriteEncryptedAlterColumns(outDir, { + dryRun, + skip: [...applied], + }) + sweepIncomplete = reportSweepResult(result, { dryRun }) + } catch (error) { + // Unlike `eql migration`, where the sweep is advisory work after a + // migration was already written, here it IS the command — so report the + // partial work (#786) and fail, rather than surfacing a raw stack trace. + reportSweepFailure(outDir, error) + p.log.error(messages.eql.repairSweepIncomplete(outDir)) + p.outro('Repair incomplete.') + throw new CliExit(1) + } + + const { refused, nearMisses } = appliedFindings + if (refused.length > 0) { + p.log.warn(messages.eql.repairAppliedRefused(refused.length)) + for (const file of refused) p.log.step(` - ${file}`) + p.log.step(` ${messages.eql.repairAppliedHazard}`) + } + // Held back with the rest of their file, so the real sweep never saw them — + // report them here with the same guidance an unapplied near-miss gets. + reportSkipped(nearMisses) + + if (sweepIncomplete || refused.length > 0 || nearMisses.length > 0) { + p.log.error(messages.eql.repairSweepIncomplete(outDir)) + p.outro('Repair incomplete.') + throw new CliExit(1) + } + if (result.rewritten.length === 0) { + p.log.success(messages.eql.repairNothingToDo) + } + p.outro(dryRun ? 'Dry run complete.' : 'Done!') +} + +/** + * What an applied migration carries, split by what the rewriter would have done + * with it — because the two need different words from us. + * + * `refused` are files the sweep WOULD have rewritten. Holding them back is the + * applied-migration hazard, and {@link messages.eql.repairAppliedHazard} + * explains it. + * + * `nearMisses` are statements the sweep would have left alone anyway — an + * undeclared source column, an already-encrypted one, an existing twin. Their + * applied-ness is beside the point: the actionable thing is the skip reason, so + * they get the same guidance an unapplied near-miss gets. Reporting them under + * the hazard banner would swap advice the user can act on for an explanation + * that does not apply to them. + */ +interface AppliedFindings { + refused: string[] + nearMisses: RewriteResult['skipped'] +} + +/** + * The applied migrations that carry an ALTER-to-encrypted statement, split into + * {@link AppliedFindings}. + * + * Answered with a second, WRITE-FREE sweep over the whole directory rather than + * a private matcher: "does this file carry a statement the repair would act on" + * is exactly the question the rewriter answers, and a hand-rolled regex here + * would be a second definition of it, free to drift. The real sweep that follows + * holds these files back, so it cannot answer this itself. + */ +async function refusedAppliedFiles( + outDir: string, + applied: ReadonlySet, +): Promise { + // The intersection is empty by construction when nothing is applied — which + // is every offline run — so don't pay for the extra pass. + if (applied.size === 0) return { refused: [], nearMisses: [] } + const preview = await rewriteEncryptedAlterColumns(outDir, { dryRun: true }) + return { + refused: preview.rewritten.filter((file) => applied.has(file)).sort(), + nearMisses: preview.skipped.filter(({ file }) => applied.has(file)), + } +} + +/** + * The migration files this database has already run, as absolute paths. + * + * Offline (no database URL resolvable) the answer is "none": see + * {@link resolveRepairDatabaseUrl} for why that is the default rather than a + * refusal. + */ +async function appliedFiles( + outDir: string, + journal: readonly JournalEntry[], + databaseUrlFlag: string | undefined, + relation: string = DEFAULT_MIGRATIONS_RELATION, +): Promise> { + const databaseUrl = await resolveRepairDatabaseUrl(databaseUrlFlag) + if (databaseUrl === undefined) { + p.log.warn(messages.eql.repairAppliedUnverified) + return new Set() + } + + let watermark: number | typeof NOTHING_APPLIED | typeof LEDGER_ABSENT + try { + watermark = await latestAppliedMillis(databaseUrl, relation) + } catch (error) { + // Asking for the check and not getting it is a hard failure. Proceeding + // here would rewrite applied migrations while having told the user the + // check was on — the precise drift they passed --database-url to avoid. + p.log.error( + messages.eql.repairAppliedCheckFailed( + error instanceof Error ? error.message : String(error), + ), + ) + p.outro('Repair incomplete.') + throw new CliExit(1) + } + // Absent relation: ambiguous, so warn rather than claim a clean check. See + // LEDGER_ABSENT. Still proceeds, for the same reason the offline path does. + if (watermark === LEDGER_ABSENT) { + p.log.warn(messages.eql.repairLedgerMissing(relation)) + return new Set() + } + if (watermark === NOTHING_APPLIED) { + p.log.info(messages.eql.repairNothingApplied) + return new Set() + } + + // `folderMillis <= max(created_at)` — drizzle's own applied-check, mirrored. + const applied = new Set( + journal + .filter((entry) => entry.when <= watermark) + .map((entry) => join(outDir, `${entry.tag}.sql`)), + ) + p.log.info(messages.eql.repairAppliedCount(applied.size)) + return applied +} + +/** + * The database URL to check applied state against, or `undefined` to skip the + * check. + * + * Deliberately only the first two tiers of `resolveDatabaseUrl` — the explicit + * flag and `DATABASE_URL` — never the interactive prompt or the hard failure + * below them. The applied-state check is an enhancement, not the command's + * purpose: prompting for a connection string in the middle of an offline repair + * would be surprising, and failing without one would make the command useless + * in the flow it exists for (a broken, never-applied migration in a project + * whose database may not even be reachable). + */ +async function resolveRepairDatabaseUrl( + flag: string | undefined, +): Promise { + if (flag === undefined && !process.env.DATABASE_URL?.trim()) return undefined + // Routed through the shared resolver so a malformed --database-url is + // rejected the same way every other command rejects it. + return await resolveDatabaseUrl({ databaseUrlFlag: flag }) +} diff --git a/packages/cli/src/commands/eql/sweep-report.ts b/packages/cli/src/commands/eql/sweep-report.ts new file mode 100644 index 000000000..5bcc8be4d --- /dev/null +++ b/packages/cli/src/commands/eql/sweep-report.ts @@ -0,0 +1,105 @@ +import * as p from '@clack/prompts' +import { + describeSkipReason, + describeStagedReconciliation, + isPartialRewriteResult, + type PartialRewriteResult, + type RewriteResult, +} from '@/commands/db/rewrite-migrations.js' +import { messages } from '@/messages.js' + +/** + * Shared rendering for a {@link RewriteResult}. + * + * `stash eql migration --drizzle` and `stash eql repair --drizzle` run the same + * sweep over the same directory, so they must say the same things about it — + * a divergence between the two surfaces is a defect, not a style difference. + * Both call these helpers rather than each formatting the result themselves. + */ + +/** Render the rewritten-file list under `lead`. */ +function reportRewritten(files: readonly string[], lead: string): void { + if (files.length === 0) return + p.log.info(lead) + for (const file of files) p.log.step(` - ${file}`) +} + +/** + * Render the near-misses the sweep left alone, each with the guidance its + * {@link describeSkipReason} carries — the reasons need different action from + * the user, so a single generic line would hide that. + */ +export function reportSkipped(skipped: PartialRewriteResult['skipped']): void { + if (!skipped || skipped.length === 0) return + p.log.warn( + `Found ${skipped.length} ALTER-to-encrypted statement(s) the sweep left alone. Review and fix them before running your migrations:`, + ) + for (const { file, statement, reason } of skipped) { + p.log.step(` - ${file}: ${statement}`) + p.log.step(` ${describeSkipReason(reason)}`) + } +} + +/** Render the schema/snapshot reconciliation a staged twin leaves behind. */ +function reportStaged(staged: PartialRewriteResult['staged']): void { + if (!staged || staged.length === 0) return + p.log.warn(describeStagedReconciliation(staged).join('\n')) +} + +/** + * Report a completed sweep. Returns `true` when it left statements behind, + * which every caller turns into a non-zero exit: the remaining SQL still fails + * at migrate time, and a zero exit would tell CI the sweep had succeeded. + */ +export function reportSweepResult( + result: RewriteResult, + options: { dryRun?: boolean } = {}, +): boolean { + reportRewritten( + result.rewritten, + options.dryRun + ? `Would rewrite ${result.rewritten.length} migration file(s) to add staged encrypted columns while preserving the source columns:` + : `Rewrote ${result.rewritten.length} migration file(s) to add staged encrypted columns while preserving the source columns:`, + ) + // The rewrite repaired SQL only, so schema.ts and the drizzle-kit snapshot now + // disagree with the database — and `drizzle-kit generate` cannot see it (#836, + // item 2). A warning rather than a failure: the swept SQL is valid and + // additive, and the reconciliation is the user's editorial call. + // + // Under --dry-run none of that has happened yet. `describeStagedReconciliation` + // is written in the past tense ("the database now has"), so printing it here + // would send the user reconciling against a column no migration adds. Count + // the twins instead, and let the real run print the reconciliation. + if (options.dryRun) { + if (result.staged.length > 0) { + p.log.info(messages.eql.repairDryRunStaged(result.staged.length)) + } + } else { + reportStaged(result.staged) + } + reportSkipped(result.skipped) + return result.skipped.length > 0 +} + +/** + * Report a sweep that threw partway through: the work it had already done + * (#786), then the failure itself. Callers always treat this as incomplete. + */ +export function reportSweepFailure(outDir: string, error: unknown): void { + const partial: PartialRewriteResult = isPartialRewriteResult(error) + ? error + : {} + reportRewritten( + partial.rewritten ?? [], + `Rewrote ${partial.rewritten?.length ?? 0} migration file(s) before the sweep stopped:`, + ) + // A partial sweep still staged real twins, so the same three-way divergence + // already exists for them. + reportStaged(partial.staged) + reportSkipped(partial.skipped) + p.log.warn( + `Could not sweep ${outDir} for unsafe ALTER COLUMN statements: ${ + error instanceof Error ? error.message : String(error) + }`, + ) +} diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts index d3b2f9c73..78e1d4a3c 100644 --- a/packages/cli/src/messages.ts +++ b/packages/cli/src/messages.ts @@ -84,6 +84,95 @@ export const messages = { /** `--name` carried characters outside `[A-Za-z0-9_-]`. */ migrationBadName: 'Migration name must contain only letters, numbers, dashes, and underscores.', + /** `stash eql repair` with no `--drizzle` target. */ + repairNeedsTarget: 'Specify a target: `stash eql repair --drizzle`.', + /** `--out` (or its `drizzle` default) points at a directory that isn't there. */ + repairOutMissing: (outDir: string) => + `Drizzle output directory not found: ${outDir}\nPass --out so it matches your drizzle.config.ts.`, + /** + * The sweep left statements it could not rewrite. Fail closed, exactly as + * `eql migration --drizzle` does: the remaining SQL still fails at migrate + * time, so a zero exit would tell CI the repair had succeeded. + */ + repairSweepIncomplete: (outDir: string) => + `The ALTER COLUMN sweep found unsafe or unverified SQL in ${outDir}. Review the statements above and use the staged stash encrypt lifecycle before running drizzle-kit migrate.`, + /** + * The dry-run stand-in for `describeStagedReconciliation`, which is written + * in the past tense and would otherwise claim a column exists that no + * migration has added yet. + */ + repairDryRunStaged: (count: number) => + `Would stage ${count} encrypted column(s). Re-run without --dry-run to apply the repair — the reconciliation your Drizzle schema and drizzle-kit snapshot then need is printed at that point.`, + /** + * Lead line for migrations left alone because the database has already run + * them. A distinct outcome from the sweep's `skipped` near-misses: those + * are statements the rewriter could not understand, this is one it + * understood perfectly and must not act on. + */ + repairAppliedRefused: (count: number) => + `Left ${count} ALREADY APPLIED migration(s) untouched. They carry ALTER-to-encrypted statements, but rewriting an applied migration is not a repair:`, + /** + * Why an applied migration is refused, and what to do instead. Named + * separately so the e2e/unit assertions pin the hazard, not the phrasing + * around it. + */ + repairAppliedHazard: + 'the database already has whatever this migration did, so rewriting its .sql leaves the file describing a shape that database never got from it — a fresh CI or staging database replaying the rewritten file would silently diverge from this one. Reconcile the environments by hand instead, and move the column onto the encrypted twin through the staged `stash encrypt` lifecycle.', + /** + * The applied-state probe could not run. Fail closed: the user asked to be + * protected from rewriting applied migrations, and a silent fallback to + * "nothing is applied" would hand them the drift they were avoiding. + */ + repairAppliedCheckFailed: (detail: string) => + `Could not check which migrations have been applied: ${detail}\nNothing was rewritten. Fix the connection and re-run, or re-run without --database-url to repair unverified (see the warning that prints in that mode).`, + /** + * `--migrations-table` is not a plain `[schema.]table`. Rejected before + * connecting: the value is quoted before it reaches SQL, so it cannot break + * out, but a malformed one would query a relation that cannot exist and the + * resulting `undefined_table` would be read as an absent ledger — silently + * downgrading the check the flag was passed to get. + */ + repairMigrationsTableInvalid: (value: string) => + `--migrations-table must be a table name, optionally schema-qualified (e.g. \`my_migrations\` or \`audit.my_migrations\`). Got: ${value}`, + /** + * The ledger relation is not there. Ambiguous, and deliberately NOT the + * confident `repairNothingApplied`: either `drizzle-kit migrate` never ran + * against this database, or the project overrode `migrations.table` / + * `migrations.schema` in drizzle.config.ts and the probe queried the wrong + * relation. Claiming "nothing applied" for the second case would rewrite + * applied migrations while reporting the check as clean. + */ + repairLedgerMissing: (relation: string) => + `Could not verify which migrations have been applied: ${relation} does not exist. Either drizzle-kit migrate has never run against this database — in which case there is nothing applied and this repair is safe — or your drizzle.config.ts sets migrations.table / migrations.schema, and the check looked in the wrong place. If it does, re-run with --migrations-table <[schema.]table> naming your ledger. Repairing anyway.`, + /** Nothing in `drizzle.__drizzle_migrations` — every migration is fair game. */ + repairNothingApplied: + 'No applied migrations found in drizzle.__drizzle_migrations — every migration in this directory can be repaired.', + /** How many of the journal's migrations the database has already run. */ + repairAppliedCount: (count: number) => + `${count} migration(s) already applied to this database; they will not be rewritten.`, + /** + * No database URL, so the applied-state check did not run. + * + * The default is to proceed. The journal proves a migration EXISTS, not + * that it ran, and refusing everything on that ambiguity would make the + * command useless in exactly the flow it exists for — a broken migration + * that failed on apply, in a project whose database may not be reachable + * from where the repair is being run. So: warn loudly, name the one case + * that is genuinely unsafe, and tell the user how to get the check. + */ + repairAppliedUnverified: + 'Could not verify which migrations have been applied: no --database-url and no DATABASE_URL. The journal shows that a migration EXISTS, not that it ran. Almost every ALTER-to-encrypted statement is un-runnable and so cannot have been applied — the exception is a jsonb column changed to an EQL domain on an empty table, which applies successfully. If you have run drizzle-kit migrate since generating these migrations, re-run with --database-url so applied migrations are left alone.', + /** A clean sweep — said out loud so silence never reads as "did not run". */ + repairNothingToDo: + 'Nothing to repair: no unsafe ALTER-to-encrypted statements found.', + /** + * The drizzle journal is missing or unparseable. `detail` names the file and + * the underlying reason. Fail closed: the journal is the only offline record + * of which migrations exist and when each was generated, so without it the + * applied-state check cannot run and a rewrite would be blind. + */ + repairJournalUnreadable: (detail: string) => + `${detail}\nstash eql repair needs drizzle-kit's meta/_journal.json to tell which migrations have already been applied. Check that --out points at your drizzle-kit output directory.`, }, db: { unknownSubcommand: 'Unknown db subcommand', diff --git a/packages/cli/tests/e2e/command-help.e2e.test.ts b/packages/cli/tests/e2e/command-help.e2e.test.ts index 411dcf3e9..1dc33110a 100644 --- a/packages/cli/tests/e2e/command-help.e2e.test.ts +++ b/packages/cli/tests/e2e/command-help.e2e.test.ts @@ -18,6 +18,7 @@ describe('per-command --help', () => { expect(r.output).toContain('Usage: npx stash eql [options]') expect(r.output).toContain('eql install') expect(r.output).toContain('eql migration') + expect(r.output).toContain('eql repair') expect(r.output).toContain('eql upgrade') expect(r.output).toContain('eql status') // A group listing must NOT be the global banner. @@ -45,6 +46,17 @@ describe('per-command --help', () => { expect(r.output).toContain('Also settable via DATABASE_URL.') }) + it('renders full command help for `eql repair --help`', async () => { + const r = await run(['eql', 'repair', '--help'], { + env: { npm_config_user_agent: '' }, + }) + expect(r.exitCode).toBe(0) + expect(r.output).toContain('Usage: npx stash eql repair [options]') + expect(r.output).toContain('--drizzle') + expect(r.output).toContain('--dry-run') + expect(r.output).toContain('--database-url') + }) + it('honours the `-h` short flag after a command', async () => { const r = await run(['eql', 'install', '-h'], { env: { npm_config_user_agent: '' }, diff --git a/packages/cli/tests/e2e/eql-repair.e2e.test.ts b/packages/cli/tests/e2e/eql-repair.e2e.test.ts new file mode 100644 index 000000000..50d0dbb27 --- /dev/null +++ b/packages/cli/tests/e2e/eql-repair.e2e.test.ts @@ -0,0 +1,75 @@ +import { + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { messages } from '../../src/messages.js' +import { run } from '../helpers/run.js' + +/** + * `stash eql repair --drizzle` end to end: argv parsing, dispatch, the sweep, + * and the exit code — the wiring the unit tests deliberately do not cover + * (they call `eqlRepairCommand` directly). + * + * The applied-state check is not exercised here: it needs a live Postgres, and + * its logic has unit coverage against a faked driver. What matters at this + * level is that the command exists, routes, repairs, and exits correctly. + */ +describe('stash eql repair', () => { + let tmp: string + beforeEach(() => { + tmp = mkdtempSync(join(tmpdir(), 'stash-repair-e2e-')) + }) + afterEach(() => { + rmSync(tmp, { recursive: true, force: true }) + }) + + it('exits 1 with the target message when --drizzle is missing', async () => { + const r = await run(['eql', 'repair'], { cwd: tmp }) + expect(r.exitCode).toBe(1) + expect(r.output).toContain(messages.eql.repairNeedsTarget) + }) + + it('rewrites a broken ALTER COLUMN in the --out directory', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(join(out, 'meta'), { recursive: true }) + writeFileSync( + join(out, '0000_declare.sql'), + 'CREATE TABLE "users" ("email" text);\n', + ) + writeFileSync( + join(out, '0001_encrypt-email.sql'), + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_text_search";\n', + ) + writeFileSync( + join(out, 'meta', '_journal.json'), + JSON.stringify({ + version: '7', + dialect: 'postgresql', + entries: [ + { idx: 0, version: '7', when: 1, tag: '0000_declare' }, + { idx: 1, version: '7', when: 2, tag: '0001_encrypt-email' }, + ], + }), + ) + + const r = await run(['eql', 'repair', '--drizzle', '--out', out], { + cwd: tmp, + }) + + expect(r.exitCode).toBe(0) + // Framed like every other top-level command, so the output reads as one + // run rather than loose log lines. + expect(r.output).toContain('CipherStash EQL repair') + const rewritten = readFileSync(join(out, '0001_encrypt-email.sql'), 'utf-8') + expect(rewritten).toContain( + 'ADD COLUMN "email_encrypted" "public"."eql_v3_text_search"', + ) + expect(rewritten).not.toContain('SET DATA TYPE') + }) +}) diff --git a/packages/cli/tests/e2e/smoke.e2e.test.ts b/packages/cli/tests/e2e/smoke.e2e.test.ts index febcf3b56..72a007d4d 100644 --- a/packages/cli/tests/e2e/smoke.e2e.test.ts +++ b/packages/cli/tests/e2e/smoke.e2e.test.ts @@ -24,6 +24,7 @@ describe('stash CLI — non-interactive smoke', () => { expect(r.output).toContain('init') expect(r.output).toContain('eql install') expect(r.output).toContain('eql migration') + expect(r.output).toContain('eql repair') expect(r.output).toContain('eql upgrade') expect(r.output).toContain('eql status') // The dotenv "injected env" banner regression guard lives in the diff --git a/packages/wizard/src/lib/rewrite-migrations.ts b/packages/wizard/src/lib/rewrite-migrations.ts index 4d8e673aa..0a152f3be 100644 --- a/packages/wizard/src/lib/rewrite-migrations.ts +++ b/packages/wizard/src/lib/rewrite-migrations.ts @@ -1046,10 +1046,17 @@ interface RewriteSweepError extends Error, PartialRewriteResult {} * shipping broken SQL. Statements sitting inside a * SQL comment — or inside a single-quoted string literal, where they are data * rather than SQL — are inert and are neither rewritten nor reported. + * + * `options.skip` names files to leave on disk — they are still READ, because a + * column's current type comes from the whole corpus, but never written. One + * path or many. `options.dryRun` computes the identical result without writing + * anything. Neither is used by this package's own sweep (which rewrites every + * candidate directory in place); they exist because the CLI's `eql repair` + * needs them, and the two copies of this file are kept in sync. */ export async function rewriteEncryptedAlterColumns( outDir: string, - options: { skip?: string } = {}, + options: { skip?: string | readonly string[]; dryRun?: boolean } = {}, ): Promise { const entries = await readdir(outDir).catch( (error: NodeJS.ErrnoException) => { @@ -1065,6 +1072,9 @@ export async function rewriteEncryptedAlterColumns( const staged: StagedColumn[] = [] const seen = new Set() const stagedTargets = new Set() + const skipFiles = new Set( + typeof options.skip === 'string' ? [options.skip] : (options.skip ?? []), + ) /** Record a skip once — the strict pass and the broad scan can both find it. */ const skip = (file: string, statement: string, reason: SkipReason): void => { @@ -1092,7 +1102,7 @@ export async function rewriteEncryptedAlterColumns( try { for (const [filePath, original] of contents) { - if (options.skip && filePath === options.skip) continue + if (skipFiles.has(filePath)) continue // Reset the regex's lastIndex — it's stateful on /g ALTER_COLUMN_TO_ENCRYPTED_RE.lastIndex = 0 @@ -1173,7 +1183,7 @@ export async function rewriteEncryptedAlterColumns( ) if (updated !== original) { - await writeFile(filePath, updated, 'utf-8') + if (!options.dryRun) await writeFile(filePath, updated, 'utf-8') rewritten.push(filePath) staged.push(...fileStaged) } diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 0b8e3f567..b086af393 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -1,6 +1,6 @@ --- name: stash-cli -description: Drive CipherStash setup and encryption migrations through the `stash` CLI — `init`, `plan`, `impl`, `status`, `auth login`, `eql install/upgrade/status`, `db validate`, `encrypt backfill/drop`, `schema build`, and `manifest --json`. Covers the agent / non-interactive interface, credential rules, and the staged EQL v3 rollout lifecycle. +description: Drive CipherStash setup and encryption migrations through the `stash` CLI — `init`, `plan`, `impl`, `status`, `auth login`, `eql install/migration/repair/upgrade/status`, `db validate`, `encrypt backfill/drop`, `schema build`, and `manifest --json`. Covers the agent / non-interactive interface, credential rules, and the staged EQL v3 rollout lifecycle. --- # CipherStash CLI (`stash`) @@ -335,6 +335,7 @@ Flags below are the decision-relevant ones. Run `stash --help` for the ```bash stash eql install stash eql migration --drizzle +stash eql repair --drizzle stash eql upgrade stash eql status ``` @@ -382,6 +383,37 @@ The sweep is fail-closed: it rewrites a statement only when the same directory a **After a successful sweep, reconcile your ORM schema.** The rewrite repairs SQL only, so the database ends up with both `email` (unchanged, still plaintext) and `email_encrypted`, while your `schema.ts` and drizzle-kit's `meta/*_snapshot.json` both still declare `email` as the encrypted domain and know nothing about the twin. `drizzle-kit generate` will **not** warn you: it diffs your schema against its snapshot, reads neither the `.sql` nor the database, and those two still agree. The command prints the divergence per column, naming the table, both columns and the domain. Until you reconcile it, reads of the source column hand plaintext to a decrypt path expecting an EQL envelope, writes store an EQL envelope in a plaintext column and *succeed*, and the new encrypted column is unreachable through the ORM. See `skills/stash-drizzle` for the reconciliation, including the step that is easy to miss: because the snapshot has never seen the encrypted twin, `drizzle-kit generate` **always** emits an `ADD COLUMN` for it, and the swept migration already adds that column — so the regenerated statement has to be deleted (or made `IF NOT EXISTS`) or migrate fails with `column already exists`. +The sweep runs *after* the install migration is written, so it only sees migrations that already exist. The broken `ALTER COLUMN` is normally generated later, by your next `drizzle-kit generate` — use `eql repair` for that, rather than regenerating an install migration you do not need. + +#### `eql repair` + +Runs the same sweep as `eql migration --drizzle`, standalone: it repairs an existing migration directory without generating anything. This is the command for the usual failure — `drizzle-kit generate` emitted an in-place `ALTER COLUMN … SET DATA TYPE ` after EQL was installed, and `drizzle-kit migrate` fails on it. + +```bash +stash eql repair --drizzle # sweep drizzle/ +stash eql repair --drizzle --dry-run # preview; writes nothing +stash eql repair --drizzle --out db/migrations \ + --database-url postgres://… # skip already-applied migrations +``` + +| Flag | Description | +|---|---| +| `--drizzle` | Required. Repair a Drizzle migration directory. | +| `--out ` | Directory to sweep. Default `drizzle`; set it to match your `drizzle.config.ts`. | +| `--dry-run` | Report what would be rewritten without writing anything. | +| `--database-url ` | Check which migrations the database has already applied, and leave those alone. Also read from `DATABASE_URL`. | +| `--migrations-table <[schema.]table>` | Drizzle's migration ledger, when `drizzle.config.ts` overrides `migrations.table` / `migrations.schema`. Defaults to `drizzle.__drizzle_migrations`. Only read alongside `--database-url`. | + +What it rewrites, what it refuses, and the schema reconciliation afterwards are identical to the `eql migration --drizzle` sweep above — both call the same rewriter and print the same report. It exits non-zero if any statement is left unrepaired. + +**Already-applied migrations are refused, not repaired.** Nearly every ALTER-to-encrypted statement is un-runnable, so its migration failed and is safe to rewrite. The exception is a `jsonb` column changed to an EQL domain on an empty table: base types are compatible, so it applies. Rewriting that migration afterwards leaves its `.sql` describing a shape the database never got from it, and a fresh CI or staging database replaying the rewritten file diverges from the one you developed on — silently, since nothing records that they should differ. With `--database-url`, repair reads `drizzle.__drizzle_migrations` and treats a migration as applied when its journal `when` is at or below the latest `created_at` — the same timestamp comparison `drizzle-kit migrate` itself makes (hashes are written but never compared). Applied migrations are listed and left untouched, and the command exits non-zero. + +Without a database URL the repair proceeds and warns that applied state could not be verified: the journal proves a migration exists, not that it ran. Pass `--database-url` whenever you have run `drizzle-kit migrate` since generating the migrations. If the check is requested but cannot run (connection refused, bad credentials), nothing is rewritten and the command exits non-zero rather than silently repairing unverified. + +**If `drizzle.config.ts` overrides `migrations.table` / `migrations.schema`, pass `--migrations-table`.** The probe cannot discover a renamed ledger, and a missing one is ambiguous — it means either `drizzle-kit migrate` never ran here, or the check looked in the wrong place. Repair reports that state as *unverified* rather than as "nothing applied" and says which relation it tried, so a custom-ledger project is never told its applied migrations are safe to rewrite. Naming the ledger turns the check back on. The value must be a plain table name, optionally schema-qualified; anything else is rejected before connecting. + +An applied migration carrying a statement the sweep would have skipped anyway — an undeclared source column, an already-encrypted one, an existing twin — is reported with that skip reason, not as an applied-migration refusal. Its applied-ness is not why it was left alone. + #### `eql upgrade` The install SQL is safe to re-run — columns and data survive — but it cascade-drops functional indexes that depend on `eql_v3`; recreate them afterward. `upgrade` is v3-only and accepts `--supabase`, `--dry-run`, and `--database-url`. diff --git a/skills/stash-drizzle/SKILL.md b/skills/stash-drizzle/SKILL.md index 779aa8f0a..8fd4cd6f7 100644 --- a/skills/stash-drizzle/SKILL.md +++ b/skills/stash-drizzle/SKILL.md @@ -61,7 +61,18 @@ stash eql migration --drizzle --supabase # also grants eql_v3 to anon/authenti The generated migration also installs the `cs_migrations` tracking schema, so a single `drizzle-kit migrate` covers everything `stash encrypt …` needs — no out-of-band `stash eql install`. EQL v3 ships one SQL bundle for every target including Supabase; `--supabase` only adds the PostgREST/RLS role grants (harmless when you connect directly as `postgres`). Requires `drizzle-kit` installed and configured. -**Changing an existing plaintext column to an encrypted one.** `drizzle-kit generate` emits an in-place `ALTER TABLE … ALTER COLUMN … SET DATA TYPE eql_v3_`, which Postgres rejects — there is no cast from `text`/`numeric` to an EQL domain. (On drizzle-kit 0.31.0 and later the emitted type is also mangled to `"undefined"."eql_v3_"`, since a `customType` has no `typeSchema`.) The `stash eql migration --drizzle` sweep repairs the invalid statement — the `stash-cli` skill covers what it rewrites. The repair is **add-only**: it adds a staged `_encrypted` column and leaves the source column in place, so it never emits `DROP COLUMN` or `RENAME COLUMN` and is safe to apply on a populated table. It repairs only what it can prove — the swept directory must also contain the migration that declared the column, so the sweep can see the source type. A statement it cannot place, one whose column is already encrypted, one whose encrypted twin already exists, or one outside the strict matcher (a hand-authored `SET DATA TYPE … USING …`) is left untouched, and the command exits non-zero so you review the directory before running `drizzle-kit migrate`. Applying the swept migration only *adds* the column — encrypting the data is the staged flow in **Migrating an Existing Column to Encrypted** below. +**Changing an existing plaintext column to an encrypted one.** `drizzle-kit generate` emits an in-place `ALTER TABLE … ALTER COLUMN … SET DATA TYPE eql_v3_`, which Postgres rejects — there is no cast from `text`/`numeric` to an EQL domain. (On drizzle-kit 0.31.0 and later the emitted type is also mangled to `"undefined"."eql_v3_"`, since a `customType` has no `typeSchema`.) Repair it with: + +```bash +stash eql repair --drizzle # sweep drizzle/, rewrite the invalid statements +stash eql repair --drizzle --dry-run # preview first; writes nothing +``` + +The repair is **add-only**: it adds a staged `_encrypted` column and leaves the source column in place, so it never emits `DROP COLUMN` or `RENAME COLUMN` and is safe to apply on a populated table. It repairs only what it can prove — the swept directory must also contain the migration that declared the column, so the sweep can see the source type. A statement it cannot place, one whose column is already encrypted, one whose encrypted twin already exists, or one outside the strict matcher (a hand-authored `SET DATA TYPE … USING …`) is left untouched, and the command exits non-zero so you review the directory before running `drizzle-kit migrate`. Applying the swept migration only *adds* the column — encrypting the data is the staged flow in **Migrating an Existing Column to Encrypted** below. + +`stash eql migration --drizzle` runs the identical sweep over its output directory, but only sees migrations that already exist when it runs — and this broken `ALTER COLUMN` is normally generated afterwards. Reach for `eql repair`; there is no reason to generate a second EQL install migration to trigger a repair. + +**If you have already run `drizzle-kit migrate`, pass `--database-url`.** Repair then reads `drizzle.__drizzle_migrations` and leaves applied migrations alone, listing them and exiting non-zero. Rewriting a migration the database has already run would leave its `.sql` describing a shape that database never got from it, so a fresh CI or staging database replaying the file diverges from yours — silently. This is rare but real: an ALTER to an EQL domain is normally un-runnable, so its migration failed and is safe to rewrite, but a `jsonb` column changed to an EQL domain on an **empty** table applies successfully. Without a URL (and without `DATABASE_URL`) the repair proceeds and warns that it could not verify applied state — the journal proves a migration exists, not that it ran. If your `drizzle.config.ts` sets `migrations.table` or `migrations.schema`, add `--migrations-table <[schema.]table>`: the probe cannot discover a renamed ledger, and without it the check finds nothing at the default relation and reports applied state as unverified rather than claiming nothing is applied. **Reconcile your schema after a sweep — `drizzle-kit generate` will not tell you to.** The sweep repairs SQL and nothing else, so once you apply the swept migration three artefacts disagree: