From cafa86a6cffa1f7fc69f018ddf4e0ff739363c6d Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 24 Apr 2026 06:18:27 +0200 Subject: [PATCH] fix(tools/sqlite): reject comments, terminators, and pagination keywords in where= The structured SQLite helper interpolates `where=` directly into SQL. A crafted clause like `where=1=1 LIMIT 1000000 --` could comment out the helper's bound `LIMIT ? OFFSET ?`, returning the full table in violation of the documented pagination contract. Validate where= at the selector boundary and reject SQL comments, statement terminators, and pagination/attach/pragma keywords. Raw SQL remains available via ?q=SELECT... for callers that need it. Fixes #735 --- .../coding-agent/src/tools/sqlite-reader.ts | 18 ++++++++++++++++++ .../coding-agent/test/tools/sqlite.test.ts | 12 ++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/coding-agent/src/tools/sqlite-reader.ts b/packages/coding-agent/src/tools/sqlite-reader.ts index b94adbf80..45a82e9d0 100644 --- a/packages/coding-agent/src/tools/sqlite-reader.ts +++ b/packages/coding-agent/src/tools/sqlite-reader.ts @@ -162,6 +162,21 @@ function parseLimit(value: string | null, fallback: number): number { return Math.min(parsed, MAX_QUERY_LIMIT); } +function validateWhereClause(where: string): void { + // Reject SQL comments and statement terminators so `where=` cannot rewrite the + // helper's pagination (e.g. `where=1=1 LIMIT 1000000 --`). + if (/--|\/\*|\*\/|;/.test(where)) { + throw new ToolError( + "SQLite 'where' clause must not contain comments or statement terminators; use '?q=SELECT ...' for raw SQL", + ); + } + if (/\b(?:limit|offset|union|intersect|except|attach|detach|pragma)\b/i.test(where)) { + throw new ToolError( + "SQLite 'where' clause must not contain LIMIT/OFFSET/UNION/INTERSECT/EXCEPT/ATTACH/DETACH/PRAGMA; use '?q=SELECT ...' for raw SQL", + ); + } +} + function parseOffset(value: string | null): number { if (value === null || value.trim().length === 0) { return 0; @@ -361,6 +376,9 @@ export function parseSqliteSelector(subPath: string, queryString: string): Sqlit } const where = params.get("where")?.trim() || undefined; + if (where !== undefined) { + validateWhereClause(where); + } const order = params.get("order")?.trim() || undefined; const hasQueryParams = params.has("limit") || params.has("offset") || order !== undefined || where !== undefined; if (hasQueryParams) { diff --git a/packages/coding-agent/test/tools/sqlite.test.ts b/packages/coding-agent/test/tools/sqlite.test.ts index f72ed8d2d..198918dcd 100644 --- a/packages/coding-agent/test/tools/sqlite.test.ts +++ b/packages/coding-agent/test/tools/sqlite.test.ts @@ -288,6 +288,18 @@ describe("SQLite tool support", () => { expect(text).not.toContain("Bob"); }); + it("rejects where= clauses that try to bypass pagination", () => { + expect(() => parseSqliteSelector("users", "where=1=1 LIMIT 1000000 --&limit=2&offset=0")).toThrow( + /comments or statement terminators/i, + ); + expect(() => parseSqliteSelector("users", "where=status='active' LIMIT 1")).toThrow( + /LIMIT\/OFFSET\/UNION/i, + ); + expect(() => parseSqliteSelector("users", "where=1=1; DROP TABLE users")).toThrow( + /comments or statement terminators/i, + ); + }); + it("executes raw read-only SQL queries", async () => { const result = await readTool.execute("sqlite-raw-query", { path: `${sqlitePath}?q=SELECT+name+FROM+users+ORDER+BY+id+LIMIT+2`,