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 0000000..8830866 --- /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 new file mode 100644 index 0000000..5489794 --- /dev/null +++ b/src/lib/__tests__/unit/sqlite-date-add.test.ts @@ -0,0 +1,135 @@ +/** + * 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`. + * + * 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('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 tmpDir: string; + let db: { execute: (sql: string, params?: any[]) => Promise<[T[], any]> }; + + 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 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; + try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch { /* already gone */ } + }); + + 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]; + // 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 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)`, + [committedAt, '2026-03-31'], + ) as [any[], any]; + return rows.length === 1; + }; + 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 2685ff0..cc18bbb 100644 --- a/src/lib/db/sqlite.ts +++ b/src/lib/db/sqlite.ts @@ -386,25 +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)'); +/** + * 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; + } - // 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')`); + 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) { @@ -440,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; }