refactor: enforce read-only queries via SQLite PRAGMA instead of regex

package.json

@@ -51,6 +51,7 @@
 		"@anthropic-ai/mcpb": "^2.1.2",
 		"@modelcontextprotocol/sdk": "^1.29.0",
 		"cdb-converter": "^0.1.2",
+		"sql-query-identifier": "^3.1.1",
 		"sql.js": "^1.14.1",
 		"zod": "^3.25.76"
 	}

src/save-db.ts

@@ -66,6 +66,8 @@ export async function withSaveDb<T extends Record<string, unknown>>(
 		const cdbBuffer = await readFile(save.path);
 		db = cdbToSql(cdbBuffer, SQL);
 
+		db.run("PRAGMA query_only = ON;");
+
 		const output = await fn(db, save);
 
 		return validResponse(output);

src/tools/query-save.ts

@@ -1,4 +1,5 @@
 import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
+import { identify } from "sql-query-identifier";
 import { z } from "zod";
 import { withSaveDb } from "../save-db";
 
@@ -109,42 +110,51 @@ function explainQueryError(error: unknown): Error {
 		);
 	}
 
+	// Raised by `PRAGMA query_only = ON` when a statement tries to write.
+	if (/readonly database|not authorized/i.test(message)) {
+		return new Error(
+			"This tool is read-only — the query attempted to modify the save, which is not allowed.",
+		);
+	}
+
 	return error instanceof Error ? error : new Error(message);
 }
 
 /**
- * Reject anything that isn't a single read-only `SELECT`/`WITH` statement.
- * The save is loaded into an in-memory sql.js database (changes are never
- * written back to disk), but we still enforce read-only intent defensively.
+ * Enforce that a query is a single read-only statement.
+ *
+ * Parsing is delegated to `sql-query-identifier`, which tokenizes SQL properly:
+ * a `;` inside a string literal, comment or quoted identifier is not mistaken
+ * for a statement separator, and CTEs are classified by their leaf operation —
+ * `WITH … SELECT` reads (`LISTING`) while `WITH … DELETE` writes
+ * (`MODIFICATION`). Anything that isn't exactly one `LISTING` statement is
+ * rejected here; `PRAGMA query_only = ON` (see {@link withSaveDb}) stays as the
+ * engine-level backstop.
  */
 export function assertReadOnlyQuery(rawQuery: string): string {
-	// Strip a single trailing semicolon, then reject any further statement
-	// separators to prevent stacked statements.
+	// Strip a single trailing semicolon so a normal `SELECT …;` is accepted;
+	// the returned query is what gets prepared.
 	const query = rawQuery.trim().replace(/;\s*$/, "");
 
 	if (query.length === 0) {
 		throw new Error("Query is empty.");
 	}
 
-	if (query.includes(";")) {
-		throw new Error(
-			"Only a single statement is allowed — remove extra semicolons.",
-		);
+	const statements = identify(query, { strict: false, dialect: "sqlite" });
+
+	if (statements.length === 0) {
+		throw new Error("Query is empty.");
 	}
 
-	if (!/^(select|with)\b/i.test(query)) {
+	if (statements.length > 1) {
 		throw new Error(
-			"Only read-only SELECT (or WITH … SELECT) queries are allowed.",
+			"Only a single statement is allowed — remove extra semicolons.",
 		);
 	}
 
-	// Defense in depth: reject statements that could mutate or attach data.
-	const forbidden =
-		/\b(insert|update|delete|drop|create|alter|replace|attach|detach|reindex|vacuum|pragma|truncate)\b/i;
-	const match = forbidden.exec(query);
-	if (match) {
+	if (statements[0].executionType !== "LISTING") {
 		throw new Error(
-			`Write/DDL keyword "${match[0].toUpperCase()}" is not allowed — this tool is read-only.`,
+			"Only read-only SELECT (or WITH … SELECT) queries are allowed.",
 		);
 	}
 

test/save-db.test.ts

@@ -22,8 +22,8 @@ const cdbToSqlMock = cdbToSql as Mock;
 
 let dir: string;
 let savePath: string;
-/** A fake sql.js database; we only care that it gets closed. */
-const fakeDb = { close: vi.fn() };
+/** A fake sql.js database; we only care that it gets closed and configured. */
+const fakeDb = { close: vi.fn(), run: vi.fn() };
 
 beforeEach(async () => {
 	dir = await mkdtemp(join(tmpdir(), "pcm-save-db-"));
@@ -33,6 +33,7 @@ beforeEach(async () => {
 	cdbToSqlMock.mockReset();
 	cdbToSqlMock.mockReturnValue(fakeDb);
 	fakeDb.close.mockReset();
+	fakeDb.run.mockReset();
 });
 
 afterEach(async () => {
@@ -60,6 +61,19 @@ describe("withSaveDb", () => {
 		expect(save.path).toBe(savePath);
 	});
 
+	it("puts the database in read-only mode before running the callback", async () => {
+		const runOrder: string[] = [];
+		fakeDb.run.mockImplementation((sql: string) => runOrder.push(sql));
+
+		await withSaveDb(savePath, () => {
+			runOrder.push("callback");
+			return {};
+		});
+
+		expect(fakeDb.run).toHaveBeenCalledWith("PRAGMA query_only = ON;");
+		expect(runOrder).toEqual(["PRAGMA query_only = ON;", "callback"]);
+	});
+
 	it("supports async callbacks", async () => {
 		const result = await withSaveDb(savePath, async () => ({ async: true }));