fix(migrations): reject temp schema objects

scripts/check-migrations.mjs

@@ -36,21 +36,33 @@ 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 <object> temp.<name>` 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|(?: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"],
   [/(?<=(?:^|;)\s*)pragma\b/gi, "PRAGMA is not supported on D1"],
   [/(?<=(?:^|;)\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 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; ) {
@@ -78,36 +90,29 @@ function cleanSql(sql) {
       }
       continue;
     }
-    if (c === "'" || c === '"' || c === "`") {
-      out += " ";
-      i += 1;
-      while (i < sql.length) {
-        if (sql[i] === c) {
-          if (sql[i + 1] === c) {
-            out += "  ";
-            i += 2;
+    if (c === "'" || c === '"' || c === "`" || c === "[") {
+      const close = c === "[" ? "]" : c;
+      let j = i + 1;
+      let content = "";
+      while (j < sql.length) {
+        if (sql[j] === close) {
+          if (close !== "]" && sql[j + 1] === close) {
+            content += close;
+            j += 2;
             continue;
           }
-          out += " ";
-          i += 1;
           break;
         }
-        out += sql[i] === "\n" ? "\n" : " ";
-        i += 1;
+        content += sql[j];
+        j += 1;
       }
-      continue;
-    }
-    if (c === "[") {
+      let k = j + 1;
+      while (k < sql.length && /\s/.test(sql[k])) k += 1;
+      const usedAsSchemaQualifier = k < sql.length && sql[k] === ".";
       out += " ";
-      i += 1;
-      while (i < sql.length && sql[i] !== "]") {
-        out += sql[i] === "\n" ? "\n" : " ";
-        i += 1;
-      }
-      if (i < sql.length) {
-        out += " ";
-        i += 1;
-      }
+      for (const ch of content) out += usedAsSchemaQualifier ? ch : ch === "\n" ? "\n" : " ";
+      if (j < sql.length) out += " ";
+      i = j < sql.length ? j + 1 : j;
       continue;
     }
     out += c;

test/unit/check-migrations-script.test.ts

@@ -34,8 +34,19 @@ 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);"],
+    ["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` });
 
     expect(r.status).toBe(1);
     expect(r.out).toContain("0001_temp.sql:1");
@@ -72,4 +83,48 @@ describe("check-migrations script", () => {
     expect(r.status).toBe(0);
     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");
+  });
+
+  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");
+    },
+  );
 });