diff --git a/.changeset/wild-donkeys-shave.md b/.changeset/wild-donkeys-shave.md new file mode 100644 index 000000000..df3c021d8 --- /dev/null +++ b/.changeset/wild-donkeys-shave.md @@ -0,0 +1,22 @@ +--- +'stash': patch +--- + +Fix invalid DDL when a Drizzle column changes to an EQL v3 domain. + +`drizzle-kit generate` emits an in-place `ALTER TABLE … ALTER COLUMN … SET DATA TYPE` +when a plaintext column is changed to an encrypted one, which Postgres rejects — there +is no cast from `text`/`numeric` to an EQL type, and on drizzle-kit 0.31.0+ the emitted +type name is additionally mangled to `"undefined"."eql_v3_"`. The migration +rewriter only recognised the EQL v2 type, so a v3 user was left with an un-runnable +migration and nothing to repair it. + +The rewriter now matches the whole `eql_v3_*` domain family alongside `eql_v2_encrypted`, +across every mangled form observed from drizzle-kit 0.24 through 0.31, and emits the +matched domain in the replacement instead of a hardcoded v2 type. `stash eql migration +--drizzle` — the EQL v3 migration-first path — now runs the same sweep that `eql install +--drizzle` has always run, so the repair actually reaches v3 projects. + +The rewrite's guidance comment now also warns that it drops the plaintext column in the +same migration, and points at the staged `stash encrypt` path (add → backfill → cutover → +drop) for populated production tables. diff --git a/packages/cli/src/__tests__/rewrite-migrations.test.ts b/packages/cli/src/__tests__/rewrite-migrations.test.ts index ad5d21b5e..e213cc886 100644 --- a/packages/cli/src/__tests__/rewrite-migrations.test.ts +++ b/packages/cli/src/__tests__/rewrite-migrations.test.ts @@ -20,7 +20,7 @@ describe('rewriteEncryptedAlterColumns', () => { const filePath = path.join(tmpDir, '0002_alter.sql') fs.writeFileSync(filePath, original) - const rewritten = await rewriteEncryptedAlterColumns(tmpDir) + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) expect(rewritten).toEqual([filePath]) const updated = fs.readFileSync(filePath, 'utf-8') @@ -51,6 +51,29 @@ describe('rewriteEncryptedAlterColumns', () => { 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' @@ -87,7 +110,7 @@ describe('rewriteEncryptedAlterColumns', () => { const filePath = path.join(tmpDir, '0001_init.sql') fs.writeFileSync(filePath, original) - const rewritten = await rewriteEncryptedAlterColumns(tmpDir) + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) expect(rewritten).toEqual([]) expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) @@ -102,7 +125,7 @@ describe('rewriteEncryptedAlterColumns', () => { 'ALTER TABLE "t" ALTER COLUMN "c" SET DATA TYPE eql_v2_encrypted;', ) - const rewritten = await rewriteEncryptedAlterColumns(tmpDir, { + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir, { skip: install, }) expect(rewritten).toEqual([alter]) @@ -111,8 +134,317 @@ describe('rewriteEncryptedAlterColumns', () => { it('returns an empty list when the directory does not exist', async () => { const missing = path.join(tmpDir, 'does-not-exist') - const rewritten = await rewriteEncryptedAlterColumns(missing) + const { rewritten } = await rewriteEncryptedAlterColumns(missing) + expect(rewritten).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. Verified against drizzle-kit 0.24.2, + // 0.28.1, 0.30.6, 0.31.0, 0.31.1 and 0.31.10 via `drizzle-kit/api`'s + // `generateMigration`: the `"undefined".` prefix appears in **0.31.0 and + // later** — 0.30.6 and earlier emit the plain form. (Issue #693 and PR #688 + // both describe it as an *older*-drizzle-kit artifact; that is backwards.) + const MANGLED_FORMS: Array<[label: string, emitted: string]> = [ + // dataType() → bare `eql_v3_text_search` (what stack emits post-#688) + ['plain, drizzle-kit <=0.30.6', 'eql_v3_text_search'], + [ + '"undefined"-prefixed, drizzle-kit >=0.31.0', + '"undefined"."eql_v3_text_search"', + ], + // dataType() → qualified `public.eql_v3_text_search` (stack pre-#688) + ['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"', + ], + // dataType() → pre-quoted `"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""', + ], + // Not observed from any released drizzle-kit, but the CREATE TABLE path + // renders this shape — guard it in case ALTER converges on it. + ['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.each([ + ['dotted, drizzle-kit <=0.30.6', 'public.eql_v2_encrypted'], + [ + 'dotted inside "undefined", drizzle-kit >=0.31.0', + '"undefined"."public.eql_v2_encrypted"', + ], + ])('rewrites the previously unmatched v2 %s form', async (_label, emitted) => { + const filePath = path.join(tmpDir, '0009_v2form.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_v2_encrypted";', + ) + 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.') + expect(updated).not.toContain('eql_v2_encrypted') + }) + + it('notes that constraints/defaults/indexes are not carried over', async () => { + const filePath = path.join(tmpDir, '0016_constraints.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v2_encrypted;\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + expect(updated).toContain('constraints, defaults, and indexes') + }) + + it('does not terminate the commented UPDATE placeholder with a semicolon', async () => { + // A runner that naively splits on `;` must not cut mid-comment. + const filePath = path.join(tmpDir, '0017_semicolon.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v2_encrypted;\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8') + const updateLine = updated + .split('\n') + .find( + (line) => line.includes('UPDATE') && line.includes('encrypted value'), + ) + expect(updateLine).toBeDefined() + expect(updateLine?.trimEnd().endsWith(';')).toBe(false) + }) + + it('separates ADD/DROP/RENAME with --> statement-breakpoint, one exec stmt per chunk', async () => { + const filePath = path.join(tmpDir, '0018_breakpoint.sql') + fs.writeFileSync( + filePath, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v2_encrypted;\n', + ) + + await rewriteEncryptedAlterColumns(tmpDir) + + const updated = fs.readFileSync(filePath, 'utf-8').trimEnd() + const chunks = updated.split('--> statement-breakpoint') + // Three executable statements: ADD, DROP, RENAME — one per chunk. + expect(chunks).toHaveLength(3) + for (const chunk of chunks) { + const execLines = chunk + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0 && !line.startsWith('--')) + expect(execLines).toHaveLength(1) + } + 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', async () => { + // A user who writes their own cast expression has a runnable statement we + // must not clobber — the tail is `\s*;`, not `[^;]*;`, precisely so the + // USING clause keeps this out of the match. + const original = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING encrypt(email);\n' + const filePath = path.join(tmpDir, '0013_using.sql') + fs.writeFileSync(filePath, original) + + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) + expect(rewritten).toEqual([]) + expect(fs.readFileSync(filePath, 'utf-8')).toBe(original) + }) + + it('reports a near-miss SET DATA TYPE as skipped and leaves it on disk', async () => { + // A hand-authored cast the strict regex won't rewrite (its USING tail keeps + // it out) — but it IS an ALTER-to-encrypted, so silently passing it over + // would ship broken SQL. The broad secondary scan must surface it. + 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, '0019_nearmiss.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 for a clean file', async () => { + const filePath = path.join(tmpDir, '0020_clean.sql') + fs.writeFileSync(filePath, 'CREATE TABLE "t" ("id" integer);\n') + + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(tmpDir) + + expect(rewritten).toEqual([]) + expect(skipped).toEqual([]) + }) + + 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([]) }) it('handles multiple ALTER statements in one file', async () => { diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 74d857222..6b61c4b2e 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -598,25 +598,44 @@ async function generateDrizzleMigration( // Step 5: Sweep for sibling migrations that drizzle-kit may have emitted // with `ALTER COLUMN ... SET DATA TYPE eql_v2_encrypted`. These fail in // Postgres because there's no implicit cast from text/numeric to the - // encrypted type. Rewrite them into the ADD/UPDATE/DROP/RENAME sequence - // that works on both empty and populated tables. CIP-2991 + CIP-2994. + // encrypted type. Rewrite them into a runnable ADD+DROP+RENAME sequence. + // That sequence is equivalent to DROP+ADD — safe on an EMPTY table, but + // data-destroying on a populated one — so the rewritten file carries a + // comment steering populated tables to the staged `stash encrypt` path. + // CIP-2991 + CIP-2994. + let sweepIncomplete = false try { - const rewritten = await rewriteEncryptedAlterColumns(outDir, { + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(outDir, { skip: generatedMigrationPath, }) if (rewritten.length > 0) { p.log.info( - `Rewrote ${rewritten.length} migration file(s) to use safe ADD+migrate+DROP for encrypted columns:`, + `Rewrote ${rewritten.length} migration file(s) into a runnable ADD+DROP+RENAME for encrypted columns (safe on empty tables; see each file's header before running against populated data):`, ) for (const file of rewritten) p.log.step(` - ${file}`) } + if (skipped.length > 0) { + sweepIncomplete = true + p.log.warn( + `Found ${skipped.length} ALTER-to-encrypted statement(s) the sweep could not rewrite automatically. Review and fix them before running your migrations:`, + ) + for (const { file, statement } of skipped) { + p.log.step(` - ${file}: ${statement}`) + } + } } catch (error) { + sweepIncomplete = true p.log.warn( `Could not rewrite ALTER COLUMN migrations: ${error instanceof Error ? error.message : String(error)}`, ) } p.log.success(`Migration created: ${generatedMigrationPath}`) + if (sweepIncomplete) { + p.log.warn( + `The ALTER COLUMN sweep did not fully complete — review the sibling migrations in ${outDir} before running drizzle-kit migrate, or you may apply broken/unsafe SQL.`, + ) + } p.note( `Run your Drizzle migrations to install EQL:\n\n ${runnerCommand(detectPackageManager(), '').trim()} drizzle-kit migrate`, 'Next Steps', diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index 9223f6c48..a96dc5229 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -2,49 +2,151 @@ import { readdir, readFile, writeFile } from 'node:fs/promises' import { join } 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: + * 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. + */ +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`: * - * - 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. + * | `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. (Issue #693 and PR #688 both describe it as an + * artifact of *older* drizzle-kit; that is backwards.) 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 #688. + * + * Ordered longest-first so the alternation cannot match a prefix of a longer + * form and leave a trailing fragment behind. + */ +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) + * - $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"`). + * A plain `\s+` between the two names could never cross the `.`, so those tables + * were silently passed over. + */ +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 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 NEAR_MISS_RE = + /[^;]*?\bSET\s+DATA\s+TYPE\b[^;]*?\beql_v[23][a-z0-9_]*[^;]*?;/gi + +/** 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. + * Replace in-place `ALTER COLUMN ... SET DATA TYPE ` + * 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. + * **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. * - * 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. + * 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 @@ -52,35 +154,91 @@ 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: nearMiss[0].trim() }) + } } - return rewritten + return { rewritten, skipped } } -function renderSafeAlter(table: string, column: string): string { +/** + * 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 stash: 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.', + `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/cli/src/commands/eql/__tests__/migration.test.ts b/packages/cli/src/commands/eql/__tests__/migration.test.ts index 5ca7b2a7b..0dfb80a8f 100644 --- a/packages/cli/src/commands/eql/__tests__/migration.test.ts +++ b/packages/cli/src/commands/eql/__tests__/migration.test.ts @@ -208,6 +208,95 @@ describe('eqlMigrationCommand — Drizzle', () => { expect(written).toContain('cs_migrations') }) + // drizzle-kit emits an un-runnable in-place `ALTER COLUMN ... SET DATA TYPE` + // when a plaintext column is changed to an encrypted one. `eql install + // --drizzle` has always swept the out directory for these; the v3 + // migration-first path must do the same, or a v3 user is left with a broken + // migration and nothing to fix it (#693). + it('rewrites a sibling migration with a broken v3 ALTER COLUMN', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + const sibling = join(out, '0001_encrypt-email.sql') + writeFileSync( + sibling, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_text_search";\n', + ) + spawnMock.mockImplementation(() => { + writeFileSync(join(out, '0002_install-eql.sql'), '') + return { status: 0, stdout: '', stderr: '' } + }) + + await eqlMigrationCommand({ drizzle: true, out }) + + const rewritten = readFileSync(sibling, 'utf-8') + expect(rewritten).toContain( + 'ALTER TABLE "users" ADD COLUMN "email__cipherstash_tmp" "public"."eql_v3_text_search";', + ) + expect(rewritten).not.toContain('SET DATA TYPE') + }) + + it('does not rewrite the EQL install migration it just generated', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + const generated = join(out, '0000_install-eql.sql') + // A sibling carrying the SAME statement — the differential that proves the + // sweep ran at all, rather than no-opping over the whole directory. + const sibling = join(out, '0001_encrypt-email.sql') + const unsafeAlter = + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."eql_v3_text_search";\n' + writeFileSync(sibling, unsafeAlter) + spawnMock.mockImplementation(() => { + fsWrite.real(generated, '') + return { status: 0, stdout: '', stderr: '' } + }) + // The install bundle contains no ALTER COLUMN of its own, so the skip would + // have nothing to skip and this test would pass with `skip` removed. Append + // one to whatever the command writes, so the skip is load-bearing. + fsWrite.spy.mockImplementation(((path: string, data: unknown, ...rest) => { + const content = + typeof data === 'string' && data.includes('EQL v3 schema creation') + ? `${data}\n${unsafeAlter}` + : data + return fsWrite.real(path, content as string, ...(rest as [])) + }) as typeof import('node:fs').writeFileSync) + + await eqlMigrationCommand({ drizzle: true, out }) + + // Skipped: the statement survives verbatim in the generated migration... + expect(readFileSync(generated, 'utf-8')).toContain(unsafeAlter) + // ...while the identical statement in the sibling was rewritten. + expect(readFileSync(sibling, 'utf-8')).not.toContain('SET DATA TYPE') + }) + + // When the sweep leaves near-misses it couldn't rewrite, the closing note + // must warn the user the sweep didn't fully complete — otherwise they run + // drizzle-kit migrate against un-swept, broken sibling SQL. + it('warns at the closing note when the sweep leaves skipped statements', async () => { + const out = join(tmp, 'drizzle') + mkdirSync(out, { recursive: true }) + // A hand-authored SET DATA TYPE ... USING the strict matcher won't rewrite, + // but the broad scan flags as a near-miss. + const sibling = join(out, '0001_nearmiss.sql') + writeFileSync( + sibling, + 'ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE eql_v3_text_search USING (email)::eql_v3_text_search;\n', + ) + spawnMock.mockImplementation(() => { + writeFileSync(join(out, '0002_install-eql.sql'), '') + return { status: 0, stdout: '', stderr: '' } + }) + + await eqlMigrationCommand({ drizzle: true, out }) + + // The near-miss statement is left untouched... + expect(readFileSync(sibling, 'utf-8')).toContain('SET DATA TYPE') + // ...and the closing note warns the sweep did not fully complete. + const warned = clack.log.warn.mock.calls.map((c) => String(c[0])) + expect(warned.some((msg) => msg.includes('did not fully complete'))).toBe( + true, + ) + }) + it('aborts (exit 1) when drizzle-kit exits non-zero', async () => { spawnMock.mockReturnValue({ status: 1, stdout: '', stderr: 'boom' }) await expect( diff --git a/packages/cli/src/commands/eql/migration.ts b/packages/cli/src/commands/eql/migration.ts index 19223ec28..ef42c39c7 100644 --- a/packages/cli/src/commands/eql/migration.ts +++ b/packages/cli/src/commands/eql/migration.ts @@ -9,6 +9,7 @@ import { findGeneratedMigration, printNextSteps, } from '@/commands/db/install.js' +import { rewriteEncryptedAlterColumns } from '@/commands/db/rewrite-migrations.js' import { detectPackageManager, execArgv, @@ -211,7 +212,54 @@ async function generateDrizzleEqlMigration( throw new CliExit(1) } + // Step 4 — sweep for sibling migrations drizzle-kit emitted with an in-place + // `ALTER COLUMN ... SET DATA TYPE `. Those fail in Postgres + // (no implicit cast from text/numeric to an EQL domain), so rewrite them into + // an ADD+DROP+RENAME sequence that is runnable. That sequence is equivalent to + // DROP+ADD — safe on an EMPTY table but data-destroying on a populated one — + // so the rewritten file carries a comment steering populated tables to the + // staged `stash encrypt` path. `eql install --drizzle` has always done this + // for v2; without it the v3 migration-first path leaves the user with broken + // SQL and no repair (#693). + // Whether the sweep failed outright or left near-misses it couldn't rewrite. + // Either way the user must review sibling migrations before running migrate, + // so surface it again at the closing note (below) — not just inline here. + let sweepIncomplete = false + try { + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(outDir, { + skip: migrationPath, + }) + if (rewritten.length > 0) { + p.log.info( + `Rewrote ${rewritten.length} migration file(s) into a runnable ADD+DROP+RENAME for encrypted columns (safe on empty tables; see each file's header before running against populated data):`, + ) + for (const file of rewritten) p.log.step(` - ${file}`) + } + if (skipped.length > 0) { + sweepIncomplete = true + p.log.warn( + `Found ${skipped.length} ALTER-to-encrypted statement(s) the sweep could not rewrite automatically. Review and fix them before running your migrations:`, + ) + for (const { file, statement } of skipped) { + p.log.step(` - ${file}: ${statement}`) + } + } + } catch (error) { + // Advisory: the install migration itself is already written and valid. + sweepIncomplete = true + p.log.warn( + `Could not sweep ${outDir} for unsafe ALTER COLUMN statements: ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + p.log.success(`Migration created: ${migrationPath}`) + if (sweepIncomplete) { + p.log.warn( + `The ALTER COLUMN sweep did not fully complete — review the sibling migrations in ${outDir} before running drizzle-kit migrate, or you may apply broken/unsafe SQL.`, + ) + } p.note( `Run your Drizzle migrations to install EQL v3:\n\n ${execCommand(pm)} drizzle-kit migrate`, 'Next Steps', diff --git a/packages/wizard/src/lib/rewrite-migrations.ts b/packages/wizard/src/lib/rewrite-migrations.ts index ea2544861..24635f10e 100644 --- a/packages/wizard/src/lib/rewrite-migrations.ts +++ b/packages/wizard/src/lib/rewrite-migrations.ts @@ -22,6 +22,11 @@ import { join } from 'node:path' * 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. */ 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 diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 8fe14a531..0878cd1bc 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -388,6 +388,8 @@ stash eql migration --drizzle --supabase # also grant eql_v3 to anon/authentic Pass exactly one of `--drizzle` / `--prisma`. The generated migration also installs the `cs_migrations` tracking schema, so one `drizzle-kit migrate` covers everything `stash encrypt …` needs. +After writing the migration, `--drizzle` sweeps the output directory for sibling migrations containing an in-place `ALTER COLUMN … SET DATA TYPE ` — drizzle-kit emits these when you change a plaintext column to an encrypted one, and Postgres rejects them (there is no cast from `text`/`numeric` to an EQL type). Each is rewritten into an `ADD COLUMN` + `DROP` + `RENAME` sequence and the rewritten files are listed. This is equivalent to DROP+ADD — it fixes the type but does **not** preserve data — so it is 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, destroying the plaintext; do not run it there. Use the staged `stash encrypt` path (add → backfill → cutover → drop) instead. + #### `eql upgrade` The install SQL is idempotent. `upgrade` checks the current version, re-runs it, and reports the new one. If EQL isn't installed it points you at `eql install`. Same `--supabase`, `--exclude-operator-family`, `--eql-version`, `--latest`, `--dry-run`, `--database-url` flags. diff --git a/skills/stash-drizzle/SKILL.md b/skills/stash-drizzle/SKILL.md index 3c2a6ebbd..4f39bfbdd 100644 --- a/skills/stash-drizzle/SKILL.md +++ b/skills/stash-drizzle/SKILL.md @@ -61,6 +61,8 @@ 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 and the rule that matters: the repair is data-destroying, so it is safe **only on an empty table**. For a table with live data, do **not** apply the swept migration; follow the staged flow in **Migrating an Existing Column to Encrypted** below instead. + ### Column Storage Each encrypted column is a concrete Postgres domain named `public.eql_v3_`: