Repository navigation
Migration rewriter: dollar-quoted DDL is invisible to the corpus index, so the already-encrypted guard is bypassed and a live DROP COLUMN is emitted over ciphertext #811
Description
The A-2 fail-closed work (#783, #785) made the migration rewriter refuse to rewrite a column it cannot prove is plaintext, by building a corpus index of every encrypted-column declaration and skipping with reason: 'already-encrypted' when it finds one. That index is blind to DDL inside a dollar-quoted body, so a column declared or renamed inside DO $$ … END $$; never enters it — and the rewriter then emits a live ALTER TABLE … DROP COLUMN over ciphertext while reporting skipped: [].
This is worse than an unguarded path: it is a guard that fails silently and reports success. Nothing warns the user, and rewritten lists the file as successfully handled.
Verified on remove-v2 @ a0acce8c, on a pure EQL v3 corpus with no v2 token anywhere. Both the CLI and wizard copies reproduce identically.
Mechanism
isInsideCommentOrString (packages/cli/src/commands/db/rewrite-migrations.ts:189, dollar-quote branch at :219 via dollarQuoteDelimiter at :260) skips a dollar-quoted body whole. That is correct for the rewrite pass — you must not rewrite SQL inside a function body.
It is wrong for the index pass. indexColumnDeclarations (:552) gates all four of its scans on the same predicate:
- CREATE TABLE head —
:557 - ADD ENCRYPTED COLUMN —
:589 - ADD COLUMN —
:595 - RENAME COLUMN —
:604
But DO $$ … END $$; is executed SQL, not inert. A column added or renamed inside one is really there in the database; it is merely invisible to the index. Confirmed against Postgres 14.19:
DO $$ BEGIN ... ALTER TABLE "users" RENAME COLUMN "email_encrypted" TO "email"; ... END $$;
DO
column_name | data_type | domain_name
-------------+-----------+--------------------
email | jsonb | eql_v3_text_search
The predicate conflates two different things: inert (comment, string literal — never executes) and opaque (dollar-quoted body — executes, but must not be rewritten).
Wizard copy: packages/wizard/src/lib/rewrite-migrations.ts:197, :560. ENCRYPTED_DOMAIN is already fully v3-aware in both copies (cli:11, wizard:17), so this is the blindness, not the generation.
Reproduction — two files, no EQL v2
-- 0000_setup.sql
CREATE TABLE "users" ("id" serial PRIMARY KEY NOT NULL, "email" text NOT NULL);
ALTER TABLE "users" ADD COLUMN "email_encrypted" "eql_v3_text_search";
DO $$ BEGIN
ALTER TABLE "users" RENAME COLUMN "email" TO "email_old";
ALTER TABLE "users" RENAME COLUMN "email_encrypted" TO "email";
END $$;
-- 0001_change_domain.sql
ALTER TABLE "users" ALTER COLUMN "email" SET DATA TYPE "eql_v3_text_eq";Running the real exported rewriteEncryptedAlterColumns, with a control that differs only in the dollar quoting:
CASE 1a v3 + DOLLAR-QUOTED rename
rewritten: [ '0001_change_domain.sql' ]
skipped: []
emitted: ALTER TABLE "users" DROP COLUMN "email";
>>> DATA LOSS
CASE 1b v3 + PLAIN rename (identical otherwise)
rewritten: []
skipped: [{ reason: 'already-encrypted', ... }]
>>> correctly refused
Applying the emitted SQL to a populated table nulled email on every row. The dollar quoting alone flips the verdict.
Related blind spots, same mechanism
| Corpus shape | Result |
|---|---|
ADD COLUMN "x" "eql_v3_*" inside DO $$ |
data loss |
$tag$ … $tag$ rename (custom tag) |
data loss |
Unterminated $$ earlier in the same file |
data loss — blinds every declaration after it |
Unterminated $$ in a different file |
safe (the scan is per-file) |
CREATE TABLE inside DO $$ |
safe — falls through to source-unknown |
Why this is reachable on a pure v3 path
drizzle-kit does not emit DO $$ for column DDL — checked 0.20.18, 0.24.2, 0.28.1, 0.30.6, 0.31.10. It wraps only CREATE TYPE … AS ENUM and ADD CONSTRAINT … FOREIGN KEY, neither of which declares or renames a column. So the trigger is user-written SQL — but two things make it a normal thing for a v3 user to write:
DO $$ … END $$is the standard idempotent-migration idiom, and the tidy-up rename it is usually used for (email_encrypted→email, after the stagedstash encryptlifecycle completes) is the natural last step of the flow we document.- We normalise the idiom ourselves.
stash encrypt drop's v3 path writes a$stash_drop$block into the same migrations directory (packages/cli/src/commands/encrypt/drop.ts:252-285).
Run on a realistic v3 lifecycle corpus (drizzle-kit ADD COLUMN + stash's own $stash_drop$ block + a tidy rename):
tidy-rename written PLAIN -> skipped ['already-encrypted'] safe
tidy-rename written IDEMPOTENT -> skipped [] *** DATA LOSS ***
The originally-reported route was stash encrypt cutover, which scaffolds a Drizzle migration (cutover.ts:229-236) and wraps both renames in DO $$ (cutover.ts:305-331). That route is EQL v2-only — v3 returns at cutover.ts:147 before the scaffold — so it dies with the v2 removal (#707). The defect does not.
Why the tests did not catch it
Existing dollar-quote coverage tests quote parity, using a body that contains no DDL:
CREATE FUNCTION note() RETURNS text AS $$ don't $$packages/cli/src/__tests__/rewrite-migrations.test.ts:835,902 and packages/wizard/src/__tests__/rewrite-migrations.test.ts:763,830. Nothing exercises a dollar-quoted body that contains a declaration, which is exactly the case the index pass gets wrong.
The guard is load-bearing, not decorative
Postgres refuses text → eql_v3_text_search automatically:
ERROR: column "email_plaintext" cannot be cast automatically to type eql_v3_text_search
So the rewriter genuinely needs the ADD/DROP/RENAME shape on the v3 path, and that emission is destructive on a populated table by design. already-encrypted is the only thing standing between a v3 user and data loss here.
Suggested fix
Split the two concerns the predicate currently conflates. Keep skipping dollar-quoted bodies in the rewrite pass; in the index pass either
- scan dollar-quoted bodies for column declarations and record them normally, or
- fail closed — mark any table touched inside a dollar-quoted body as
source-unknown, so the rewriter refuses and reports rather than silently emitting.
(2) is the smaller change and matches the fail-closed policy #783/#785 established. (1) is better UX because it keeps the legitimate plaintext→v3 rewrite working for users who write idempotent migrations.
Either way, add coverage for a dollar-quoted body containing DDL — ADD COLUMN, RENAME COLUMN, custom $tag$, and an unterminated $$ earlier in the same file — in both copies. scripts/__tests__/rewriter-copies-in-sync.test.mjs will keep them aligned.
Relationship to other issues
- Drizzle sweep: partial-write-then-throw is reported as unverified, not destructive #786 — sibling. Both are fail-open paths the A-2 work did not reach. Drizzle sweep: partial-write-then-throw is reported as unverified, not destructive #786 needs a filesystem fault (the sweep throws after writing file N−1, the accumulated
rewrittenarray is discarded, and the wizard takes the softer prompt arm). This one needs nothing to go wrong: it is the happy path, and it reportsskipped: []. Worth fixing together; different root causes. - Drizzle: no standalone repair for a broken encrypted ALTER COLUMN migration #710 (closed) — about when the sweep runs, and a
jsonb-coercion hazard on re-running an applied migration. Unrelated mechanism. - drizzle-kit v3: ALTER COLUMN to an eql_v3 domain emits invalid DDL (no v3 rewriter) #693 (closed) — taught the matcher the
eql_v3_*domain family, which is why the corpus index is v3-aware today. The blindness is one layer below that fix.
Note on main
main's rewriter is 244 lines with no corpus index, no isInsideCommentOrString and no skip reasons; it emits DROP COLUMN unconditionally (packages/cli/src/commands/db/rewrite-migrations.ts:240 on main). So this exact defect does not exist there — main has the broader unguarded problem that #783/#785 were built to close. Fix belongs on remove-v2 (or wherever that work lands), not as a main backport.
Verified by executing the branch's real exported functions against a live Postgres 14.19, with matched control runs isolating the dollar quoting as the sole cause.
Activity
test(cli,wizard): cover dollar-quoted DDL regression (#811)
test(cli,wizard): cover dollar-quoted DDL regression (#811)
tobyhede commentedon Jul 29, 2026
Correction to the closure (2026-07-30)
This was closed as completed alongside #813 when #823 merged. That overstates what happened: #823 mitigated this, it did not fix the mechanism.
What #823 fixed is the consequence.
renderSafeAlteris now add-only, so the "liveDROP COLUMNemitted over ciphertext" in this issue's title can no longer happen. The severity drops from data loss to a silent wrong outcome.What survives is the mechanism this issue actually reported — the corpus index is still blind to dollar-quoted DDL, so the
already-encryptedguard is still bypassed.indexColumnDeclarationsstill gates five of its six scans onisInsideCommentOrString(packages/cli/src/commands/db/rewrite-migrations.ts:562, 588, 594, 600, 609; wizard copy:602). Neither fix this issue suggested — scanning dollar-quoted bodies for the index pass, or failing closed on tables touched inside one — was implemented.Today a column genuinely holding ciphertext, declared inside
DO $$ … END $$;, is still rewritten: the sweep adds an empty<column>_encryptedtwin, reports the file as successfully rewritten withskipped: [], and exits 0. #823's own test codifies exactly that outcome atpackages/cli/src/__tests__/rewrite-migrations.test.ts:977-998.Leaving this closed, since the reported harm is genuinely gone. The surviving blindness is now tracked in #836.
fix(cli,wizard): index dollar-quoted DDL so the already-encrypted gua…
fix(cli,wizard): follow rename chains across the dollar-quoted boundary