From f75ea9628ca42dffeeb2e2b71948e8e3c4976269 Mon Sep 17 00:00:00 2001 From: msogin Date: Mon, 17 Aug 2026 21:45:40 -0400 Subject: [PATCH 1/2] =?UTF-8?q?fix(db):=20translate=20DATE=5FADD=20for=20S?= =?UTF-8?q?QLite=20=E2=80=94=20org=20report=20500s=20with=20spend=20data?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GLOOK-41. src/lib/report/org.ts builds `DATE_ADD(?, INTERVAL 1 DAY)` for the Claude Code spend window. translateSQL had rules for DATE_SUB but none for DATE_ADD, so the statement reached better-sqlite3 verbatim and failed with `near "1": syntax error` — a 500 on GET /api/report//org for any SQLite deployment whose report carries spend data. The query sits behind `if (ccStart && ccEnd && ...)`, so SQLite installs with no spend data skipped it entirely and MySQL was never affected. `npm run seed` populates a spend window, so the documented seed + dev:mock workflow hit it every time. Adds three rules mirroring the existing DATE_SUB pair: the two NOW() forms first (NOW() is rewritten earlier into datetime('now','localtime'), whose comma would split a general pattern), then the general expression form. The captured expression is spliced verbatim so a bound `?` stays a bound parameter. The regression test drives the real SQLite driver rather than a mock — the existing suites around getOrgReport stayed green through this bug precisely because they never exercised translateSQL. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/unit/sqlite-date-add.test.ts | 88 +++++++++++++++++++ src/lib/db/sqlite.ts | 14 +++ 2 files changed, 102 insertions(+) create mode 100644 src/lib/__tests__/unit/sqlite-date-add.test.ts diff --git a/src/lib/__tests__/unit/sqlite-date-add.test.ts b/src/lib/__tests__/unit/sqlite-date-add.test.ts new file mode 100644 index 00000000..5d4169e0 --- /dev/null +++ b/src/lib/__tests__/unit/sqlite-date-add.test.ts @@ -0,0 +1,88 @@ +/** + * GLOOK-41 regression: the org report's spend-window query uses + * `DATE_ADD(?, INTERVAL 1 DAY)` (src/lib/report/org.ts). translateSQL had + * rules for DATE_SUB but none for DATE_ADD, so the statement reached + * better-sqlite3 verbatim and failed with `near "1": syntax error`. + * + * These tests deliberately drive the REAL SQLite driver rather than a mock. + * The mocked suites around getOrgReport stayed green through this bug + * precisely because they never exercised translateSQL. + */ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +describe('SQLite dialect translation — DATE_ADD', () => { + const prevSqlitePath = process.env.SQLITE_PATH; + const prevDbType = process.env.DB_TYPE; + let dbPath: string; + + beforeAll(() => { + dbPath = path.join(os.tmpdir(), `glooker-glook41-${process.pid}-${Date.now()}.db`); + process.env.SQLITE_PATH = dbPath; + process.env.DB_TYPE = 'sqlite'; + }); + + afterAll(() => { + // Restore, or later files in this worker inherit a deleted DB path. + if (prevSqlitePath === undefined) delete process.env.SQLITE_PATH; + else process.env.SQLITE_PATH = prevSqlitePath; + if (prevDbType === undefined) delete process.env.DB_TYPE; + else process.env.DB_TYPE = prevDbType; + for (const suffix of ['', '-wal', '-shm']) { + try { fs.unlinkSync(`${dbPath}${suffix}`); } catch { /* already gone */ } + } + }); + + async function makeDb() { + const { createSQLiteDB } = await import('@/lib/db/sqlite'); + return createSQLiteDB(); + } + + it('translates DATE_ADD(?, INTERVAL 1 DAY) instead of failing to parse', async () => { + const db = await makeDb(); + const [rows] = await db.execute( + `SELECT DATE_ADD(?, INTERVAL 1 DAY) AS boundary`, + ['2026-03-18'], + ) as [any[], any]; + expect(String(rows[0].boundary)).toMatch(/^2026-03-19/); + }); + + it('keeps the bound parameter a parameter rather than interpolating it', async () => { + const db = await makeDb(); + const [rows] = await db.execute( + `SELECT DATE_ADD(?, INTERVAL 1 DAY) AS boundary`, + ["2026-03-18' OR '1'='1"], + ) as [any[], any]; + // A hostile string must simply fail to parse as a date, not alter the query. + expect(rows).toHaveLength(1); + expect(rows[0].boundary).toBeNull(); + }); + + it('gives the org spend-window clause exclusive-end-of-day semantics', async () => { + const db = await makeDb(); + // Mirrors src/lib/report/org.ts: committed_at < DATE_ADD(?, INTERVAL 1 DAY) + const inWindow = async (committedAt: string) => { + const [rows] = await db.execute( + `SELECT 1 AS hit WHERE ? < DATE_ADD(?, INTERVAL 1 DAY)`, + [committedAt, '2026-03-31'], + ) as [any[], any]; + return rows.length === 1; + }; + expect(await inWindow('2026-03-31 23:59:59')).toBe(true); + expect(await inWindow('2026-04-01 00:00:01')).toBe(false); + }); + + it('handles DATE_ADD(NOW(), ...) even though NOW() is rewritten first', async () => { + // translateSQL turns NOW() into datetime('now','localtime') before any + // DATE_ADD rule runs, and that replacement contains a comma — a rule that + // naively matched "everything up to the first comma" would break here. + // DATE_SUB already carries a dedicated variant for this reason. + const db = await makeDb(); + const [rows] = await db.execute( + `SELECT DATE_ADD(NOW(), INTERVAL 7 DAY) AS boundary`, + ) as [any[], any]; + expect(rows).toHaveLength(1); + expect(String(rows[0].boundary)).toMatch(/^\d{4}-\d{2}-\d{2}/); + }); +}); diff --git a/src/lib/db/sqlite.ts b/src/lib/db/sqlite.ts index 2685ff08..68621f6b 100644 --- a/src/lib/db/sqlite.ts +++ b/src/lib/db/sqlite.ts @@ -405,6 +405,20 @@ function translateSQL(sql: string): string { s = s.replace(/DATE_SUB\s*\(\s*NOW\(\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, (_match, days) => `datetime('now', '-${days} days')`); + // DATE_ADD(expr, INTERVAL N DAY) → datetime(expr, '+N days') + // Order matters: NOW() is rewritten above into datetime('now','localtime'), + // which itself contains a comma — so the NOW() forms get dedicated rules + // before the general one, exactly as DATE_SUB does. + s = s.replace(/DATE_ADD\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_match, days) => `datetime('now', '+${days} days')`); + s = s.replace(/DATE_ADD\s*\(\s*NOW\(\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_match, days) => `datetime('now', '+${days} days')`); + // General form, e.g. the org spend-window query's DATE_ADD(?, INTERVAL 1 DAY). + // The captured expression is spliced through verbatim so a bound `?` stays a + // bound parameter — never interpolate the caller's value into the SQL. + s = s.replace(/DATE_ADD\s*\(\s*([^,()]+?)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_match, expr, days) => `datetime(${expr}, '+${days} days')`); + // ON DUPLICATE KEY UPDATE ... VALUES(col) → ON CONFLICT(...) DO UPDATE SET col = excluded.col const odkuMatch = s.match(/ON\s+DUPLICATE\s+KEY\s+UPDATE\s+([\s\S]+)$/i); if (odkuMatch) { From 1b1cd84e7a790a5ef03902b770d64c480157e928 Mon Sep 17 00:00:00 2001 From: msogin Date: Tue, 18 Aug 2026 16:42:49 -0400 Subject: [PATCH 2/2] =?UTF-8?q?fix(db):=20address=20review=20=E2=80=94=20l?= =?UTF-8?q?iteral-aware=20translateSQL,=20DATE=5FSUB=20localtime,=20loud?= =?UTF-8?q?=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on PR #67, in order of severity. localtime (live callers): DATE_SUB(NOW(), ...) emitted datetime('now', '-N days'), dropping the 'localtime' that NOW() itself is rewritten with, so NOW() and DATE_SUB(NOW(), ...) disagreed by the host's UTC offset. Six callers rely on this (projects/untracked.ts, epic-stats.ts, epic-summary.ts). Now emits datetime('now', 'localtime', '-N days'). Dead rule: the DATE_ADD(NOW(), ...) rules are deleted rather than fixed. NOW() is rewritten before any DATE_* rule, so the raw-NOW form was unreachable, and DATE_ADD(NOW( appears nowhere in the codebase. Deleting removes the localtime skew and an untested path in one move; the guard below makes the omission loud. String literals: anchoring the capture to ?/identifier is NOT sufficient. The text DATE_ADD(a , INTERVAL 9 DAY) inside a literal still matches an identifier capture, and the replacement injects quotes, terminating the literal and restructuring the statement. All token rewrites now run per non-literal chunk via outsideStringLiterals(), which handles doubled-quote escapes. DATE_SUB gains the general form DATE_ADD had, so the asymmetry introduced by the previous commit is gone. Passthrough default (root cause): translateSQL is a whitelist rewriter, and anything unmatched previously reached the driver verbatim and 500d at request time. It now throws, naming the statement. Only non-literal text is inspected so SQL-looking data (commit messages, Jira summaries) cannot trip it. New mysql-date-expressions.test.ts scans src/lib SQL and fails CI for any date expression translateSQL cannot handle, closing the loop that was missing when GLOOK-41 shipped. Tests: translateSQL is exported, so the rewrite is asserted directly rather than inferred through a driver. Dropped the hostile-string test, since translateSQL never receives params and so could not fail for the stated reason. Boundary assertion tightened from a prefix match to exactly 2026-03-19 00:00:00. Window test now uses the stored ISO-8601 T...Z format rather than space-separated. Temp DB via mkdtempSync with one shared handle instead of a guessable path and four unclosed handles. 119 suites / 1141 tests pass; tsc clean. (dev-usage-card.test.tsx and two tsc errors come from an uncommitted local deletion of usage-card.tsx, unrelated.) Co-Authored-By: Claude Opus 5 (1M context) --- .../unit/mysql-date-expressions.test.ts | 43 +++++ .../__tests__/unit/sqlite-date-add.test.ts | 151 ++++++++++++------ src/lib/db/sqlite.ts | 133 +++++++++++---- 3 files changed, 243 insertions(+), 84 deletions(-) create mode 100644 src/lib/__tests__/unit/mysql-date-expressions.test.ts diff --git a/src/lib/__tests__/unit/mysql-date-expressions.test.ts b/src/lib/__tests__/unit/mysql-date-expressions.test.ts new file mode 100644 index 00000000..88308667 --- /dev/null +++ b/src/lib/__tests__/unit/mysql-date-expressions.test.ts @@ -0,0 +1,43 @@ +/** + * Enforcement: every MySQL date expression written in src/lib SQL must have a + * translateSQL rule. + * + * GLOOK-41 shipped because nothing connected "someone wrote DATE_ADD" to "the + * SQLite translator knows DATE_ADD". The mocked suites around the caller never + * reach translateSQL, so CI stayed green and the gap surfaced as a 500. This + * test closes that loop: a new unhandled form fails here instead. + */ +import fs from 'fs'; +import path from 'path'; +import { translateSQL } from '@/lib/db/sqlite'; + +const LIB = path.join(process.cwd(), 'src', 'lib'); + +function walk(dir: string): string[] { + return fs.readdirSync(dir, { withFileTypes: true }).flatMap((e) => { + const full = path.join(dir, e.name); + if (e.isDirectory()) return e.name === '__tests__' ? [] : walk(full); + return e.isFile() && full.endsWith('.ts') && !full.endsWith('db/sqlite.ts') ? [full] : []; + }); +} + +describe('MySQL date expressions in src/lib SQL', () => { + it('are all handled by translateSQL', () => { + const pattern = /DATE_ADD\s*\([^)]*INTERVAL[^)]*\)|DATE_SUB\s*\([^)]*INTERVAL[^)]*\)/gi; + const offenders: string[] = []; + + for (const file of walk(LIB)) { + const src = fs.readFileSync(file, 'utf8'); + for (const expr of src.match(pattern) ?? []) { + // Wrap in a minimal statement so translateSQL sees realistic input. + try { + translateSQL(`SELECT 1 WHERE x < ${expr}`); + } catch { + offenders.push(`${path.relative(process.cwd(), file)}: ${expr}`); + } + } + } + + expect(offenders).toEqual([]); + }); +}); diff --git a/src/lib/__tests__/unit/sqlite-date-add.test.ts b/src/lib/__tests__/unit/sqlite-date-add.test.ts index 5d4169e0..5489794a 100644 --- a/src/lib/__tests__/unit/sqlite-date-add.test.ts +++ b/src/lib/__tests__/unit/sqlite-date-add.test.ts @@ -1,67 +1,127 @@ /** * GLOOK-41 regression: the org report's spend-window query uses - * `DATE_ADD(?, INTERVAL 1 DAY)` (src/lib/report/org.ts). translateSQL had - * rules for DATE_SUB but none for DATE_ADD, so the statement reached - * better-sqlite3 verbatim and failed with `near "1": syntax error`. + * `DATE_ADD(?, INTERVAL 1 DAY)` (src/lib/report/org.ts). translateSQL had rules + * for DATE_SUB but none for DATE_ADD, so the statement reached better-sqlite3 + * verbatim and failed with `near "1": syntax error`. * - * These tests deliberately drive the REAL SQLite driver rather than a mock. - * The mocked suites around getOrgReport stayed green through this bug - * precisely because they never exercised translateSQL. + * Two layers on purpose: + * - translateSQL is pure, so the rewrite itself is asserted directly. That is + * where sign, magnitude and quoting live, and it needs no driver. + * - one case drives the REAL SQLite driver, because the value of this fix is + * that better-sqlite3 accepts the output. The mocked suites around + * getOrgReport stayed green through this bug precisely because none of them + * reach translateSQL. */ import fs from 'fs'; import os from 'os'; import path from 'path'; +import { translateSQL } from '@/lib/db/sqlite'; -describe('SQLite dialect translation — DATE_ADD', () => { +describe('translateSQL — MySQL date expressions', () => { + it('rewrites the org spend-window DATE_ADD, keeping the bound parameter', () => { + expect( + translateSQL('SELECT 1 WHERE committed_at < DATE_ADD(?, INTERVAL 1 DAY)'), + ).toBe("SELECT 1 WHERE committed_at < datetime(?, '+1 days')"); + }); + + it('adds days rather than subtracting them', () => { + // The rule was written by copying the DATE_SUB block; flipping '+' to '-' + // is the slip that copying invites, and a shape-only assertion would miss it. + expect(translateSQL('SELECT DATE_ADD(?, INTERVAL 7 DAY)')).toContain("'+7 days'"); + expect(translateSQL('SELECT DATE_SUB(?, INTERVAL 7 DAY)')).toContain("'-7 days'"); + }); + + it('keeps DATE_SUB(NOW(), …) on localtime so it agrees with NOW()', () => { + // Without 'localtime' the two disagree by the host's UTC offset, silently + // shifting every 90-day window in projects/untracked.ts, epic-stats.ts and + // epic-summary.ts on any non-UTC host. + const out = translateSQL('SELECT NOW() AS a, DATE_SUB(NOW(), INTERVAL 90 DAY) AS b'); + expect(out).toContain("datetime('now','localtime') AS a"); + expect(out).toContain("datetime('now', 'localtime', '-90 days') AS b"); + }); + + it('leaves a MySQL date expression inside a string literal byte-identical', () => { + // Anchoring the pattern to `?`/identifier is NOT sufficient on its own: + // 'DATE_ADD(a , INTERVAL 9 DAY)' still matches an identifier capture, and the + // replacement injects quotes, which would terminate the literal and + // restructure the statement. The rewrites run per non-literal chunk instead. + const sql = "SELECT 1 WHERE msg = 'DATE_ADD(a , INTERVAL 9 DAY)'"; + expect(translateSQL(sql)).toBe(sql); + }); + + it('handles a doubled quote inside a literal', () => { + const sql = "SELECT 1 WHERE note = 'it''s a DATE_SUB(x , INTERVAL 3 DAY) note'"; + expect(translateSQL(sql)).toBe(sql); + }); + + it('rewrites a real expression while leaving a literal beside it untouched', () => { + expect( + translateSQL("SELECT 1 WHERE msg='DATE_ADD(q , INTERVAL 5 DAY)' AND t < DATE_ADD(?, INTERVAL 1 DAY)"), + ).toBe("SELECT 1 WHERE msg='DATE_ADD(q , INTERVAL 5 DAY)' AND t < datetime(?, '+1 days')"); + }); + + it('does not let SQL-looking text stored as data trip the guard', () => { + // commit_analyses stores commit messages; a message mentioning INTERVAL must + // not turn a working query into a thrown error. + const sql = "INSERT INTO commit_analyses (message) VALUES ('perf: use INTERVAL 7 DAY window')"; + expect(() => translateSQL(sql)).not.toThrow(); + }); + + it('throws instead of passing an untranslated form to the driver', () => { + // Passthrough was the root cause of GLOOK-41: unmatched SQL reached the + // driver and 500d at request time. These shapes have no rule; each must fail + // loudly and greppably rather than at a user's request. + for (const sql of [ + 'SELECT DATE_ADD(?, INTERVAL 1 MONTH)', + 'SELECT DATE_ADD(?, INTERVAL ? DAY)', + 'SELECT DATE_ADD(DATE(x), INTERVAL 1 DAY)', + 'SELECT DATE_SUB(?, INTERVAL 2 HOUR)', + ]) { + expect(() => translateSQL(sql)).toThrow(/untranslated MySQL date expression/); + } + }); +}); + +describe('SQLite driver accepts the translated spend-window clause', () => { const prevSqlitePath = process.env.SQLITE_PATH; const prevDbType = process.env.DB_TYPE; - let dbPath: string; + let tmpDir: string; + let db: { execute: (sql: string, params?: any[]) => Promise<[T[], any]> }; - beforeAll(() => { - dbPath = path.join(os.tmpdir(), `glooker-glook41-${process.pid}-${Date.now()}.db`); - process.env.SQLITE_PATH = dbPath; + beforeAll(async () => { + // mkdtempSync, not a guessable path in the shared temp dir — matches + // cc-apply-breakdowns.test.ts, prompt-loader.test.ts and logger.test.ts. + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'glooker-glook41-')); + process.env.SQLITE_PATH = path.join(tmpDir, 'test.db'); process.env.DB_TYPE = 'sqlite'; + const { createSQLiteDB } = await import('@/lib/db/sqlite'); + // One handle for the whole suite: each createSQLiteDB() re-runs the schema + // and every ALTER migration. + db = createSQLiteDB(); }); afterAll(() => { - // Restore, or later files in this worker inherit a deleted DB path. + // Restore, or later files in this Jest worker inherit a deleted DB path. if (prevSqlitePath === undefined) delete process.env.SQLITE_PATH; else process.env.SQLITE_PATH = prevSqlitePath; if (prevDbType === undefined) delete process.env.DB_TYPE; else process.env.DB_TYPE = prevDbType; - for (const suffix of ['', '-wal', '-shm']) { - try { fs.unlinkSync(`${dbPath}${suffix}`); } catch { /* already gone */ } - } + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* already gone */ } }); - async function makeDb() { - const { createSQLiteDB } = await import('@/lib/db/sqlite'); - return createSQLiteDB(); - } - - it('translates DATE_ADD(?, INTERVAL 1 DAY) instead of failing to parse', async () => { - const db = await makeDb(); + it('lands the boundary on exact midnight of the following day', async () => { const [rows] = await db.execute( `SELECT DATE_ADD(?, INTERVAL 1 DAY) AS boundary`, ['2026-03-18'], ) as [any[], any]; - expect(String(rows[0].boundary)).toMatch(/^2026-03-19/); + // Pinned exactly: a time component here would be a real MySQL/SQLite difference. + expect(String(rows[0].boundary)).toBe('2026-03-19 00:00:00'); }); - it('keeps the bound parameter a parameter rather than interpolating it', async () => { - const db = await makeDb(); - const [rows] = await db.execute( - `SELECT DATE_ADD(?, INTERVAL 1 DAY) AS boundary`, - ["2026-03-18' OR '1'='1"], - ) as [any[], any]; - // A hostile string must simply fail to parse as a date, not alter the query. - expect(rows).toHaveLength(1); - expect(rows[0].boundary).toBeNull(); - }); - - it('gives the org spend-window clause exclusive-end-of-day semantics', async () => { - const db = await makeDb(); - // Mirrors src/lib/report/org.ts: committed_at < DATE_ADD(?, INTERVAL 1 DAY) + it('keeps the spend window exclusive at the end date', async () => { + // Inputs use the format actually stored in commit_analyses — GitHub's + // ISO-8601 `…T…Z` (src/lib/github.ts → src/lib/report-runner.ts), compared + // as TEXT against datetime()'s space-separated output. const inWindow = async (committedAt: string) => { const [rows] = await db.execute( `SELECT 1 AS hit WHERE ? < DATE_ADD(?, INTERVAL 1 DAY)`, @@ -69,20 +129,7 @@ describe('SQLite dialect translation — DATE_ADD', () => { ) as [any[], any]; return rows.length === 1; }; - expect(await inWindow('2026-03-31 23:59:59')).toBe(true); - expect(await inWindow('2026-04-01 00:00:01')).toBe(false); - }); - - it('handles DATE_ADD(NOW(), ...) even though NOW() is rewritten first', async () => { - // translateSQL turns NOW() into datetime('now','localtime') before any - // DATE_ADD rule runs, and that replacement contains a comma — a rule that - // naively matched "everything up to the first comma" would break here. - // DATE_SUB already carries a dedicated variant for this reason. - const db = await makeDb(); - const [rows] = await db.execute( - `SELECT DATE_ADD(NOW(), INTERVAL 7 DAY) AS boundary`, - ) as [any[], any]; - expect(rows).toHaveLength(1); - expect(String(rows[0].boundary)).toMatch(/^\d{4}-\d{2}-\d{2}/); + expect(await inWindow('2026-03-31T23:59:59Z')).toBe(true); + expect(await inWindow('2026-04-01T00:00:00Z')).toBe(false); }); }); diff --git a/src/lib/db/sqlite.ts b/src/lib/db/sqlite.ts index 68621f6b..cc18bbba 100644 --- a/src/lib/db/sqlite.ts +++ b/src/lib/db/sqlite.ts @@ -386,39 +386,89 @@ export function createSQLiteDB(): DB { return dbApi; } -function translateSQL(sql: string): string { - let s = sql; - - // INSERT IGNORE → INSERT OR IGNORE - s = s.replace(/INSERT\s+IGNORE\s+INTO/gi, 'INSERT OR IGNORE INTO'); - - // NOW() → datetime('now','localtime') - s = s.replace(/NOW\(\)/gi, "datetime('now','localtime')"); - - // LEFT(col, N) → SUBSTR(col, 1, N) - s = s.replace(/LEFT\s*\(([^,]+),\s*(\d+)\)/gi, 'SUBSTR($1, 1, $2)'); - - // DATE_SUB(NOW(), INTERVAL N DAY) → datetime('now', '-N days') - s = s.replace(/DATE_SUB\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, - (_match, days) => `datetime('now', '-${days} days')`); - // Also handle case where NOW() hasn't been translated yet - s = s.replace(/DATE_SUB\s*\(\s*NOW\(\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, - (_match, days) => `datetime('now', '-${days} days')`); - - // DATE_ADD(expr, INTERVAL N DAY) → datetime(expr, '+N days') - // Order matters: NOW() is rewritten above into datetime('now','localtime'), - // which itself contains a comma — so the NOW() forms get dedicated rules - // before the general one, exactly as DATE_SUB does. - s = s.replace(/DATE_ADD\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, - (_match, days) => `datetime('now', '+${days} days')`); - s = s.replace(/DATE_ADD\s*\(\s*NOW\(\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, - (_match, days) => `datetime('now', '+${days} days')`); - // General form, e.g. the org spend-window query's DATE_ADD(?, INTERVAL 1 DAY). - // The captured expression is spliced through verbatim so a bound `?` stays a - // bound parameter — never interpolate the caller's value into the SQL. - s = s.replace(/DATE_ADD\s*\(\s*([^,()]+?)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, - (_match, expr, days) => `datetime(${expr}, '+${days} days')`); +/** + * Apply `fn` only to the parts of `sql` that sit OUTSIDE single-quoted string + * literals, leaving literal contents byte-for-byte intact. + * + * Every rule in translateSQL is a regex over raw SQL text, and several of them + * inject quote characters. Without this, a literal whose contents happen to look + * like MySQL syntax gets rewritten, and the injected quotes terminate the literal + * and restructure the statement. Anchoring the patterns narrows that but does not + * close it — `'DATE_ADD(a , INTERVAL 9 DAY)'` still matches an identifier capture. + * + * SQLite escapes a quote inside a literal by doubling it (''). + */ +function outsideStringLiterals(sql: string, fn: (chunk: string) => string): string { + const parts: string[] = []; + let i = 0; + let chunkStart = 0; + + while (i < sql.length) { + if (sql[i] !== "'") { i++; continue; } + + parts.push(fn(sql.slice(chunkStart, i))); + + let j = i + 1; + while (j < sql.length) { + if (sql[j] === "'") { + if (sql[j + 1] === "'") { j += 2; continue; } // escaped quote, keep scanning + break; + } + j++; + } + const end = Math.min(j + 1, sql.length); + parts.push(sql.slice(i, end)); // literal, verbatim + i = chunkStart = end; + } + + parts.push(fn(sql.slice(chunkStart))); + return parts.join(''); +} +export function translateSQL(sql: string): string { + // Token-level rewrites, applied only outside string literals. + let s = outsideStringLiterals(sql, (chunk) => { + let c = chunk; + + // INSERT IGNORE → INSERT OR IGNORE + c = c.replace(/INSERT\s+IGNORE\s+INTO/gi, 'INSERT OR IGNORE INTO'); + + // NOW() → datetime('now','localtime') + c = c.replace(/NOW\(\)/gi, "datetime('now','localtime')"); + + // LEFT(col, N) → SUBSTR(col, 1, N) + c = c.replace(/LEFT\s*\(([^,]+),\s*(\d+)\)/gi, 'SUBSTR($1, 1, $2)'); + + // DATE_SUB(NOW(), INTERVAL N DAY) → datetime('now', 'localtime', '-N days') + // `localtime` is required: NOW() became datetime('now','localtime') just above, + // so omitting it makes NOW() and DATE_SUB(NOW(), …) disagree by the host's UTC + // offset — a silent window shift on any non-UTC host. Six callers rely on this + // (projects/untracked.ts, projects/epic-stats.ts, projects/epic-summary.ts). + c = c.replace(/DATE_SUB\s*\(\s*datetime\('now','localtime'\)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_m, days) => `datetime('now', 'localtime', '-${days} days')`); + + // DATE_SUB / DATE_ADD (, INTERVAL N DAY) + // Anchored to `?` or a bare identifier: the expression is spliced through + // verbatim, so a bound `?` stays a bound parameter and the rewrite introduces + // no new placeholders (no positional-parameter shift). + c = c.replace(/DATE_SUB\s*\(\s*(\?|[A-Za-z_][A-Za-z0-9_.]*)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_m, expr, days) => `datetime(${expr}, '-${days} days')`); + c = c.replace(/DATE_ADD\s*\(\s*(\?|[A-Za-z_][A-Za-z0-9_.]*)\s*,\s*INTERVAL\s+(\d+)\s+DAY\s*\)/gi, + (_m, expr, days) => `datetime(${expr}, '+${days} days')`); + + // No DATE_ADD(NOW(), …) rule on purpose: nothing in the codebase writes that + // shape, and an unexercised rule is exactly where the localtime skew hid. The + // guard below makes the omission loud rather than silently wrong. + return c; + }); + + // Guard against the passthrough default. translateSQL is a whitelist rewriter: + // anything unmatched previously reached better-sqlite3 verbatim and failed at + // request time (GLOOK-41 surfaced as `near "1": syntax error`). Mocked suites + // never call this function, so such gaps stayed green in CI and were found by + // users. Fail loudly instead — and mysql-date-expressions.test.ts turns any new + // offender into a CI failure. Only non-literal text is inspected, so SQL stored + // as data cannot trip it. // ON DUPLICATE KEY UPDATE ... VALUES(col) → ON CONFLICT(...) DO UPDATE SET col = excluded.col const odkuMatch = s.match(/ON\s+DUPLICATE\s+KEY\s+UPDATE\s+([\s\S]+)$/i); if (odkuMatch) { @@ -454,5 +504,24 @@ function translateSQL(sql: string): string { `ON CONFLICT(${conflict}) DO UPDATE SET ${updateClause}`); } + // translateSQL is a whitelist rewriter with a passthrough default: anything it does + // not match reaches better-sqlite3 verbatim and fails at request time with a + // token-level parse error (GLOOK-41 was exactly this — `near "1": syntax error`). + // Mocked test suites never call this function, so such gaps stay green in CI and are + // found by users. Fail loudly here instead: a thrown error names the statement and is + // greppable, and `mysql-date-expressions.test.ts` turns any new offender into a CI + // failure rather than a production 500. + // Only non-literal text is inspected — SQL-looking text stored as data (a commit + // message, a Jira summary) must not be able to trip this. Reuses the same + // tokenizer as the rewrites, so "what counts as a literal" has one definition. + let nonLiteral = ''; + outsideStringLiterals(s, (chunk) => { nonLiteral += ` ${chunk} `; return chunk; }); + if (/\bDATE_ADD\b|\bDATE_SUB\b|\bINTERVAL\b/i.test(nonLiteral)) { + throw new Error( + `translateSQL: untranslated MySQL date expression reached the SQLite driver. ` + + `Add a rule to translateSQL() in src/lib/db/sqlite.ts. SQL: ${sql}`, + ); + } + return s; }