From f628463543ef74ed3a0a384e0bef4063d1547c46 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 20 Jul 2026 15:39:36 +1000 Subject: [PATCH 1/6] fix(cli): rewrite ALTER COLUMN to EQL v3 domains, not just eql_v2_encrypted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drizzle-kit emits an in-place `ALTER TABLE ... ALTER COLUMN ... SET DATA TYPE` when a plaintext column is changed to an encrypted one. Postgres rejects it (no cast from text/numeric to an EQL type), and on drizzle-kit 0.31.0+ the type name is additionally mangled to `"undefined".""` because a customType has no `typeSchema`. `rewriteEncryptedAlterColumns` repaired this only for the v2 type, so an EQL v3 user got an un-runnable migration with nothing to fix it (#693). - Match the whole `eql_v3_*` domain family alongside `eql_v2_encrypted`, by shape rather than an enumerated list, so a new domain needs no edit here. - Capture the mangled type blob and thread the extracted domain through `renderSafeAlter`, which no longer hardcodes the v2 type. - Cover all six mangled forms, verified empirically against drizzle-kit 0.24.2 / 0.28.1 / 0.30.6 / 0.31.0 / 0.31.1 / 0.31.10 via `drizzle-kit/api`. The `"undefined".` prefix is a 0.31.0-AND-LATER behaviour; #693 and #688 both describe it as an older-drizzle-kit artifact, which is backwards. Two forms this adds (`public.eql_vN_...` and `"undefined"."public.eql_vN_..."`) were unmatched for v2 as well, and the latter is what stack emitted for v3 before #688. - Run the sweep from `stash eql migration --drizzle` too. That is the v3 migration-first path and it never called the rewriter, so the regex fix alone would not have reached a single v3 user. Backfill guidance: v3 stores the ZeroKMS-produced EQL jsonb envelope rather than v2's composite, but it is still not expressible in SQL, so the UPDATE stays a guidance comment on both surfaces. What is worth adding is that this sequence DROPS the plaintext column in the same migration, whereas the staged `stash encrypt` lifecycle keeps both alive across deploys — so the comment now points there for populated tables. The wizard's copy of the rewriter stays v2-only (it only scaffolds v2 columns); its cross-reference comment records the divergence. Closes #693 --- .changeset/wild-donkeys-shave.md | 22 +++ .../src/__tests__/rewrite-migrations.test.ts | 170 ++++++++++++++++++ .../cli/src/commands/db/rewrite-migrations.ts | 105 +++++++++-- .../commands/eql/__tests__/migration.test.ts | 42 +++++ packages/cli/src/commands/eql/migration.ts | 27 +++ packages/wizard/src/lib/rewrite-migrations.ts | 5 + skills/stash-cli/SKILL.md | 2 + skills/stash-drizzle/SKILL.md | 2 + 8 files changed, 359 insertions(+), 16 deletions(-) create mode 100644 .changeset/wild-donkeys-shave.md 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..e17dfecd4 100644 --- a/packages/cli/src/__tests__/rewrite-migrations.test.ts +++ b/packages/cli/src/__tests__/rewrite-migrations.test.ts @@ -115,6 +115,176 @@ describe('rewriteEncryptedAlterColumns', () => { 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') + }) + + // 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('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('handles multiple ALTER statements in one file', async () => { const original = [ 'ALTER TABLE "a" ALTER COLUMN "x" SET DATA TYPE eql_v2_encrypted;', diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index 9223f6c48..dc184cb58 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -2,24 +2,69 @@ 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. + */ +const DOMAIN_RE = /eql_v2_encrypted|eql_v3_[a-z0-9_]+/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_…"` | * - * - 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 `"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) + * - $3: the mangled type blob — feed it to {@link DOMAIN_RE} for the bare name */ -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( + String.raw`ALTER TABLE "([^"]+)"\s+ALTER COLUMN "([^"]+)"\s+SET DATA TYPE (${MANGLED_TYPE_FORMS})[^;]*;`, + 'gi', +) /** * Replace in-place `ALTER COLUMN ... SET DATA TYPE eql_v2_encrypted` statements @@ -59,7 +104,13 @@ export async function rewriteEncryptedAlterColumns( const updated = original.replace( ALTER_COLUMN_TO_ENCRYPTED_RE, - (_match, table: string, column: string) => renderSafeAlter(table, column), + (match: string, table: string, column: string, mangledType: string) => { + 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) + }, ) if (updated !== original) { @@ -71,14 +122,36 @@ export async function rewriteEncryptedAlterColumns( return rewritten } -function renderSafeAlter(table: string, column: string): string { +/** + * The rewrite is identical for v2 and v3, and so is the backfill guidance. + * + * v3 stores the encrypted EQL jsonb envelope rather than v2's composite, which + * might suggest the UPDATE could become real SQL — it cannot. The envelope is + * produced by ZeroKMS via the client (`encryptModel` / `bulkEncryptModels`), so + * there is no expression Postgres can evaluate to fill it, exactly as for v2. + * The UPDATE therefore stays a guidance comment on both surfaces. + * + * What the v3 rollout path does change is the warning worth giving. This + * sequence DROPS the plaintext column in the same migration; the staged + * `stash encrypt` lifecycle (add → backfill → cutover → drop) deliberately + * keeps both columns alive across deploys. On a populated production table that + * staged path is the right one, so say so instead of implying this migration + * implements it. + */ +function renderSafeAlter( + table: string, + column: string, + domain: string, +): string { const tmp = `${column}__cipherstash_tmp` return [ '-- Rewritten by stash: in-place ALTER COLUMN cannot cast to', - `-- eql_v2_encrypted. If "${table}" already has rows, backfill the new`, + `-- ${domain}. 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";`, + `-- This migration DROPS "${column}" — on a populated production table prefer`, + '-- the staged `stash encrypt` path (add -> backfill -> cutover -> drop).', + `ALTER TABLE "${table}" ADD COLUMN "${tmp}" "public"."${domain}";`, `-- UPDATE "${table}" SET "${tmp}" = /* encrypted value for ${column} */ NULL;`, `ALTER TABLE "${table}" DROP COLUMN "${column}";`, `ALTER TABLE "${table}" RENAME COLUMN "${tmp}" TO "${column}";`, diff --git a/packages/cli/src/commands/eql/__tests__/migration.test.ts b/packages/cli/src/commands/eql/__tests__/migration.test.ts index 5ca7b2a7b..732eb788d 100644 --- a/packages/cli/src/commands/eql/__tests__/migration.test.ts +++ b/packages/cli/src/commands/eql/__tests__/migration.test.ts @@ -208,6 +208,48 @@ 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') + spawnMock.mockImplementation(() => { + writeFileSync(generated, '') + return { status: 0, stdout: '', stderr: '' } + }) + + await eqlMigrationCommand({ drizzle: true, out }) + + // Untouched by the sweep: still the EQL v3 install bundle, verbatim. + expect(readFileSync(generated, 'utf-8')).toContain('EQL v3 schema creation') + }) + 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..63efa1221 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,6 +212,32 @@ 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 + // the ADD/backfill/DROP/RENAME sequence that works on empty and populated + // tables alike. `eql install --drizzle` has always done this for v2; without + // it here the v3 migration-first path leaves the user with broken SQL and no + // repair (#693). + try { + const rewritten = await rewriteEncryptedAlterColumns(outDir, { + skip: migrationPath, + }) + if (rewritten.length > 0) { + p.log.info( + `Rewrote ${rewritten.length} migration file(s) to use safe ADD+migrate+DROP for encrypted columns:`, + ) + for (const file of rewritten) p.log.step(` - ${file}`) + } + } catch (error) { + // Advisory: the install migration itself is already written and valid. + 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}`) p.note( `Run your Drizzle migrations to install EQL v3:\n\n ${execCommand(pm)} drizzle-kit migrate`, 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..96b7fb8eb 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` + backfill-comment + `DROP` + `RENAME` sequence and the rewritten files are listed. The rewrite **drops the plaintext column**, so it is safe on an empty table; on a populated production table 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..c5fda0e3c 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_` for that diff, 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`.) `stash eql migration --drizzle` sweeps the output directory and rewrites each such statement into `ADD COLUMN` + backfill-comment + `DROP` + `RENAME`. The encrypted value cannot be produced in SQL — it is the ZeroKMS-produced EQL envelope — so the backfill stays a comment: encrypt existing rows in application code (`encryptModel` / `bulkEncryptModels`) before running the migration. Because the rewrite drops the plaintext column in the same migration, it is safe on an empty table; for a populated production table use the staged `stash encrypt` path (add → backfill → cutover → drop) instead. + ### Column Storage Each encrypted column is a concrete Postgres domain named `public.eql_v3_`: From df59d2ee5e877495830c61f79ad8abfa07490d01 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Mon, 20 Jul 2026 17:44:10 +1000 Subject: [PATCH 2/6] test(cli): make the install-migration skip test load-bearing The test asserted only that the generated migration still contained the EQL bundle. The bundle carries no ALTER COLUMN of its own, so the sweep would never have rewritten it and the test passed with `skip` removed. Append an unsafe ALTER to what the command writes, and put the identical statement in a sibling file: the generated migration must keep it verbatim while the sibling gets rewritten. Verified it fails when `skip` is dropped. Found by CodeRabbit. --- .../commands/eql/__tests__/migration.test.ts | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/eql/__tests__/migration.test.ts b/packages/cli/src/commands/eql/__tests__/migration.test.ts index 732eb788d..f701184da 100644 --- a/packages/cli/src/commands/eql/__tests__/migration.test.ts +++ b/packages/cli/src/commands/eql/__tests__/migration.test.ts @@ -239,15 +239,33 @@ describe('eqlMigrationCommand — Drizzle', () => { 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(() => { - writeFileSync(generated, '') + 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 }) - // Untouched by the sweep: still the EQL v3 install bundle, verbatim. - expect(readFileSync(generated, 'utf-8')).toContain('EQL v3 schema creation') + // 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') }) it('aborts (exit 1) when drizzle-kit exits non-zero', async () => { From 81e0957c995fc5c7a74e2009983b8370de64e963 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 21 Jul 2026 10:27:26 +1000 Subject: [PATCH 3/6] fix(cli): correct backfill guidance and tighten the ALTER match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on the rewrite: - The guidance comment told the user to "backfill the new column BEFORE running this migration" — impossible, since the temp column is created BY this migration's ADD COLUMN and the original is dropped in the same run. The sequence is equivalent to DROP+ADD: it fixes the type but does not preserve data, so it is safe ONLY on an empty table. Reworded the comment (and the two skills) to say exactly that and to point a populated table at the staged `stash encrypt` path instead. - Narrow the match tail from `[^;]*;` to `\s*;` so a hand-authored `SET DATA TYPE USING ;` — a runnable statement the user wrote deliberately — is left untouched rather than silently rewritten with its USING clause discarded. drizzle-kit's own output has no trailing clause, so this changes nothing for the real broken forms. Added a regression test (fails under the old `[^;]*;`). Found by CodeRabbit. --- .../src/__tests__/rewrite-migrations.test.ts | 15 +++++++ .../cli/src/commands/db/rewrite-migrations.ts | 43 +++++++++++-------- skills/stash-cli/SKILL.md | 2 +- skills/stash-drizzle/SKILL.md | 2 +- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/__tests__/rewrite-migrations.test.ts b/packages/cli/src/__tests__/rewrite-migrations.test.ts index e17dfecd4..ea8b97073 100644 --- a/packages/cli/src/__tests__/rewrite-migrations.test.ts +++ b/packages/cli/src/__tests__/rewrite-migrations.test.ts @@ -285,6 +285,21 @@ describe('rewriteEncryptedAlterColumns', () => { 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('handles multiple ALTER statements in one file', async () => { const original = [ 'ALTER TABLE "a" ALTER COLUMN "x" SET DATA TYPE eql_v2_encrypted;', diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index dc184cb58..e5f6a7e6f 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -62,7 +62,12 @@ const MANGLED_TYPE_FORMS = [ * - $3: the mangled type blob — feed it to {@link DOMAIN_RE} for the bare name */ const ALTER_COLUMN_TO_ENCRYPTED_RE = new RegExp( - String.raw`ALTER TABLE "([^"]+)"\s+ALTER COLUMN "([^"]+)"\s+SET DATA TYPE (${MANGLED_TYPE_FORMS})[^;]*;`, + // `\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', ) @@ -123,20 +128,22 @@ export async function rewriteEncryptedAlterColumns( } /** - * The rewrite is identical for v2 and v3, and so is the backfill guidance. + * 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. * - * v3 stores the encrypted EQL jsonb envelope rather than v2's composite, which - * might suggest the UPDATE could become real SQL — it cannot. The envelope is - * produced by ZeroKMS via the client (`encryptModel` / `bulkEncryptModels`), so - * there is no expression Postgres can evaluate to fill it, exactly as for v2. - * The UPDATE therefore stays a guidance comment on both surfaces. + * 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.) * - * What the v3 rollout path does change is the warning worth giving. This - * sequence DROPS the plaintext column in the same migration; the staged - * `stash encrypt` lifecycle (add → backfill → cutover → drop) deliberately - * keeps both columns alive across deploys. On a populated production table that - * staged path is the right one, so say so instead of implying this migration - * implements it. + * 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, @@ -146,11 +153,11 @@ function renderSafeAlter( const tmp = `${column}__cipherstash_tmp` return [ '-- Rewritten by stash: in-place ALTER COLUMN cannot cast to', - `-- ${domain}. 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.', - `-- This migration DROPS "${column}" — on a populated production table prefer`, - '-- the staged `stash encrypt` path (add -> backfill -> cutover -> drop).', + `-- ${domain}. This ADD+DROP+RENAME equals DROP+ADD and is safe ONLY if`, + `-- "${table}" 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.', `ALTER TABLE "${table}" ADD COLUMN "${tmp}" "public"."${domain}";`, `-- UPDATE "${table}" SET "${tmp}" = /* encrypted value for ${column} */ NULL;`, `ALTER TABLE "${table}" DROP COLUMN "${column}";`, diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 96b7fb8eb..0878cd1bc 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -388,7 +388,7 @@ 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` + backfill-comment + `DROP` + `RENAME` sequence and the rewritten files are listed. The rewrite **drops the plaintext column**, so it is safe on an empty table; on a populated production table use the staged `stash encrypt` path (add → backfill → cutover → drop) instead. +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` diff --git a/skills/stash-drizzle/SKILL.md b/skills/stash-drizzle/SKILL.md index c5fda0e3c..22baa06a9 100644 --- a/skills/stash-drizzle/SKILL.md +++ b/skills/stash-drizzle/SKILL.md @@ -61,7 +61,7 @@ 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_` for that diff, 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`.) `stash eql migration --drizzle` sweeps the output directory and rewrites each such statement into `ADD COLUMN` + backfill-comment + `DROP` + `RENAME`. The encrypted value cannot be produced in SQL — it is the ZeroKMS-produced EQL envelope — so the backfill stays a comment: encrypt existing rows in application code (`encryptModel` / `bulkEncryptModels`) before running the migration. Because the rewrite drops the plaintext column in the same migration, it is safe on an empty table; for a populated production table use the staged `stash encrypt` path (add → backfill → cutover → drop) instead. +**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_` for that diff, 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`.) `stash eql migration --drizzle` sweeps the output directory and rewrites each such statement into `ADD COLUMN` + `DROP` + `RENAME`. This sequence is equivalent to DROP+ADD: it makes the type valid 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 — the plaintext is destroyed. Do not run it there. Instead use the staged `stash encrypt` path — add the encrypted column, backfill via application code (`encryptModel` / `bulkEncryptModels`), cut over reads, then drop the plaintext column — which keeps both columns alive across deploys. (The encrypted value is the ZeroKMS-produced EQL envelope and cannot be produced in SQL, which is why the rewrite leaves the backfill to application code rather than emitting a runnable `UPDATE`.) ### Column Storage From a71b9a870dea1d97cd82ac150e199fdcfdea19c1 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 21 Jul 2026 10:34:25 +1000 Subject: [PATCH 4/6] docs(cli): stop the sweep messages claiming populated-table safety MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Step 4/5 comment and `p.log.info` on both sweep call sites (`eql migration --drizzle` and `eql install --drizzle`) still said the rewrite "works on empty and populated tables alike" / was a "safe ADD+migrate+DROP" — the same overclaim just corrected in the rewriter and skills. The sequence is equivalent to DROP+ADD: runnable, safe on an empty table, data-destroying on a populated one. Reword both to say so and to point at each file's header comment (which steers populated tables to the staged `stash encrypt` path). Found by CodeRabbit. Two further findings on the same review — the rewrite not preserving column constraints, and the regex not being SQL-context-aware — are limitations of the shared v2 rewriter this change inherits rather than defects in it; deferred to the standalone repair-command follow-up (#710) rather than redesigning shipped v2 behaviour inside a v3 bugfix. --- packages/cli/src/commands/db/install.ts | 9 ++++++--- packages/cli/src/commands/eql/migration.ts | 12 +++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 74d857222..5fb1d4120 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -598,15 +598,18 @@ 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. try { const rewritten = 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}`) } diff --git a/packages/cli/src/commands/eql/migration.ts b/packages/cli/src/commands/eql/migration.ts index 63efa1221..317b71bcc 100644 --- a/packages/cli/src/commands/eql/migration.ts +++ b/packages/cli/src/commands/eql/migration.ts @@ -215,17 +215,19 @@ async function generateDrizzleEqlMigration( // 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 - // the ADD/backfill/DROP/RENAME sequence that works on empty and populated - // tables alike. `eql install --drizzle` has always done this for v2; without - // it here the v3 migration-first path leaves the user with broken SQL and no - // repair (#693). + // 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). try { const rewritten = await rewriteEncryptedAlterColumns(outDir, { skip: migrationPath, }) 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}`) } From 17a5dfef8489efc5abeac1e1c280c49752eda0a3 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 21 Jul 2026 16:24:50 +1000 Subject: [PATCH 5/6] fix(cli): address PR review on the v3 ALTER COLUMN rewriter Respond to review feedback on the migration rewriter: - Match schema-qualified tables (app.users from pgSchema()); thread the schema through renderSafeAlter so every emitted statement stays qualified. - Derive DOMAIN_RE from ENCRYPTED_DOMAIN so the domain alternation has one source of truth (silent-miss if they drift). - Warn in the emitted guidance that NOT NULL, DEFAULT, UNIQUE, and indexes are not carried over by the ADD/DROP/RENAME. - Drop the trailing ; from the commented UPDATE placeholder so naive semicolon-splitting runners can't cut mid-comment. - Rewrite the exported docblock: the sequence destroys data and is empty-table only, matching renderSafeAlter; covers both v2 and the v3 domain family. - Report near-miss encrypted ALTERs the strict regex didn't rewrite (skipped) instead of passing them over silently; return { rewritten, skipped }. - Insert --> statement-breakpoint between emitted statements so Neon HTTP / postgres.js prepared-mode drivers don't reject multi-command chunks. - Surface an incomplete sweep in the closing next-steps note, not just an inline warning sandwiched between success messages. - Reduce the duplicated plaintext->encrypted warning in stash-drizzle to the drizzle-specific detail plus a cross-reference; stash-cli owns the full safety explanation. --- .../src/__tests__/rewrite-migrations.test.ts | 161 +++++++++++++++++- packages/cli/src/commands/db/install.ts | 10 +- .../cli/src/commands/db/rewrite-migrations.ts | 138 +++++++++++---- .../commands/eql/__tests__/migration.test.ts | 29 ++++ packages/cli/src/commands/eql/migration.ts | 21 ++- skills/stash-drizzle/SKILL.md | 2 +- 6 files changed, 321 insertions(+), 40 deletions(-) diff --git a/packages/cli/src/__tests__/rewrite-migrations.test.ts b/packages/cli/src/__tests__/rewrite-migrations.test.ts index ea8b97073..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,7 +134,7 @@ 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([]) }) @@ -152,7 +175,7 @@ describe('rewriteEncryptedAlterColumns', () => { `ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "undefined"."${domain}";\n`, ) - const rewritten = await rewriteEncryptedAlterColumns(tmpDir) + const { rewritten } = await rewriteEncryptedAlterColumns(tmpDir) expect(rewritten).toEqual([filePath]) const updated = fs.readFileSync(filePath, 'utf-8') @@ -166,6 +189,29 @@ describe('rewriteEncryptedAlterColumns', () => { 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 @@ -248,6 +294,64 @@ describe('rewriteEncryptedAlterColumns', () => { 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( @@ -279,7 +383,7 @@ describe('rewriteEncryptedAlterColumns', () => { const filePath = path.join(tmpDir, '0012_other.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) @@ -294,12 +398,55 @@ describe('rewriteEncryptedAlterColumns', () => { const filePath = path.join(tmpDir, '0013_using.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) + }) + + 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 () => { const original = [ 'ALTER TABLE "a" ALTER COLUMN "x" SET DATA TYPE eql_v2_encrypted;', diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 5fb1d4120..17db8c86a 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -604,7 +604,7 @@ async function generateDrizzleMigration( // comment steering populated tables to the staged `stash encrypt` path. // CIP-2991 + CIP-2994. try { - const rewritten = await rewriteEncryptedAlterColumns(outDir, { + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(outDir, { skip: generatedMigrationPath, }) if (rewritten.length > 0) { @@ -613,6 +613,14 @@ async function generateDrizzleMigration( ) for (const file of rewritten) p.log.step(` - ${file}`) } + if (skipped.length > 0) { + 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) { p.log.warn( `Could not rewrite ALTER COLUMN migrations: ${error instanceof Error ? error.message : String(error)}`, diff --git a/packages/cli/src/commands/db/rewrite-migrations.ts b/packages/cli/src/commands/db/rewrite-migrations.ts index e5f6a7e6f..a96dc5229 100644 --- a/packages/cli/src/commands/db/rewrite-migrations.ts +++ b/packages/cli/src/commands/db/rewrite-migrations.ts @@ -11,9 +11,11 @@ import { join } from 'node:path' const ENCRYPTED_DOMAIN = String.raw`eql_v2_encrypted|eql_v3_[a-z0-9_]+` /** - * Extracts the bare domain name out of whichever mangled form matched. + * 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 = /eql_v2_encrypted|eql_v3_[a-z0-9_]+/i +const DOMAIN_RE = new RegExp(ENCRYPTED_DOMAIN, 'i') /** * The mangled forms drizzle-kit emits for a customType in an ALTER COLUMN. @@ -57,9 +59,15 @@ const MANGLED_TYPE_FORMS = [ * type, in any of the forms above. * * Captures: - * - $1: table name (without quotes) - * - $2: column name (without quotes) - * - $3: the mangled type blob — feed it to {@link DOMAIN_RE} for the bare name + * - $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 @@ -67,34 +75,78 @@ const ALTER_COLUMN_TO_ENCRYPTED_RE = new RegExp( // 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*;`, + String.raw`ALTER TABLE "([^"]+)"(?:\."([^"]+)")?\s+ALTER COLUMN "([^"]+)"\s+SET DATA TYPE (${MANGLED_TYPE_FORMS})\s*;`, 'gi', ) /** - * Replace in-place `ALTER COLUMN ... SET DATA TYPE eql_v2_encrypted` statements - * with an ADD + DROP + RENAME sequence. + * 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 + +/** 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 ` + * 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 @@ -102,19 +154,28 @@ 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: string, table: string, column: string, mangledType: string) => { + ( + 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) + return renderSafeAlter(table, column, domain, schema) }, ) @@ -122,9 +183,17 @@ export async function rewriteEncryptedAlterColumns( 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 } } /** @@ -149,18 +218,27 @@ 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', `-- ${domain}. This ADD+DROP+RENAME equals DROP+ADD and is safe ONLY if`, - `-- "${table}" is empty. On a populated table it DESTROYS existing "${column}"`, + `-- ${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.', - `ALTER TABLE "${table}" ADD COLUMN "${tmp}" "public"."${domain}";`, - `-- UPDATE "${table}" SET "${tmp}" = /* encrypted value for ${column} */ NULL;`, - `ALTER TABLE "${table}" DROP COLUMN "${column}";`, - `ALTER TABLE "${table}" RENAME COLUMN "${tmp}" TO "${column}";`, + '-- 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 f701184da..0dfb80a8f 100644 --- a/packages/cli/src/commands/eql/__tests__/migration.test.ts +++ b/packages/cli/src/commands/eql/__tests__/migration.test.ts @@ -268,6 +268,35 @@ describe('eqlMigrationCommand — Drizzle', () => { 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 317b71bcc..ef42c39c7 100644 --- a/packages/cli/src/commands/eql/migration.ts +++ b/packages/cli/src/commands/eql/migration.ts @@ -221,8 +221,12 @@ async function generateDrizzleEqlMigration( // 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 = await rewriteEncryptedAlterColumns(outDir, { + const { rewritten, skipped } = await rewriteEncryptedAlterColumns(outDir, { skip: migrationPath, }) if (rewritten.length > 0) { @@ -231,8 +235,18 @@ async function generateDrizzleEqlMigration( ) 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) @@ -241,6 +255,11 @@ async function generateDrizzleEqlMigration( } 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/skills/stash-drizzle/SKILL.md b/skills/stash-drizzle/SKILL.md index 22baa06a9..4f39bfbdd 100644 --- a/skills/stash-drizzle/SKILL.md +++ b/skills/stash-drizzle/SKILL.md @@ -61,7 +61,7 @@ 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_` for that diff, 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`.) `stash eql migration --drizzle` sweeps the output directory and rewrites each such statement into `ADD COLUMN` + `DROP` + `RENAME`. This sequence is equivalent to DROP+ADD: it makes the type valid 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 — the plaintext is destroyed. Do not run it there. Instead use the staged `stash encrypt` path — add the encrypted column, backfill via application code (`encryptModel` / `bulkEncryptModels`), cut over reads, then drop the plaintext column — which keeps both columns alive across deploys. (The encrypted value is the ZeroKMS-produced EQL envelope and cannot be produced in SQL, which is why the rewrite leaves the backfill to application code rather than emitting a runnable `UPDATE`.) +**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 From 8dcbf05390020d7fe8ffca6aa7d2bbee3d41fc73 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Tue, 21 Jul 2026 17:48:20 +1000 Subject: [PATCH 6/6] fix(cli): re-surface incomplete ALTER sweep at install closing note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The v2 install path (generateDrizzleMigration) warned inline on a skipped statement or a thrown sweep, then fell through to the 'Migration created' success line and the 'run drizzle-kit migrate' note with no re-surfacing — so the warning scrolled past and the user was steered to migrate against un-swept, broken sibling SQL. Mirror the sweepIncomplete flag + closing-note warning already used in the v3 stash eql migration --drizzle path (migration.ts), giving the two migration surfaces parity. Addresses the CodeRabbit parity nitpick on #728. --- packages/cli/src/commands/db/install.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 17db8c86a..6b61c4b2e 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -603,6 +603,7 @@ async function generateDrizzleMigration( // 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, skipped } = await rewriteEncryptedAlterColumns(outDir, { skip: generatedMigrationPath, @@ -614,6 +615,7 @@ async function generateDrizzleMigration( 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:`, ) @@ -622,12 +624,18 @@ async function generateDrizzleMigration( } } } 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',