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
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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`,
|
||||
|
||||
Reference in New Issue
Block a user