From d47a97ec6bc758461326e6909276a74c8e4412a1 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:39:40 -0700 Subject: [PATCH 1/4] fix(migrations): reject temp schema objects --- scripts/check-migrations.mjs | 7 ++++--- test/unit/check-migrations-script.test.ts | 9 +++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/scripts/check-migrations.mjs b/scripts/check-migrations.mjs index 8d5ca2a30d..ff20caa934 100644 --- a/scripts/check-migrations.mjs +++ b/scripts/check-migrations.mjs @@ -36,12 +36,13 @@ const fail = (message) => { // migrations to, but the REMOTE D1 authorizer rejects them at `wrangler d1 migrations apply --remote` with // `not authorized: SQLITE_AUTH [code: 7500]` — which breaks the deploy AFTER merge, where pre-merge CI can't // see it (a `CREATE TEMP TABLE` in 0083 did exactly this). This scan is the only pre-merge gate for that class. -// `CREATE TEMP` matches anywhere (it always starts a statement); the rest anchor to a statement boundary -// (start-of-file or after a `;`) so a trigger body's own `BEGIN`/`END` and mid-statement words don't trip. +// `CREATE TEMP` and `CREATE temp.` match anywhere (they always start a statement); +// the rest anchor to a statement boundary (start-of-file or after a `;`) so a trigger body's own +// `BEGIN`/`END` and mid-statement words don't trip. // The anchored patterns use a variable-length lookbehind (`(?<=(?:^|;)\s*)`, supported by V8/Node) so the // match starts on the keyword itself — reported line numbers point at the statement, not the preceding `;`. const D1_FORBIDDEN = [ - [/create\s+temp(?:orary)?\b/gi, "temporary object (CREATE TEMP/TEMPORARY) — D1 rejects temp tables/triggers/views/indexes; rewrite without one (e.g. DELETE the losers, then UPDATE the survivors)"], + [/create\s+(?:temp(?:orary)?\b|(?:table|index|view|trigger)\s+(?:if\s+not\s+exists\s+)?temp\s*\.)/gi, "temporary object (CREATE TEMP/TEMPORARY or temp schema) — D1 rejects temp tables/triggers/views/indexes; rewrite without one (e.g. DELETE the losers, then UPDATE the survivors)"], [/(?<=(?:^|;)\s*)attach\b/gi, "ATTACH is not supported on D1"], [/(?<=(?:^|;)\s*)detach\b/gi, "DETACH is not supported on D1"], [/(?<=(?:^|;)\s*)vacuum\b/gi, "VACUUM is not supported on D1"], diff --git a/test/unit/check-migrations-script.test.ts b/test/unit/check-migrations-script.test.ts index 1320c32f88..3b376f7cab 100644 --- a/test/unit/check-migrations-script.test.ts +++ b/test/unit/check-migrations-script.test.ts @@ -34,8 +34,13 @@ describe("check-migrations script", () => { expect(output).toContain("(3 grandfathered duplicates: 0015, 0017, 0074)"); }); - it("rejects a migration that creates a temporary object (the D1 remote authorizer blocks it)", () => { - const r = runCheck({ "0001_temp.sql": "CREATE TEMP TABLE scratch AS SELECT 1;\n" }); + it.each([ + ["TEMP keyword", "CREATE TEMP TABLE scratch AS SELECT 1;"], + ["TEMPORARY keyword", "CREATE TEMPORARY VIEW scratch AS SELECT 1;"], + ["temp schema table", "CREATE TABLE temp.scratch AS SELECT 1;"], + ["temp schema index", "CREATE INDEX IF NOT EXISTS temp.scratch_idx ON scratch(id);"], + ])("rejects a migration that creates a temporary object via %s (the D1 remote authorizer blocks it)", (_name, sql) => { + const r = runCheck({ "0001_temp.sql": `${sql}\n` }); expect(r.status).toBe(1); expect(r.out).toContain("0001_temp.sql:1"); From 31f9495ea3dd8a4cfa08c8f3bcd3ee3527b0ef4a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:12:57 -0700 Subject: [PATCH 2/4] fix(migrations): match CREATE UNIQUE INDEX temp schema guard The temp-schema regex anchored table|index|view|trigger directly after `create`, so `CREATE UNIQUE INDEX temp.idx ...` slipped past the D1 remote-authorizer guard despite being rejected at deploy. --- scripts/check-migrations.mjs | 2 +- test/unit/check-migrations-script.test.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/scripts/check-migrations.mjs b/scripts/check-migrations.mjs index ff20caa934..85aa151fd5 100644 --- a/scripts/check-migrations.mjs +++ b/scripts/check-migrations.mjs @@ -42,7 +42,7 @@ const fail = (message) => { // The anchored patterns use a variable-length lookbehind (`(?<=(?:^|;)\s*)`, supported by V8/Node) so the // match starts on the keyword itself — reported line numbers point at the statement, not the preceding `;`. const D1_FORBIDDEN = [ - [/create\s+(?:temp(?:orary)?\b|(?:table|index|view|trigger)\s+(?:if\s+not\s+exists\s+)?temp\s*\.)/gi, "temporary object (CREATE TEMP/TEMPORARY or temp schema) — D1 rejects temp tables/triggers/views/indexes; rewrite without one (e.g. DELETE the losers, then UPDATE the survivors)"], + [/create\s+(?:temp(?:orary)?\b|(?:unique\s+)?(?:table|index|view|trigger)\s+(?:if\s+not\s+exists\s+)?temp\s*\.)/gi, "temporary object (CREATE TEMP/TEMPORARY or temp schema) — D1 rejects temp tables/triggers/views/indexes; rewrite without one (e.g. DELETE the losers, then UPDATE the survivors)"], [/(?<=(?:^|;)\s*)attach\b/gi, "ATTACH is not supported on D1"], [/(?<=(?:^|;)\s*)detach\b/gi, "DETACH is not supported on D1"], [/(?<=(?:^|;)\s*)vacuum\b/gi, "VACUUM is not supported on D1"], diff --git a/test/unit/check-migrations-script.test.ts b/test/unit/check-migrations-script.test.ts index 3b376f7cab..b1694e0e07 100644 --- a/test/unit/check-migrations-script.test.ts +++ b/test/unit/check-migrations-script.test.ts @@ -39,6 +39,7 @@ describe("check-migrations script", () => { ["TEMPORARY keyword", "CREATE TEMPORARY VIEW scratch AS SELECT 1;"], ["temp schema table", "CREATE TABLE temp.scratch AS SELECT 1;"], ["temp schema index", "CREATE INDEX IF NOT EXISTS temp.scratch_idx ON scratch(id);"], + ["temp schema unique index", "CREATE UNIQUE INDEX temp.scratch_idx ON scratch(id);"], ])("rejects a migration that creates a temporary object via %s (the D1 remote authorizer blocks it)", (_name, sql) => { const r = runCheck({ "0001_temp.sql": `${sql}\n` }); @@ -77,4 +78,11 @@ describe("check-migrations script", () => { expect(r.status).toBe(0); expect(r.out).toContain("1 migrations OK"); }); + + it("does not flag a CREATE UNIQUE INDEX that is not in the temp schema", () => { + const r = runCheck({ "0001_ok.sql": "CREATE UNIQUE INDEX idx_t_id ON t(id);\n" }); + + expect(r.status).toBe(0); + expect(r.out).toContain("1 migrations OK"); + }); }); From 12bd55fba5afb26d4b0ac1167036caaeb830bb84 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:28:10 -0700 Subject: [PATCH 3/4] fix(migrations): preserve quoted identifiers when scanning for temp schema cleanSql blanked the contents of double-quoted, backtick-quoted, and bracket-quoted identifiers the same way it blanks string literals, hiding a legitimate "temp".scratch (or `temp`.scratch, [temp].scratch) reference from the D1_FORBIDDEN temp-schema scan. Also close the same gap for SQLite's single-quote-as-identifier fallback ('temp'.scratch), distinguishing it from an ordinary string value by checking whether the closing quote is immediately followed by a schema-qualifying dot. --- scripts/check-migrations.mjs | 63 ++++++++++++++++------- test/unit/check-migrations-script.test.ts | 28 ++++++++++ 2 files changed, 71 insertions(+), 20 deletions(-) diff --git a/scripts/check-migrations.mjs b/scripts/check-migrations.mjs index 85aa151fd5..07b360c260 100644 --- a/scripts/check-migrations.mjs +++ b/scripts/check-migrations.mjs @@ -50,8 +50,12 @@ const D1_FORBIDDEN = [ [/(?<=(?:^|;)\s*)(?:begin|commit|rollback|savepoint|release)\b/gi, "explicit transaction control — wrangler wraps each migration in its own transaction"], ]; -// Blank out comments and string/identifier literals (preserving newlines for accurate line numbers) so a -// forbidden keyword inside a comment or a quoted value can never trip a false positive. +// Blank out comments and string literals (preserving newlines for accurate line numbers) so a forbidden +// keyword inside a comment or a quoted VALUE can never trip a false positive. Quoted IDENTIFIERS (`"..."`, +// `` `...` ``, `[...]`) are different: that text names a real schema/table/column, so blanking it would hide +// a genuine forbidden reference — `"temp".scratch` is exactly as much a temp-schema object as unquoted +// `temp.scratch` — from D1_FORBIDDEN below. Those three delimiter forms strip only the quote characters +// themselves and keep the identifier text intact for the scan. function cleanSql(sql) { let out = ""; for (let i = 0; i < sql.length; ) { @@ -79,13 +83,45 @@ function cleanSql(sql) { } continue; } - if (c === "'" || c === '"' || c === "`") { + if (c === "'") { + // SQLite's documented "single-quote misfeature": a single-quoted token used where an identifier is + // expected (i.e. immediately schema-qualifying a `.`) is treated as an IDENTIFIER, not a string + // value — `'temp'.scratch` genuinely creates a temp-schema object exactly like `"temp".scratch` + // does. An ordinary string VALUE is never immediately followed (past optional whitespace) by a bare + // `.` in valid SQL, so peeking past the closing quote for one distinguishes the two without a real + // SQL parser, while still blanking the overwhelmingly common case (a value) so forbidden-looking + // text inside it can't trip a false positive on the unanchored temp-schema pattern below. + let j = i + 1; + let content = ""; + while (j < sql.length) { + if (sql[j] === "'") { + if (sql[j + 1] === "'") { + content += "'"; + j += 2; + continue; + } + break; + } + content += sql[j]; + j += 1; + } + let k = j + 1; + while (k < sql.length && /\s/.test(sql[k])) k += 1; + const usedAsIdentifier = k < sql.length && sql[k] === "."; + out += " "; + for (const ch of content) out += usedAsIdentifier ? ch : ch === "\n" ? "\n" : " "; + if (j < sql.length) out += " "; + i = j < sql.length ? j + 1 : j; + continue; + } + if (c === '"' || c === "`" || c === "[") { + const close = c === "[" ? "]" : c; out += " "; i += 1; while (i < sql.length) { - if (sql[i] === c) { - if (sql[i + 1] === c) { - out += " "; + if (sql[i] === close) { + if (close !== "]" && sql[i + 1] === close) { + out += close + close; i += 2; continue; } @@ -93,20 +129,7 @@ function cleanSql(sql) { i += 1; break; } - out += sql[i] === "\n" ? "\n" : " "; - i += 1; - } - continue; - } - if (c === "[") { - out += " "; - i += 1; - while (i < sql.length && sql[i] !== "]") { - out += sql[i] === "\n" ? "\n" : " "; - i += 1; - } - if (i < sql.length) { - out += " "; + out += sql[i]; i += 1; } continue; diff --git a/test/unit/check-migrations-script.test.ts b/test/unit/check-migrations-script.test.ts index b1694e0e07..f104bf6e94 100644 --- a/test/unit/check-migrations-script.test.ts +++ b/test/unit/check-migrations-script.test.ts @@ -40,6 +40,11 @@ describe("check-migrations script", () => { ["temp schema table", "CREATE TABLE temp.scratch AS SELECT 1;"], ["temp schema index", "CREATE INDEX IF NOT EXISTS temp.scratch_idx ON scratch(id);"], ["temp schema unique index", "CREATE UNIQUE INDEX temp.scratch_idx ON scratch(id);"], + ["double-quoted temp schema", 'CREATE TABLE "temp".scratch AS SELECT 1;'], + ["double-quoted temp schema, both sides quoted", 'CREATE TABLE "temp"."scratch" AS SELECT 1;'], + ["backtick-quoted temp schema", "CREATE TABLE `temp`.scratch AS SELECT 1;"], + ["bracket-quoted temp schema", "CREATE TABLE [temp].scratch AS SELECT 1;"], + ["single-quoted temp schema (SQLite's single-quote-as-identifier misfeature)", "CREATE TABLE 'temp'.scratch AS SELECT 1;"], ])("rejects a migration that creates a temporary object via %s (the D1 remote authorizer blocks it)", (_name, sql) => { const r = runCheck({ "0001_temp.sql": `${sql}\n` }); @@ -79,10 +84,33 @@ describe("check-migrations script", () => { expect(r.out).toContain("1 migrations OK"); }); + it("does not flag a single-quoted VALUE that literally contains the temp-schema pattern's text, since it is not schema-qualifying a dot", () => { + // The temp-schema alternative in D1_FORBIDDEN has no start-of-statement anchor (unlike attach/vacuum/ + // pragma/etc.), so a single-quoted value's content can't be blanket-preserved just because SOME + // single-quoted tokens are legitimately identifiers (see the SQLite single-quote-misfeature test + // above) -- only a value immediately followed by a `.` is treated as an identifier. + const r = runCheck({ + "0001_ok.sql": "INSERT INTO logs (msg) VALUES ('create temporary object warning');\n", + }); + + expect(r.status).toBe(0); + expect(r.out).toContain("1 migrations OK"); + }); + it("does not flag a CREATE UNIQUE INDEX that is not in the temp schema", () => { const r = runCheck({ "0001_ok.sql": "CREATE UNIQUE INDEX idx_t_id ON t(id);\n" }); expect(r.status).toBe(0); expect(r.out).toContain("1 migrations OK"); }); + + it("does not flag a quoted identifier that merely contains \"temp\" without a schema-qualifying dot", () => { + const r = runCheck({ + "0001_ok.sql": + 'CREATE TABLE "temp_settings" (id INTEGER PRIMARY KEY);\n' + "CREATE TABLE `temp_cache` (id INTEGER PRIMARY KEY);\n", + }); + + expect(r.status).toBe(0); + expect(r.out).toContain("1 migrations OK"); + }); }); From 36156b6317834345a4b9af732f6301a52b717362 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 01:32:21 -0700 Subject: [PATCH 4/4] fix(migrations): only expose quoted identifiers to the scan when dot-qualified The temp-schema pattern in D1_FORBIDDEN is deliberately unanchored, so unconditionally preserving every quoted identifier's text let an ordinary column/table name that merely spells out forbidden-looking text (e.g. a column named "create temp note") leak into the scan and false-positive. Unify all four quoting styles (', ", `, []) under one rule: only preserve content when the closing quote is immediately followed by a schema-qualifying dot; otherwise blank it like a value. --- scripts/check-migrations.mjs | 59 ++++++++--------------- test/unit/check-migrations-script.test.ts | 14 ++++++ 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/scripts/check-migrations.mjs b/scripts/check-migrations.mjs index 07b360c260..05fd9f82ee 100644 --- a/scripts/check-migrations.mjs +++ b/scripts/check-migrations.mjs @@ -50,12 +50,19 @@ const D1_FORBIDDEN = [ [/(?<=(?:^|;)\s*)(?:begin|commit|rollback|savepoint|release)\b/gi, "explicit transaction control — wrangler wraps each migration in its own transaction"], ]; -// Blank out comments and string literals (preserving newlines for accurate line numbers) so a forbidden -// keyword inside a comment or a quoted VALUE can never trip a false positive. Quoted IDENTIFIERS (`"..."`, -// `` `...` ``, `[...]`) are different: that text names a real schema/table/column, so blanking it would hide -// a genuine forbidden reference — `"temp".scratch` is exactly as much a temp-schema object as unquoted -// `temp.scratch` — from D1_FORBIDDEN below. Those three delimiter forms strip only the quote characters -// themselves and keep the identifier text intact for the scan. +// Blank out comments and quoted VALUES (preserving newlines for accurate line numbers) so a forbidden +// keyword inside a comment or a quoted value can never trip a false positive. A quoted token used as a +// schema-qualifying IDENTIFIER is different: `"temp".scratch`, `` `temp`.scratch ``, `[temp].scratch`, and +// even `'temp'.scratch` (SQLite's documented single-quote-as-identifier fallback) are exactly as much a +// temp-schema object as unquoted `temp.scratch` and must not be hidden from D1_FORBIDDEN below. But the +// temp-schema pattern is deliberately UNANCHORED (it must match anywhere a statement can start), so +// preserving a quoted token's content unconditionally — merely because ITS quote style is capable of +// being an identifier — would leak an ordinary column/table NAME's text into that scan too, e.g. a column +// literally named "create temp note" is not a temp-schema reference. An identifier is only ever +// schema-qualifying something when its closing quote is immediately followed (past optional whitespace) +// by a `.`; an ordinary name or value never is. Peeking past the closing quote for one, uniformly across +// all four quoting styles, distinguishes "used as a schema qualifier" from "used as a name or value" +// without a real SQL parser. function cleanSql(sql) { let out = ""; for (let i = 0; i < sql.length; ) { @@ -83,20 +90,14 @@ function cleanSql(sql) { } continue; } - if (c === "'") { - // SQLite's documented "single-quote misfeature": a single-quoted token used where an identifier is - // expected (i.e. immediately schema-qualifying a `.`) is treated as an IDENTIFIER, not a string - // value — `'temp'.scratch` genuinely creates a temp-schema object exactly like `"temp".scratch` - // does. An ordinary string VALUE is never immediately followed (past optional whitespace) by a bare - // `.` in valid SQL, so peeking past the closing quote for one distinguishes the two without a real - // SQL parser, while still blanking the overwhelmingly common case (a value) so forbidden-looking - // text inside it can't trip a false positive on the unanchored temp-schema pattern below. + if (c === "'" || c === '"' || c === "`" || c === "[") { + const close = c === "[" ? "]" : c; let j = i + 1; let content = ""; while (j < sql.length) { - if (sql[j] === "'") { - if (sql[j + 1] === "'") { - content += "'"; + if (sql[j] === close) { + if (close !== "]" && sql[j + 1] === close) { + content += close; j += 2; continue; } @@ -107,33 +108,13 @@ function cleanSql(sql) { } let k = j + 1; while (k < sql.length && /\s/.test(sql[k])) k += 1; - const usedAsIdentifier = k < sql.length && sql[k] === "."; + const usedAsSchemaQualifier = k < sql.length && sql[k] === "."; out += " "; - for (const ch of content) out += usedAsIdentifier ? ch : ch === "\n" ? "\n" : " "; + for (const ch of content) out += usedAsSchemaQualifier ? ch : ch === "\n" ? "\n" : " "; if (j < sql.length) out += " "; i = j < sql.length ? j + 1 : j; continue; } - if (c === '"' || c === "`" || c === "[") { - const close = c === "[" ? "]" : c; - out += " "; - i += 1; - while (i < sql.length) { - if (sql[i] === close) { - if (close !== "]" && sql[i + 1] === close) { - out += close + close; - i += 2; - continue; - } - out += " "; - i += 1; - break; - } - out += sql[i]; - i += 1; - } - continue; - } out += c; i += 1; } diff --git a/test/unit/check-migrations-script.test.ts b/test/unit/check-migrations-script.test.ts index f104bf6e94..2d40576eea 100644 --- a/test/unit/check-migrations-script.test.ts +++ b/test/unit/check-migrations-script.test.ts @@ -113,4 +113,18 @@ describe("check-migrations script", () => { expect(r.status).toBe(0); expect(r.out).toContain("1 migrations OK"); }); + + it.each([ + ["double-quoted column name", 'CREATE TABLE t ("create temp note" TEXT);'], + ["backtick-quoted column name", "CREATE TABLE t (`create temp note` TEXT);"], + ["bracket-quoted column name", "CREATE TABLE t ([create temp note] TEXT);"], + ])( + "does not flag a %s that merely spells out the forbidden phrase, since the temp-schema pattern is unanchored and only a schema-qualifying dot should expose quoted identifier text to it", + (_name, sql) => { + const r = runCheck({ "0001_ok.sql": `${sql}\n` }); + + expect(r.status).toBe(0); + expect(r.out).toContain("1 migrations OK"); + }, + ); });