diff --git a/.changeset/cli-near-miss-statement-trim.md b/.changeset/cli-near-miss-statement-trim.md new file mode 100644 index 000000000..6cd7180aa --- /dev/null +++ b/.changeset/cli-near-miss-statement-trim.md @@ -0,0 +1,15 @@ +--- +'stash': patch +--- + +Trim the leading comment block from near-miss statements reported by the Drizzle +migration rewriter (`stash eql migration --drizzle`, `stash eql install`). + +The broad near-miss scan is anchored on the previous `;`, so a +`SET DATA TYPE … USING …` it could not safely repair was quoted back to the user +with every preceding comment and blank line glued to its front — in a file +opening with a comment block, that meant the whole header. The reported +statement is now the offending statement alone. Detection is unchanged; only the +text shown to the user is affected. + +Keeps this rewriter in sync with its sibling in `@cipherstash/wizard`. diff --git a/.changeset/wizard-eql-v3-migration-rewrite.md b/.changeset/wizard-eql-v3-migration-rewrite.md new file mode 100644 index 000000000..a627e5eb9 --- /dev/null +++ b/.changeset/wizard-eql-v3-migration-rewrite.md @@ -0,0 +1,36 @@ +--- +'@cipherstash/wizard': minor +--- + +Teach the wizard's post-agent Drizzle step to repair EQL **v3** migrations, not +just legacy EQL v2. + +The wizard now scaffolds EQL v3 columns, so `drizzle-kit generate` emits +`ALTER TABLE … ALTER COLUMN … SET DATA TYPE eql_v3_` — which Postgres +rejects (there is no cast from `text`/`numeric` to an EQL domain). The migration +rewriter previously matched only the single `eql_v2_encrypted` type, so those v3 +statements slipped through unrepaired and failed at migrate time. + +The rewriter is ported to match the whole EQL v3 concrete-domain family +(`eql_v3_text_search`, `eql_v3_integer_ord`, …) alongside legacy +`eql_v2_encrypted`, across every mangled form drizzle-kit emits (including the +`"undefined".` prefix from 0.31.0+ and schema-qualified `pgSchema()` tables). It +now also flags near-miss `SET DATA TYPE … USING …` statements it cannot safely +repair instead of leaving broken SQL, and each rewritten file carries a clearer +warning that the ADD+DROP+RENAME is data-destroying and safe only on an empty +table — a populated table must use the staged `stash encrypt` flow. This +re-converges the rewriter with the sibling copy in the `stash` CLI. + +The post-agent step now sweeps **every** candidate migration directory +(`drizzle/`, `migrations/`, `src/db/migrations/`) rather than stopping at the +first one that exists. Previously an empty or already-rewritten `drizzle/` +sitting next to a project's real `migrations/` caused those migrations to be +skipped entirely, so they still failed at migrate time. A directory that can't +be read is reported and the remaining candidates are still swept. Reported +near-miss statements are also trimmed of any preceding comment block, so the +statement quoted back to the user is the offending statement alone. + +Database introspection also recognises v3 encrypted columns: `isEqlEncrypted` +now reports both `eql_v2_encrypted` and the `eql_v3_*` family as already +encrypted, so the agent won't scaffold over existing encrypted data of either +generation. diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d98cde74b..1b96cbb2f 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -146,6 +146,12 @@ jobs: - name: Typecheck (bench — guards the stack/stack-drizzle importers) run: pnpm exec turbo run build --filter @cipherstash/bench + # The wizard is built by tsup, which transpiles without typechecking, so + # nothing here caught the three `auth.AutoStrategy` resolution errors that + # sat in `main` until #771. Gate it so they cannot come back silently. + - name: Typecheck (wizard) + run: pnpm --filter @cipherstash/wizard run typecheck + - name: Lint — no hardcoded package-manager runners run: pnpm run lint:runners diff --git a/packages/cli/src/__tests__/rewrite-migrations.test.ts b/packages/cli/src/__tests__/rewrite-migrations.test.ts index e213cc886..173cba0fb 100644 --- a/packages/cli/src/__tests__/rewrite-migrations.test.ts +++ b/packages/cli/src/__tests__/rewrite-migrations.test.ts @@ -424,6 +424,66 @@ describe('rewriteEncryptedAlterColumns', () => { expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) }) + // A near-miss is quoted back to the user verbatim, so it must read as the + // offending statement alone. NEAR_MISS_RE opens with a lazy `[^;]*?`, which + // can only be bounded by the previous `;` — so without an explicit trim the + // reported "statement" drags in every comment and blank line since then. + it('reports a near-miss without the file-leading comment block', async () => { + const statement = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;' + const filePath = path.join(tmpDir, '0022_preamble.sql') + fs.writeFileSync( + filePath, + [ + '-- Custom SQL migration file, put your code below! --', + '-- Hand-converts the email column in place.', + '', + statement, + '', + ].join('\n'), + ) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + + it('reports a near-miss without a preceding statement-breakpoint marker', async () => { + const statement = + 'ALTER TABLE "users" ALTER COLUMN "meta" SET DATA TYPE eql_v3_json USING (meta)::jsonb;' + const filePath = path.join(tmpDir, '0023_breakpoint-preamble.sql') + fs.writeFileSync( + filePath, + [ + 'CREATE TABLE "users" ("id" integer PRIMARY KEY);', + '--> statement-breakpoint', + statement, + '', + ].join('\n'), + ) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + + it('keeps a multi-line near-miss statement intact after the preamble trim', async () => { + const statement = [ + 'ALTER TABLE "users"', + ' ALTER COLUMN "email"', + ' SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;', + ].join('\n') + const filePath = path.join(tmpDir, '0024_multiline.sql') + fs.writeFileSync(filePath, `-- leading note\n\n${statement}\n`) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + it('reports no skipped statements for a clean file', async () => { const filePath = path.join(tmpDir, '0020_clean.sql') fs.writeFileSync(filePath, 'CREATE TABLE "t" ("id" integer);\n') diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index a96dc5229..966cd54d8 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -95,6 +95,27 @@ const ALTER_COLUMN_TO_ENCRYPTED_RE = new RegExp( const NEAR_MISS_RE = /[^;]*?\bSET\s+DATA\s+TYPE\b[^;]*?\beql_v[23][a-z0-9_]*[^;]*?;/gi +/** + * Blank lines and `--` line comments (including drizzle-kit's + * `--> statement-breakpoint`) at the head of a {@link NEAR_MISS_RE} match. + * + * That regex opens with a lazy `[^;]*?`, whose only left boundary is the + * previous `;` — or the start of the file when there is no preceding statement. + * So the raw match drags in every comment and blank line since then, and a + * near-miss in a file that opens with a comment block gets reported to the user + * with that whole block glued to its front. Strip the preamble so the statement + * we quote back reads as the offending statement alone. + * + * Only line comments are handled — `/* … *\/` block comments are not something + * drizzle-kit emits, and a stray one costs cosmetics, not correctness. + */ +const STATEMENT_PREAMBLE_RE = /^(?:[^\S\n]*(?:--[^\n]*)?\n)+/ + +/** Drop the leading blank/comment lines a `[^;]*?`-anchored match dragged in. */ +function trimStatementPreamble(statement: string): string { + return statement.replace(STATEMENT_PREAMBLE_RE, '').trim() +} + /** A statement the sweep recognised as ALTER-to-encrypted but did NOT rewrite. */ export interface SkippedAlter { /** Absolute path of the migration file the statement lives in. */ @@ -189,7 +210,10 @@ export async function rewriteEncryptedAlterColumns( // matcher. Flag it — non-fatally — rather than leave the user shipping SQL // that fails at migrate time. for (const nearMiss of updated.matchAll(NEAR_MISS_RE)) { - skipped.push({ file: filePath, statement: nearMiss[0].trim() }) + skipped.push({ + file: filePath, + statement: trimStatementPreamble(nearMiss[0]), + }) } } diff --git a/packages/wizard/package.json b/packages/wizard/package.json index e57cf8198..99353b657 100644 --- a/packages/wizard/package.json +++ b/packages/wizard/package.json @@ -26,6 +26,7 @@ "postbuild": "chmod +x ./dist/bin/wizard.js", "dev": "tsup --watch", "test": "vitest run", + "typecheck": "tsc --project tsconfig.json --noEmit", "lint": "biome check ." }, "dependencies": { diff --git a/packages/wizard/src/__tests__/rewrite-migrations.test.ts b/packages/wizard/src/__tests__/rewrite-migrations.test.ts new file mode 100644 index 000000000..41507fe86 --- /dev/null +++ b/packages/wizard/src/__tests__/rewrite-migrations.test.ts @@ -0,0 +1,564 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + rewriteEncryptedAlterColumns, + sweepMigrationDirs, +} from '../lib/rewrite-migrations.js' + +describe('rewriteEncryptedAlterColumns', () => { + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wizard-rewrite-')) + }) + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + it('rewrites an in-place ALTER COLUMN with the bare v2 type name', async () => { + const original = `ALTER TABLE "transactions" ALTER COLUMN "amount" SET DATA TYPE eql_v2_encrypted;\n` + const filePath = path.join(tmpDir, '0002_alter.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "transactions" ADD COLUMN "amount__cipherstash_tmp" "public"."eql_v2_encrypted";', + ) + expect(updated).toContain( + 'ALTER TABLE "transactions" DROP COLUMN "amount";', + ) + expect(updated).toContain( + 'ALTER TABLE "transactions" RENAME COLUMN "amount__cipherstash_tmp" TO "amount";', + ) + expect(updated).not.toContain('SET DATA TYPE') + // Wizard-branded header. + expect(updated).toContain('-- Rewritten by @cipherstash/wizard') + }) + + it('rewrites a bare v3 domain (the generation the wizard now scaffolds)', async () => { + const original = `ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n` + const filePath = path.join(tmpDir, '0002_v3.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "users" ADD COLUMN "email__cipherstash_tmp" "public"."eql_v3_text_search";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('rewrites the schema-qualified form produced by drizzle-kit', async () => { + const original = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "public"."eql_v3_text_search";\n' + const filePath = path.join(tmpDir, '0003_alter.sql') + fs.writeFileSync(filePath, original) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "users" ADD COLUMN "email__cipherstash_tmp" "public"."eql_v3_text_search";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('rewrites a schema-qualified table produced by pgSchema()', async () => { + // drizzle-kit emits `"app"."users"` for a table declared in a pgSchema(); + // the old `\s+` between the table and ALTER COLUMN could never cross the `.`. + const original = + 'ALTER TABLE "app"."users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_text_search";\n' + const filePath = path.join(tmpDir, '0014_qualified.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + const updated = fs.readFileSync(filePath, 'utf-8') + // Every emitted statement keeps the schema qualifier. + expect(updated).toContain( + 'ALTER TABLE "app"."users" ADD COLUMN "email__cipherstash_tmp" "public"."eql_v3_text_search";', + ) + expect(updated).toContain('ALTER TABLE "app"."users" DROP COLUMN "email";') + expect(updated).toContain( + 'ALTER TABLE "app"."users" RENAME COLUMN "email__cipherstash_tmp" TO "email";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('rewrites the "undefined" schema form drizzle-kit emits for bare custom types', async () => { + const original = + 'ALTER TABLE "transactions" ALTER COLUMN "amount" SET DATA TYPE "undefined"."eql_v2_encrypted";\n' + const filePath = path.join(tmpDir, '0005_undef.sql') + fs.writeFileSync(filePath, original) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "transactions" ADD COLUMN "amount__cipherstash_tmp" "public"."eql_v2_encrypted";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('rewrites the double-quoted form produced by stack 0.15.0', async () => { + const original = + 'ALTER TABLE "transactions" ALTER COLUMN "description" SET DATA TYPE "undefined".""public"."eql_v2_encrypted"";\n' + const filePath = path.join(tmpDir, '0006_double.sql') + fs.writeFileSync(filePath, original) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "transactions" ADD COLUMN "description__cipherstash_tmp" "public"."eql_v2_encrypted";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('leaves unrelated migrations untouched', async () => { + const original = + 'CREATE TABLE "widgets" ("id" integer PRIMARY KEY, "name" text);\n' + const filePath = path.join(tmpDir, '0001_init.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([]) + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) + }) + + it('skips the file passed in options.skip', async () => { + const install = path.join(tmpDir, '0000_install-eql.sql') + const alter = path.join(tmpDir, '0002_alter.sql') + fs.writeFileSync(install, 'CREATE SCHEMA eql_v2;\n') + fs.writeFileSync( + alter, + 'ALTER TABLE "t" ALTER COLUMN "c" SET DATA TYPE eql_v2_encrypted;', + ) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir, { + skip: install, + }) + expect(rewritten).toEqual([alter]) + expect(fs.readFileSync(install, 'utf-8')).toBe('CREATE SCHEMA eql_v2;\n') + }) + + it('returns an empty result when the directory does not exist', async () => { + const missing = path.join(tmpDir, 'does-not-exist') + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(missing) + expect(rewritten).toEqual([]) + expect(skipped).toEqual([]) + }) + + // Every concrete `eql_v3_*` domain shipped by `@cipherstash/stack/eql/v3` + // (see `packages/stack/src/eql/v3/columns.ts`). Eight scalar bases carry the + // four storage/eq/ord flavours; text adds `_match`/`_search`; boolean and json + // stand alone. + const V3_SCALAR_BASES = [ + 'integer', + 'smallint', + 'bigint', + 'numeric', + 'real', + 'double', + 'date', + 'timestamp', + ] + const V3_DOMAINS = [ + ...V3_SCALAR_BASES.flatMap((base) => + ['', '_eq', '_ord', '_ord_ore'].map( + (flavour) => `eql_v3_${base}${flavour}`, + ), + ), + 'eql_v3_text', + 'eql_v3_text_eq', + 'eql_v3_text_match', + 'eql_v3_text_ord', + 'eql_v3_text_ord_ore', + 'eql_v3_text_search', + 'eql_v3_boolean', + 'eql_v3_json', + ] + + it.each(V3_DOMAINS)('rewrites an ALTER COLUMN to %s', async (domain) => { + const filePath = path.join(tmpDir, '0007_v3.sql') + fs.writeFileSync( + filePath, + `ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."${domain}";\n`, + ) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + `ALTER TABLE "users" ADD COLUMN "email__cipherstash_tmp" "public"."${domain}";`, + ) + expect(updated).toContain('ALTER TABLE "users" DROP COLUMN "email";') + expect(updated).toContain( + 'ALTER TABLE "users" RENAME COLUMN "email__cipherstash_tmp" TO "email";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + // DOMAIN_RE is derived from ENCRYPTED_DOMAIN, so drift between the two can't + // silently leave a domain unrewritten. Prove every domain the alternation + // recognises is actually extracted into the emitted ADD COLUMN. + it.each([ + ...V3_DOMAINS, + 'eql_v2_encrypted', + ])('extracts the bare domain %s from a mangled ALTER', async (domain) => { + const filePath = path.join(tmpDir, '0015_drift.sql') + fs.writeFileSync( + filePath, + `ALTER TABLE "t" ALTER COLUMN "c" SET DATA TYPE "undefined"."${domain}";\n`, + ) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + `ALTER TABLE "t" ADD COLUMN "c__cipherstash_tmp" "public"."${domain}";`, + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + // The mangled forms are the cross product of what `dataType()` returns and + // which drizzle-kit era renders it (see the file's comment table). + const MANGLED_FORMS: Array<[label: string, emitted: string]> = [ + ['plain, drizzle-kit <=0.30.6', 'eql_v3_text_search'], + [ + '"undefined"-prefixed, drizzle-kit >=0.31.0', + '"undefined"."eql_v3_text_search"', + ], + ['dotted, drizzle-kit <=0.30.6', 'public.eql_v3_text_search'], + [ + 'dotted inside "undefined", drizzle-kit >=0.31.0', + '"undefined"."public.eql_v3_text_search"', + ], + ['pre-quoted, drizzle-kit <=0.30.6', '"public"."eql_v3_text_search"'], + [ + 'pre-quoted inside "undefined", drizzle-kit >=0.31.0', + '"undefined".""public"."eql_v3_text_search""', + ], + ['bare-quoted (speculative)', '"eql_v3_text_search"'], + ] + + it.each(MANGLED_FORMS)('rewrites the v3 %s form', async (_label, emitted) => { + const filePath = path.join(tmpDir, '0008_form.sql') + fs.writeFileSync( + filePath, + `ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE ${emitted};\n`, + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "users" ADD COLUMN "email__cipherstash_tmp" "public"."eql_v3_text_search";', + ) + expect(updated).not.toContain('SET DATA TYPE') + }) + + it('names the target domain in the guidance comment', async () => { + const filePath = path.join(tmpDir, '0010_comment.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_integer_ord";\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain('-- eql_v3_integer_ord.') + }) + + it('warns that the rewrite is data-destroying / empty-table-only', async () => { + const filePath = path.join(tmpDir, '0016_warn.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain('safe ONLY if') + expect(updated).toContain('constraints, defaults, and indexes') + expect(updated).toContain('stash encrypt') + }) + + it('separates ADD/DROP/RENAME with --> statement-breakpoint', async () => { + const filePath = path.join(tmpDir, '0018_breakpoint.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8').trimEnd() + const chunks = updated.split('--> statement-breakpoint') + expect(chunks).toHaveLength(3) + expect(chunks[0]).toContain('ADD COLUMN') + expect(chunks[1]).toContain('DROP COLUMN') + expect(chunks[2]).toContain('RENAME COLUMN') + }) + + it('rewrites each statement to its own domain when v2 and v3 are mixed', async () => { + const filePath = path.join(tmpDir, '0011_mixed.sql') + fs.writeFileSync( + filePath, + [ + 'ALTER TABLE "a" ALTER COLUMN "x" SET DATA TYPE eql_v2_encrypted;', + 'ALTER TABLE "a" ALTER COLUMN "y" SET DATA TYPE "undefined"."eql_v3_json";', + ].join('\n'), + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain( + 'ALTER TABLE "a" ADD COLUMN "x__cipherstash_tmp" "public"."eql_v2_encrypted";', + ) + expect(updated).toContain( + 'ALTER TABLE "a" ADD COLUMN "y__cipherstash_tmp" "public"."eql_v3_json";', + ) + }) + + it.each([ + ['a plaintext type', 'text'], + ['jsonb', 'jsonb'], + ['a lookalike from another EQL major', 'eql_v4_text_search'], + ['a lookalike prefix', 'not_eql_v3_text_search'], + ])('leaves an ALTER COLUMN to %s untouched', async (_label, type) => { + const original = `ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE ${type};\n` + const filePath = path.join(tmpDir, '0012_other.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([]) + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) + }) + + it('leaves a hand-authored SET DATA TYPE ... USING conversion untouched but flags it', async () => { + const original = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;\n' + const filePath = path.join(tmpDir, '0013_using.sql') + fs.writeFileSync(filePath, original) + + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([]) + expect(skipped).toHaveLength(1) + expect(skipped[0].file).toBe(filePath) + expect(skipped[0].statement).toContain('SET DATA TYPE') + expect(skipped[0].statement).toContain('eql_v3_text_search') + // Left untouched on disk — we flag, we don't guess. + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) + }) + + it('reports no skipped statements when the strict rewrite fully handled the file', async () => { + const filePath = path.join(tmpDir, '0021_handled.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n', + ) + + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([filePath]) + expect(skipped).toEqual([]) + }) + + // A near-miss is quoted back to the user verbatim, so it must read as the + // offending statement alone. NEAR_MISS_RE opens with a lazy `[^;]*?`, which + // can only be bounded by the previous `;` — so without an explicit trim the + // reported "statement" drags in every comment and blank line since then. + it('reports a near-miss without the file-leading comment block', async () => { + const statement = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;' + const original = [ + '-- Custom SQL migration file, put your code below! --', + '-- Hand-converts the email column in place.', + '', + statement, + '', + ].join('\n') + const filePath = path.join(tmpDir, '0022_preamble.sql') + fs.writeFileSync(filePath, original) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + + it('reports a near-miss without a preceding statement-breakpoint marker', async () => { + const statement = + 'ALTER TABLE "users" ALTER COLUMN "meta" SET DATA TYPE eql_v3_json USING (meta)::jsonb;' + const original = [ + 'CREATE TABLE "users" ("id" integer PRIMARY KEY);', + '--> statement-breakpoint', + statement, + '', + ].join('\n') + const filePath = path.join(tmpDir, '0023_breakpoint-preamble.sql') + fs.writeFileSync(filePath, original) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + + it('keeps a multi-line near-miss statement intact after the preamble trim', async () => { + const statement = [ + 'ALTER TABLE "users"', + ' ALTER COLUMN "email"', + ' SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;', + ].join('\n') + const filePath = path.join(tmpDir, '0024_multiline.sql') + fs.writeFileSync(filePath, `-- leading note\n\n${statement}\n`) + + const { skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(skipped).toHaveLength(1) + expect(skipped[0].statement).toBe(statement) + }) + + it('handles multiple ALTER statements in one file', async () => { + const original = [ + 'ALTER TABLE "a" ALTER COLUMN "x" SET DATA TYPE eql_v3_text_search;', + 'ALTER TABLE "a" ALTER COLUMN "y" SET DATA TYPE eql_v3_text_search;', + 'CREATE INDEX "a_z" ON "a" ("z");', + ].join('\n') + const filePath = path.join(tmpDir, '0004_multi.sql') + fs.writeFileSync(filePath, original) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated.match(/ADD COLUMN/g)?.length).toBe(2) + expect(updated.match(/DROP COLUMN/g)?.length).toBe(2) + // Non-matching statement preserved + expect(updated).toContain('CREATE INDEX "a_z" ON "a" ("z");') + }) +}) + +describe('sweepMigrationDirs', () => { + let tmpDir: string + + const ALTER = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search;\n' + + /** Create `dir` under the sandbox, optionally seeding `name` with `sql`. */ + const seedDir = (dir: string, name?: string, sql?: string): string => { + const abs = path.join(tmpDir, dir) + fs.mkdirSync(abs, { recursive: true }) + if (name) fs.writeFileSync(path.join(abs, name), sql ?? ALTER) + return abs + } + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'wizard-sweep-')) + }) + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + it('skips candidate directories that do not exist', async () => { + const abs = seedDir('migrations', '0001_alter.sql') + + const results = await sweepMigrationDirs(tmpDir, ['drizzle', 'migrations']) + + expect(results.map((r) => r.dir)).toEqual(['migrations']) + expect(results[0].rewritten).toEqual([path.join(abs, '0001_alter.sql')]) + }) + + // The regression this test locks in: the old loop `return`ed after the FIRST + // existing candidate, so a project with an empty `drizzle/` alongside a real + // `migrations/` had its actual migrations silently left unrepaired. + it('keeps sweeping when an earlier candidate directory yields no matches', async () => { + seedDir('drizzle') + const abs = seedDir('migrations', '0002_alter.sql') + + const results = await sweepMigrationDirs(tmpDir, ['drizzle', 'migrations']) + + expect(results.map((r) => r.dir)).toEqual(['drizzle', 'migrations']) + expect(results[0].rewritten).toEqual([]) + expect(results[1].rewritten).toEqual([path.join(abs, '0002_alter.sql')]) + expect( + fs.readFileSync(path.join(abs, '0002_alter.sql'), 'utf-8'), + ).not.toContain('SET DATA TYPE') + }) + + it('rewrites every candidate directory that holds encrypted alters', async () => { + const drizzle = seedDir('drizzle', '0001_alter.sql') + const nested = seedDir('src/db/migrations', '0002_alter.sql') + + const results = await sweepMigrationDirs(tmpDir, [ + 'drizzle', + 'src/db/migrations', + ]) + + expect(results.flatMap((r) => r.rewritten)).toEqual([ + path.join(drizzle, '0001_alter.sql'), + path.join(nested, '0002_alter.sql'), + ]) + }) + + it('is idempotent — a second sweep rewrites nothing', async () => { + const abs = seedDir('drizzle', '0001_alter.sql') + + await sweepMigrationDirs(tmpDir, ['drizzle']) + const afterFirst = fs.readFileSync( + path.join(abs, '0001_alter.sql'), + 'utf-8', + ) + const results = await sweepMigrationDirs(tmpDir, ['drizzle']) + + expect(results[0].rewritten).toEqual([]) + expect(fs.readFileSync(path.join(abs, '0001_alter.sql'), 'utf-8')).toBe( + afterFirst, + ) + }) + + it('surfaces a failing directory as an error and still sweeps the rest', async () => { + // A directory named `*.sql` makes readFile throw EISDIR mid-sweep. + const broken = seedDir('drizzle') + fs.mkdirSync(path.join(broken, '0001_alter.sql')) + const abs = seedDir('migrations', '0002_alter.sql') + + const results = await sweepMigrationDirs(tmpDir, ['drizzle', 'migrations']) + + expect(results[0].dir).toBe('drizzle') + expect(results[0].error).toBeDefined() + expect(results[1].rewritten).toEqual([path.join(abs, '0002_alter.sql')]) + }) + + it('reports near-misses per directory', async () => { + const abs = seedDir( + 'drizzle', + '0001_using.sql', + 'ALTER TABLE "t" ALTER COLUMN "c" SET DATA TYPE eql_v3_json USING (c)::jsonb;\n', + ) + + const results = await sweepMigrationDirs(tmpDir, ['drizzle']) + + expect(results[0].skipped).toHaveLength(1) + expect(results[0].skipped[0].file).toBe(path.join(abs, '0001_using.sql')) + }) +}) diff --git a/packages/wizard/src/lib/post-agent.ts b/packages/wizard/src/lib/post-agent.ts index 34c18f2a7..e66af3b1a 100644 --- a/packages/wizard/src/lib/post-agent.ts +++ b/packages/wizard/src/lib/post-agent.ts @@ -6,11 +6,9 @@ */ import { execSync } from 'node:child_process' -import { existsSync } from 'node:fs' -import { resolve } from 'node:path' import * as p from '@clack/prompts' import type { GatheredContext } from './gather.js' -import { rewriteEncryptedAlterColumns } from './rewrite-migrations.js' +import { sweepMigrationDirs } from './rewrite-migrations.js' import type { DetectedPackageManager, Integration } from './types.js' interface PostAgentOptions { @@ -21,8 +19,10 @@ interface PostAgentOptions { } /** - * Candidate directories drizzle-kit may write migrations to. We check in - * order and rewrite the first one that exists; `drizzle` is the default. + * Candidate directories drizzle-kit may write migrations to. `drizzle` is the + * default, but a project's configured `out` is not discoverable from here, so + * every candidate that exists is swept — see {@link sweepMigrationDirs} for why + * stopping at the first one loses migrations. */ const DRIZZLE_OUT_DIRS = ['drizzle', 'migrations', 'src/db/migrations'] @@ -76,8 +76,10 @@ export async function runPostAgentSteps(opts: PostAgentOptions): Promise { cwd, ) - // Rewrite any `ALTER COLUMN ... SET DATA TYPE eql_v2_encrypted` that - // drizzle-kit just produced — those fail in Postgres. CIP-2991 + CIP-2994. + // Rewrite any `ALTER COLUMN ... SET DATA TYPE ` that + // drizzle-kit just produced — those fail in Postgres (no cast from + // text/numeric to an EQL domain). Covers the EQL v3 family the wizard now + // scaffolds, and legacy eql_v2_encrypted. CIP-2991 + CIP-2994 + #693. await rewriteEncryptedMigrations(cwd) const shouldMigrate = await p.confirm({ @@ -113,28 +115,29 @@ export async function runPostAgentSteps(opts: PostAgentOptions): Promise { } async function rewriteEncryptedMigrations(cwd: string): Promise { - for (const dir of DRIZZLE_OUT_DIRS) { - const abs = resolve(cwd, dir) - if (!existsSync(abs)) continue - - try { - const rewritten = await rewriteEncryptedAlterColumns(abs) - if (rewritten.length > 0) { - p.log.info( - `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to use ADD+DROP+RENAME for encrypted columns.`, - ) - for (const file of rewritten) p.log.step(` - ${file}`) - p.log.warn( - 'If any of these tables already have rows, backfill the new column via @cipherstash/stack before running the migration in production. See the comments in the rewritten SQL.', - ) - } - // Only rewrite the first dir that matches — running again on a - // different candidate would double-transform already-rewritten SQL. - return - } catch (err) { - const message = err instanceof Error ? err.message : String(err) - p.log.warn(`Could not rewrite migrations in ${dir}: ${message}`) - return + const results = await sweepMigrationDirs(cwd, DRIZZLE_OUT_DIRS) + + for (const { dir, rewritten, skipped, error } of results) { + if (error) { + p.log.warn(`Could not rewrite migrations in ${dir}: ${error}`) + continue + } + + if (rewritten.length > 0) { + p.log.info( + `Rewrote ${rewritten.length} migration file(s) in ${dir}/ to use ADD+DROP+RENAME for encrypted columns.`, + ) + for (const file of rewritten) p.log.step(` - ${file}`) + p.log.warn( + 'This rewrite is data-destroying — safe only on an EMPTY table. If any of these tables already have rows, do NOT run the migration; use the staged `stash encrypt` flow (add -> backfill via @cipherstash/stack -> cutover -> drop) instead. See the comments in the rewritten SQL.', + ) + } + + if (skipped.length > 0) { + p.log.warn( + `${skipped.length} statement(s) look like an ALTER-to-encrypted the rewrite could not safely repair (e.g. a hand-authored SET DATA TYPE ... USING ...). Review them before migrating:`, + ) + for (const s of skipped) p.log.step(` - ${s.file}: ${s.statement}`) } } } diff --git a/packages/wizard/src/lib/rewrite-migrations.ts b/packages/wizard/src/lib/rewrite-migrations.ts index 24635f10e..ce3ce1ab6 100644 --- a/packages/wizard/src/lib/rewrite-migrations.ts +++ b/packages/wizard/src/lib/rewrite-migrations.ts @@ -1,60 +1,181 @@ +import { existsSync } from 'node:fs' import { readdir, readFile, writeFile } from 'node:fs/promises' -import { join } from 'node:path' +import { join, resolve } from 'node:path' /** - * Matches drizzle-kit's generated in-place type change to the encrypted - * column type. drizzle-kit's ALTER COLUMN path wraps the customType - * `dataType()` return value in double-quotes and prepends `"{typeSchema}".`. - * Custom types have no `typeSchema`, so we see several mangled forms - * depending on what `dataType()` returned. We match all of them: - * - * - bare `eql_v2_encrypted` → `"undefined"."eql_v2_encrypted"` - * - pre-quoted `"public"."eql_v2_encrypted"` (stack 0.15.0 regression) → - * `"undefined".""public"."eql_v2_encrypted""` - * - the plain `eql_v2_encrypted` and `"public"."eql_v2_encrypted"` forms, - * in case a future drizzle-kit release stops prepending undefined. + * The encrypted column types this rewrite applies to: the single EQL v2 type, + * and the whole EQL v3 concrete-domain family (`eql_v3_text_search`, + * `eql_v3_integer_ord`, …). The v3 side is matched by shape rather than by an + * enumerated list so a newly added domain is covered without touching this file + * — the `eql_v3_` prefix is unambiguous in a `SET DATA TYPE` position. + * + * v2 stays matched even though the wizard now scaffolds v3 exclusively: a user's + * migration directory can hold files generated against an older stack version, + * and existing v2 ciphertext stays valid — we still want their ALTER-to-encrypted + * statements repaired. + */ +const ENCRYPTED_DOMAIN = String.raw`eql_v2_encrypted|eql_v3_[a-z0-9_]+` + +/** + * Extracts the bare domain name out of whichever mangled form matched. Derived + * from {@link ENCRYPTED_DOMAIN} so the two can never drift out of sync — a new + * domain added to the alternation is extracted here for free. + */ +const DOMAIN_RE = new RegExp(ENCRYPTED_DOMAIN, 'i') + +/** + * The mangled forms drizzle-kit emits for a customType in an ALTER COLUMN. + * + * drizzle-kit's ALTER COLUMN path wraps the customType `dataType()` return + * value in double-quotes and prepends `"{typeSchema}".`. Custom types have no + * `typeSchema`, so the emitted SQL depends on BOTH what `dataType()` returned + * and which drizzle-kit renders it. Verified against 0.24.2, 0.28.1, 0.30.6, + * 0.31.0, 0.31.1 and 0.31.10 through `drizzle-kit/api`'s `generateMigration`: + * + * | `dataType()` returns | <= 0.30.6 | >= 0.31.0 | + * | ------------------------------- | --------------------------- | ---------------------------------- | + * | `eql_v3_text_search` (current) | `eql_v3_text_search` | `"undefined"."eql_v3_text_search"` | + * | `public.eql_v3_text_search` | `public.eql_v3_text_search` | `"undefined"."public.eql_v3_…"` | + * | `"public"."eql_v2_encrypted"` | `"public"."eql_v2_…"` | `"undefined".""public"."eql_v2_…"` | + * + * The `"undefined".` prefix is a **0.31.0-and-later** behaviour — 0.30.6 and + * earlier emit the plain form. All six forms stay matched because a user's + * migration directory can hold files generated by any drizzle-kit version + * against any stack version — including the qualified `public.eql_v3_*` that + * stack emitted before the unqualified fix. + * + * Ordered longest-first so the alternation cannot match a prefix of a longer + * form and leave a trailing fragment behind. + * + * NOTE: this file is the sibling of `packages/cli/src/commands/db/rewrite-migrations.ts`, + * which cli's `stash eql migration --drizzle` uses. Both are tightly coupled to + * drizzle-kit's output format — if drizzle-kit changes, both need updating + * together. Keep them in sync. + */ +const MANGLED_TYPE_FORMS = [ + String.raw`"undefined"\.""public"\."(?:${ENCRYPTED_DOMAIN})""`, + String.raw`"undefined"\."public\.(?:${ENCRYPTED_DOMAIN})"`, + String.raw`"undefined"\."(?:${ENCRYPTED_DOMAIN})"`, + String.raw`"public"\."(?:${ENCRYPTED_DOMAIN})"`, + String.raw`public\.(?:${ENCRYPTED_DOMAIN})`, + // Not emitted by any released drizzle-kit ALTER path, but it is the shape the + // CREATE TABLE path renders — guard it in case ALTER converges on it. + String.raw`"(?:${ENCRYPTED_DOMAIN})"`, + String.raw`(?:${ENCRYPTED_DOMAIN})`, +].join('|') + +/** + * Matches drizzle-kit's generated in-place type change to an encrypted column + * type, in any of the forms above. * * Captures: - * - $1: table name (without quotes) - * - $2: column name (without quotes) - * - * Note: a copy of this lives in `stash` (`db/rewrite-migrations.ts`) - * because cli's `eql install --drizzle` uses the same fix. Both copies are - * tightly coupled to drizzle-kit's output format — if drizzle-kit changes, - * both need to be updated together. - * - * The copies have DIVERGED as of #693: the `stash` one also matches the EQL v3 - * domain family (`eql_v3_*`) and two further mangled forms, and documents which - * drizzle-kit versions emit which. This copy stays v2-only because the wizard - * only ever scaffolds v2 columns. Port the v3 handling here if that changes. + * - $1: table name, OR the schema when a qualifier follows (both without quotes) + * - $2: table name when schema-qualified (`"app"."users"`), else undefined + * - $3: column name (without quotes) + * - $4: the mangled type blob — feed it to {@link DOMAIN_RE} for the bare name + * + * The optional `(?:\."([^"]+)")?` after the first quoted name matches the + * schema qualifier drizzle-kit emits for a `pgSchema()` table (`"app"."users"`). */ -const ALTER_COLUMN_TO_ENCRYPTED_RE = - /ALTER TABLE "([^"]+)"\s+ALTER COLUMN "([^"]+)"\s+SET DATA TYPE (?:"undefined"\.""public"\."eql_v2_encrypted""|"undefined"\."eql_v2_encrypted"|"public"\."eql_v2_encrypted"|eql_v2_encrypted)[^;]*;/gi +const ALTER_COLUMN_TO_ENCRYPTED_RE = new RegExp( + // `\s*;` — not `[^;]*;` — so the type blob must run straight into the + // statement terminator. drizzle-kit emits exactly that (no trailing clause), + // and the tighter tail means a HAND-AUTHORED `SET DATA TYPE … USING ;` + // is left untouched instead of being silently rewritten (and its USING + // discarded). + String.raw`ALTER TABLE "([^"]+)"(?:\."([^"]+)")?\s+ALTER COLUMN "([^"]+)"\s+SET DATA TYPE (${MANGLED_TYPE_FORMS})\s*;`, + 'gi', +) + +/** + * A deliberately BROAD scan for statements that look like an in-place change to + * an encrypted column but that the strict {@link ALTER_COLUMN_TO_ENCRYPTED_RE} + * did not rewrite — a hand-authored `SET DATA TYPE … USING …;`, or some future + * drizzle-kit form the strict matcher doesn't yet cover. + * + * It matches any single (`;`-terminated) statement that contains both + * `SET DATA TYPE` and an `eql_v2`/`eql_v3` domain token at a word boundary (so + * lookalikes like `eql_v4_…` or `not_eql_v3_…` don't trip it). Run AFTER the + * strict rewrite so genuinely-rewritten statements — which no longer contain + * `SET DATA TYPE` — are already gone; whatever this still finds is a near-miss + * we flag for human review rather than silently ship as broken SQL. + */ +const NEAR_MISS_RE = + /[^;]*?\bSET\s+DATA\s+TYPE\b[^;]*?\beql_v[23][a-z0-9_]*[^;]*?;/gi + +/** + * Blank lines and `--` line comments (including drizzle-kit's + * `--> statement-breakpoint`) at the head of a {@link NEAR_MISS_RE} match. + * + * That regex opens with a lazy `[^;]*?`, whose only left boundary is the + * previous `;` — or the start of the file when there is no preceding statement. + * So the raw match drags in every comment and blank line since then, and a + * near-miss in a file that opens with a comment block gets reported to the user + * with that whole block glued to its front. Strip the preamble so the statement + * we quote back reads as the offending statement alone. + * + * Only line comments are handled — `/* … *\/` block comments are not something + * drizzle-kit emits, and a stray one costs cosmetics, not correctness. + */ +const STATEMENT_PREAMBLE_RE = /^(?:[^\S\n]*(?:--[^\n]*)?\n)+/ + +/** Drop the leading blank/comment lines a `[^;]*?`-anchored match dragged in. */ +function trimStatementPreamble(statement: string): string { + return statement.replace(STATEMENT_PREAMBLE_RE, '').trim() +} + +/** A statement the sweep recognised as ALTER-to-encrypted but did NOT rewrite. */ +export interface SkippedAlter { + /** Absolute path of the migration file the statement lives in. */ + file: string + /** The offending statement, verbatim (trimmed), for the user to review. */ + statement: string +} + +/** Outcome of a sweep: the files rewritten, and near-misses left for review. */ +export interface RewriteResult { + /** Absolute paths of files whose unsafe ALTER COLUMN(s) were rewritten. */ + rewritten: string[] + /** Near-miss statements the strict matcher passed over — flag, don't guess. */ + skipped: SkippedAlter[] +} /** - * Replace in-place `ALTER COLUMN ... SET DATA TYPE eql_v2_encrypted` statements - * with an ADD + DROP + RENAME sequence. - * - * **Why this exists (CIP-2991, CIP-2994):** Postgres has no implicit cast from - * `text`/`numeric` to `eql_v2_encrypted`, so `ALTER COLUMN ... SET DATA TYPE - * eql_v2_encrypted` fails with `cannot cast type ... to eql_v2_encrypted`. - * The fix that works on both empty and non-empty tables is to add a new - * encrypted column, backfill it, drop the original, and rename the new - * column into place. For empty tables the UPDATE is a no-op and the - * sequence is effectively equivalent to DROP+ADD. - * - * We only rewrite the statement — the actual encryption of existing rows has - * to happen in application code (via `encryptModel` from - * `@cipherstash/stack`), which is why the UPDATE is emitted as a guidance - * comment rather than real SQL. Running this migration against a populated - * table leaves the new column NULL until the app backfills it. + * Replace in-place `ALTER COLUMN ... SET DATA TYPE ` + * statements with an ADD + DROP + RENAME sequence. + * + * **Why this exists (CIP-2991, CIP-2994, #693):** Postgres has no implicit cast + * from `text`/`numeric` to an encrypted domain, so `ALTER COLUMN ... SET DATA + * TYPE eql_v2_encrypted` (or any `eql_v3_*` domain) fails at migrate time with + * `cannot cast type ... to `. This applies equally to the single EQL v2 + * type and the whole EQL v3 concrete-domain family. + * + * The rewrite is an ADD+DROP+RENAME, which is **equivalent to DROP+ADD**: it + * makes the column type valid but does NOT preserve the column's data. It is + * therefore safe ONLY on an EMPTY table. On a populated table the new column + * starts NULL and the original is dropped in the same migration, so the + * plaintext is destroyed. The commented UPDATE is a placeholder that can never + * become real SQL (the encrypted value is the EQL envelope produced by ZeroKMS + * via the client — there is no expression Postgres can evaluate to fill it), so + * a populated table must instead use the staged `stash encrypt` lifecycle + * (add → backfill via `@cipherstash/stack`'s `encryptModel` → cutover → drop), + * which keeps both columns alive across deploys. Each rewritten file carries a + * header comment saying exactly this. + * + * Returns {@link RewriteResult}: the files rewritten, plus `skipped` near-misses + * — statements that look like an ALTER-to-encrypted but fall outside the strict + * matcher (a hand-authored `SET DATA TYPE … USING …;`, or a future drizzle-kit + * form). Near-misses are left untouched on disk and surfaced non-fatally so the + * caller can tell the user to review them, rather than silently shipping broken + * SQL. */ export async function rewriteEncryptedAlterColumns( outDir: string, options: { skip?: string } = {}, -): Promise { +): Promise { const entries = await readdir(outDir).catch(() => []) const rewritten: string[] = [] + const skipped: SkippedAlter[] = [] for (const entry of entries) { if (!entry.endsWith('.sql')) continue @@ -62,35 +183,154 @@ export async function rewriteEncryptedAlterColumns( if (options.skip && filePath === options.skip) continue const original = await readFile(filePath, 'utf-8') - if (!ALTER_COLUMN_TO_ENCRYPTED_RE.test(original)) continue // Reset the regex's lastIndex — it's stateful on /g ALTER_COLUMN_TO_ENCRYPTED_RE.lastIndex = 0 const updated = original.replace( ALTER_COLUMN_TO_ENCRYPTED_RE, - (_match, table: string, column: string) => renderSafeAlter(table, column), + ( + match: string, + first: string, + second: string | undefined, + column: string, + mangledType: string, + ) => { + // When schema-qualified (`"app"."users"`) the first capture is the + // schema and the second is the table; otherwise the first is the table. + const schema = second === undefined ? undefined : first + const table = second === undefined ? first : second + const domain = DOMAIN_RE.exec(mangledType)?.[0]?.toLowerCase() + // Unreachable — the outer regex only matches when a domain is present — + // but leave the statement alone rather than emit a broken rewrite. + if (!domain) return match + return renderSafeAlter(table, column, domain, schema) + }, ) if (updated !== original) { await writeFile(filePath, updated, 'utf-8') rewritten.push(filePath) } + + // Broad secondary scan on the POST-rewrite content: anything still carrying + // `SET DATA TYPE` near an eql_v2/eql_v3 token slipped past the strict + // matcher. Flag it — non-fatally — rather than leave the user shipping SQL + // that fails at migrate time. + for (const nearMiss of updated.matchAll(NEAR_MISS_RE)) { + skipped.push({ + file: filePath, + statement: trimStatementPreamble(nearMiss[0]), + }) + } } - return rewritten + return { rewritten, skipped } +} + +/** One candidate migration directory's outcome from {@link sweepMigrationDirs}. */ +export interface DirRewriteResult extends RewriteResult { + /** The candidate directory as supplied (relative to `cwd`), for reporting. */ + dir: string + /** Set when this directory's sweep threw; the sweep continues regardless. */ + error?: string } -function renderSafeAlter(table: string, column: string): string { +/** + * Sweep every candidate migration directory under `cwd`, rewriting each one's + * unsafe ALTER-to-encrypted statements. Directories that don't exist are + * skipped; the returned array carries one entry per directory that did. + * + * **Why every directory, not just the first:** a project's drizzle-kit `out` + * directory is not discoverable from here, so the caller passes a candidate + * list. Stopping at the first candidate that merely *exists* means an empty or + * already-rewritten `drizzle/` sitting next to the project's real `migrations/` + * silently leaves those migrations unrepaired — they then fail at migrate time + * with the very `cannot cast type ...` error this function exists to prevent. + * + * Sweeping all of them is safe because the rewrite is idempotent: a rewritten + * statement no longer contains `SET DATA TYPE`, so neither + * {@link ALTER_COLUMN_TO_ENCRYPTED_RE} nor {@link NEAR_MISS_RE} can match it a + * second time. Two candidates resolving to the same directory (via a symlink) + * therefore cost a redundant read, not a double transform. + * + * A directory that throws mid-sweep is reported via `error` rather than + * aborting — one unreadable candidate must not strand the others. + */ +export async function sweepMigrationDirs( + cwd: string, + dirs: readonly string[], +): Promise { + const results: DirRewriteResult[] = [] + + for (const dir of dirs) { + const abs = resolve(cwd, dir) + if (!existsSync(abs)) continue + + try { + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(abs) + results.push({ dir, rewritten, skipped }) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + results.push({ dir, rewritten: [], skipped: [], error: message }) + } + } + + return results +} + +/** + * The rewrite is identical for v2 and v3, and the ADD+DROP+RENAME sequence is + * equivalent to DROP+ADD: it makes the column type valid but does NOT preserve + * the column's data. It is therefore safe only on an EMPTY table. On a populated + * table the new column starts NULL and the old one is dropped in the same + * migration, so the plaintext is destroyed. + * + * The commented UPDATE is only a placeholder — it can never become real SQL. The + * encrypted value is the EQL envelope produced by ZeroKMS via the client + * (`encryptModel` / `bulkEncryptModels`); there is no expression Postgres can + * evaluate to fill it. (v3 stores that envelope as jsonb rather than v2's + * composite, but this is equally true on both surfaces.) + * + * So the guidance does NOT tell the user to backfill and run this migration — + * that would still lose data on cutover. It points a populated table at the + * staged `stash encrypt` lifecycle (add → backfill → cutover → drop), which + * keeps both columns alive across deploys. + */ +function renderSafeAlter( + table: string, + column: string, + domain: string, + schema?: string, +): string { const tmp = `${column}__cipherstash_tmp` + // Preserve the schema qualifier drizzle-kit emitted for pgSchema() tables so + // the rewritten statements target the same object. + const qualifiedTable = schema ? `"${schema}"."${table}"` : `"${table}"` return [ '-- Rewritten by @cipherstash/wizard: in-place ALTER COLUMN cannot cast to', - `-- eql_v2_encrypted. If "${table}" already has rows, backfill the new`, - "-- column via @cipherstash/stack's encryptModel in application code BEFORE", - '-- running this migration in production. Empty tables are safe as-is.', - `ALTER TABLE "${table}" ADD COLUMN "${tmp}" "public"."eql_v2_encrypted";`, - `-- UPDATE "${table}" SET "${tmp}" = /* encrypted value for ${column} */ NULL;`, - `ALTER TABLE "${table}" DROP COLUMN "${column}";`, - `ALTER TABLE "${table}" RENAME COLUMN "${tmp}" TO "${column}";`, + `-- ${domain}. This ADD+DROP+RENAME equals DROP+ADD and is safe ONLY if`, + `-- ${qualifiedTable} is empty. On a populated table it DESTROYS existing "${column}"`, + '-- data (the new column starts NULL) — do NOT run it there. Use the staged', + "-- `stash encrypt` path instead: add -> backfill via @cipherstash/stack's", + '-- encryptModel in application code -> cutover -> drop.', + '-- NOTE: constraints, defaults, and indexes on the original column are NOT', + '-- carried over by this ADD/DROP/RENAME — re-add any NOT NULL, DEFAULT,', + '-- UNIQUE, or index definitions manually.', + // The domain is emitted as `"public".""` unconditionally: EQL + // installs its domains into `public` (both `stash eql install` and the + // adapters' baseline migrations do), and the qualifier makes the rewrite + // independent of the session `search_path`. It is an ASSUMPTION, not + // something read back from the matched SQL — the `schema` capture above is + // the TABLE's schema (from a pgSchema() table) and says nothing about where + // the domain lives. If EQL ever supports installing into a non-`public` + // schema, this needs the install schema threaded in, here and in the + // sibling `packages/cli/src/commands/db/rewrite-migrations.ts`. + `ALTER TABLE ${qualifiedTable} ADD COLUMN "${tmp}" "public"."${domain}";`, + `-- UPDATE ${qualifiedTable} SET "${tmp}" = /* encrypted value for ${column} */ NULL`, + '--> statement-breakpoint', + `ALTER TABLE ${qualifiedTable} DROP COLUMN "${column}";`, + '--> statement-breakpoint', + `ALTER TABLE ${qualifiedTable} RENAME COLUMN "${tmp}" TO "${column}";`, ].join('\n') } diff --git a/packages/wizard/src/tools/wizard-tools.ts b/packages/wizard/src/tools/wizard-tools.ts index 3a589b7bf..f60673aa5 100644 --- a/packages/wizard/src/tools/wizard-tools.ts +++ b/packages/wizard/src/tools/wizard-tools.ts @@ -149,6 +149,25 @@ interface DbColumn { isEqlEncrypted: boolean } +/** + * Is a Postgres domain-type (`udt_name`) an EQL-encrypted column? + * + * Used purely for DETECTION — telling the agent "this column is already + * encrypted, don't scaffold over it". It therefore recognises BOTH generations: + * the legacy single `eql_v2_encrypted` type AND the EQL v3 concrete-domain + * family (`eql_v3_text_search`, `eql_v3_integer_ord`, …). The wizard now only + * *emits* v3, but a project may already carry v2-encrypted columns whose + * ciphertext stays valid — misreporting them as plaintext would let the agent + * clobber real encrypted data. + * + * The `eql_v3_` prefix (trailing underscore included) matches the convention in + * `@cipherstash/migrate`'s `classifyEqlDomain` — a bare `eql_v3` would also + * claim a hypothetical future major like `eql_v30_*`. + */ +function isEqlEncryptedDomain(udtName: string): boolean { + return udtName === 'eql_v2_encrypted' || udtName.startsWith('eql_v3_') +} + interface DbTable { tableName: string columns: DbColumn[] @@ -183,7 +202,7 @@ export async function introspectDatabase( columnName: row.column_name, dataType: row.data_type, udtName: row.udt_name, - isEqlEncrypted: row.udt_name === 'eql_v2_encrypted', + isEqlEncrypted: isEqlEncryptedDomain(row.udt_name), }) tableMap.set(row.table_name, cols) } diff --git a/packages/wizard/tsconfig.json b/packages/wizard/tsconfig.json index 1f60894ec..d88a9b847 100644 --- a/packages/wizard/tsconfig.json +++ b/packages/wizard/tsconfig.json @@ -7,6 +7,17 @@ "allowJs": true, "esModuleInterop": true, "moduleResolution": "bundler", + + // `@cipherstash/auth` uses conditional exports — `node` picks `index.d.ts` + // (the full surface, including `AutoStrategy`), `default` picks + // `wasm-types.d.ts` (which has no `AutoStrategy`). Bundler resolution does + // not include `node` by default, so without this the wizard's three + // `auth.AutoStrategy` call sites fail to typecheck even though they run + // fine — the wizard is a Node CLI and always loads the `node` branch. + // Matches `packages/stack`, `packages/prisma-next`, `packages/test-kit`, + // `packages/stack-drizzle` and `packages/stack-supabase`. + "customConditions": ["node"], + "allowImportingTsExtensions": true, "verbatimModuleSyntax": true, "noEmit": true,