Skip to content

Commit 880bf60

Browse files
committed
test(tsql): pin that no mutating query can pass the query boundary
The agent's run_query path compiles TRQL, which has no write statements, so a mutation cannot parse. Cover the deny-list evasions a keyword filter would miss (casing, comments, a statement smuggled after a semicolon) and keep positive controls so the negatives can't pass by rejecting everything.
1 parent f5a146b commit 880bf60

1 file changed

Lines changed: 100 additions & 0 deletions

File tree

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { describe, expect, it } from "vitest";
2+
import { compileTSQL, parseTSQLSelect, SyntaxError as TSQLSyntaxError } from "./index.js";
3+
import { column, type TableSchema } from "./query/schema.js";
4+
5+
// TSQL has no write statements at all, so a mutating query cannot parse.
6+
// These tests pin that, so extending the grammar can't quietly add a write path.
7+
8+
const taskRunsSchema: TableSchema = {
9+
name: "task_runs",
10+
clickhouseName: "trigger_dev.task_runs_v2",
11+
columns: {
12+
id: { name: "id", ...column("String") },
13+
status: { name: "status", ...column("String") },
14+
created_at: { name: "created_at", ...column("DateTime64") },
15+
organization_id: { name: "organization_id", ...column("String") },
16+
project_id: { name: "project_id", ...column("String") },
17+
environment_id: { name: "environment_id", ...column("String") },
18+
},
19+
tenantColumns: {
20+
organizationId: "organization_id",
21+
projectId: "project_id",
22+
environmentId: "environment_id",
23+
},
24+
};
25+
26+
const compileOptions = {
27+
tableSchema: [taskRunsSchema],
28+
enforcedWhereClause: {
29+
organization_id: { op: "eq", value: "org_123" },
30+
project_id: { op: "eq", value: "proj_456" },
31+
environment_id: { op: "eq", value: "env_789" },
32+
},
33+
} as const;
34+
35+
const mutating = [
36+
["INSERT", "INSERT INTO task_runs (id) VALUES ('run_1')"],
37+
["UPDATE", "UPDATE task_runs SET status = 'COMPLETED' WHERE id = 'run_1'"],
38+
["DELETE", "DELETE FROM task_runs WHERE id = 'run_1'"],
39+
["DROP", "DROP TABLE task_runs"],
40+
["TRUNCATE TABLE", "TRUNCATE TABLE task_runs"],
41+
["ALTER", "ALTER TABLE task_runs ADD COLUMN leaked String"],
42+
["CREATE", "CREATE TABLE leaked (id String)"],
43+
["GRANT", "GRANT SELECT ON task_runs TO someone"],
44+
["OPTIMIZE", "OPTIMIZE TABLE task_runs FINAL"],
45+
["SYSTEM", "SYSTEM SHUTDOWN"],
46+
];
47+
48+
// Shapes that defeat a keyword deny-list but not a grammar without write statements.
49+
const evasions = [
50+
["lower case", "delete from task_runs where id = 'run_1'"],
51+
["mixed case", "DeLeTe FROM task_runs WHERE id = 'run_1'"],
52+
["leading line comment", "-- harmless\nDELETE FROM task_runs"],
53+
["leading block comment", "/* harmless */ DROP TABLE task_runs"],
54+
["comment between keywords", "DROP /* x */ TABLE task_runs"],
55+
["leading whitespace and newlines", "\n\n\t TRUNCATE TABLE task_runs"],
56+
];
57+
58+
describe("TSQL is read-only by construction", () => {
59+
it.each(mutating)("rejects %s at the parse boundary", (_label, query) => {
60+
expect(() => parseTSQLSelect(query)).toThrow(TSQLSyntaxError);
61+
});
62+
63+
it.each(evasions)("rejects a mutating query written as %s", (_label, query) => {
64+
expect(() => parseTSQLSelect(query)).toThrow(TSQLSyntaxError);
65+
});
66+
67+
it.each(mutating)("refuses to compile %s", (_label, query) => {
68+
expect(() => compileTSQL(query, compileOptions as never)).toThrow();
69+
});
70+
71+
// Positive control: the negatives above must fail because they mutate,
72+
// not because the parser rejects everything.
73+
it("still parses an ordinary SELECT", () => {
74+
const ast = parseTSQLSelect("SELECT id, status FROM task_runs WHERE status = 'FAILED'");
75+
expect(ast.expression_type).toBe("select_query");
76+
});
77+
78+
// TRUNCATE is a keyword in the lexer because it is a rounding function,
79+
// not because a TRUNCATE statement exists.
80+
it("treats TRUNCATE as a function, not a statement", () => {
81+
const ast = parseTSQLSelect("SELECT truncate(1.9) FROM task_runs");
82+
expect(ast.expression_type).toBe("select_query");
83+
});
84+
});
85+
86+
describe("a mutation cannot ride along behind a valid SELECT", () => {
87+
const smuggled = [
88+
["semicolon", "SELECT id FROM task_runs; DROP TABLE task_runs"],
89+
["semicolon and newline", "SELECT id FROM task_runs;\nDELETE FROM task_runs"],
90+
["two semicolons", "SELECT id FROM task_runs;; TRUNCATE TABLE task_runs"],
91+
];
92+
93+
// The parser is anchored to EOF, so a trailing statement is rejected rather
94+
// than silently dropped. Silently dropping it would also be safe, but it
95+
// would hide the smuggling attempt from the caller.
96+
it.each(smuggled)("rejects a mutation appended after a %s", (_label, query) => {
97+
expect(() => parseTSQLSelect(query)).toThrow(TSQLSyntaxError);
98+
expect(() => compileTSQL(query, compileOptions as never)).toThrow();
99+
});
100+
});

0 commit comments

Comments
 (0)