I wrote an MCP server that lets an AI agent query a warehouse. A second model reviewed it and found a way past my SQL guard with a one-line alias. This post covers the design, the bug, the fix, and the other gaps a red team found.
The repo is warehouse-mcp. It serves DuckDB, Postgres and BigQuery.
The problem
Agents are good at writing SQL and bad at knowing what they should not read. Giving one a warehouse connection raises three worries:
- It reads personal data it should not see.
- It writes, or reads files and system state, because a prompt told it to.
- It runs a query that scans terabytes and sends you the bill.
You cannot fix this by asking the model to behave. A prompt-injected agent does what the injected text says. The controls have to sit outside the model.
The design
Three decisions shaped the server.
Parse, do not pattern-match. The guard parses agent SQL with sqlglot in the backend's dialect. It accepts one statement, a SELECT only, over declared datasets only, with a function allowlist and a capped LIMIT. A function the parser does not model is denied by default, because dialect-specific functions are where pg_read_file and sleeps hide.
Pseudonymize at the source. The guard rewrites every dataset reference into a policy subquery. That subquery projects only declared columns, replaces personal columns with a salted hash, and applies the role's row filter. All of it runs inside the warehouse.
def pseudonym(column: str, salt: str) -> exp.Expr:
"""A stable, salted pseudonym: equal inputs stay equal (joins, counts), names never leave."""
template = (f"CASE WHEN {column} IS NULL THEN NULL ELSE CONCAT('pii_', "
f"SUBSTRING(MD5(CONCAT('{salt}', CAST({column} AS TEXT))), 1, 12)) END")
return sqlglot.parse_one(template, read="duckdb")
count(DISTINCT editor) stays exact and joins on the pseudonym work. A probe like WHERE editor = 'Alice' matches nothing, because the comparison runs against the hash. Masking the output after lineage analysis can be bypassed with aliases and expressions. Here the raw column is not reachable.
The guard is not the boundary. Each backend is locked down on its own:
- DuckDB: read-only, no file access.
- Postgres: read-only transaction plus grants.
- BigQuery: dry run plus a byte cap.
On Postgres, the tests send writes straight to the backend with the guard out of the way and expect the engine to refuse them: first the read-only transaction, then, with that bypassed too, the grants. A parser gap alone should not be a breach. That goal mattered, because I then found a parser gap.
The bug
An independent review by a second model read the guard and found something my own suite had missed. Here is the shape of it, taken from a regression test:
WITH x AS (SELECT wiki FROM edits)
SELECT editor FROM edits_raw x, x
edits_raw is a physical table, not a declared dataset.
The guard has to tell two kinds of table node apart. A reference to a real dataset must be rewritten into the policy subquery. A reference to a CTE must be left alone, because the CTE body was already guarded.
My version decided this by looking up the table's alias in the scope's sources. This is the line that the fix (commit d1369b3) removed:
if isinstance(scope.sources.get(table.alias_or_name), Scope):
cte_refs.add(id(table))
alias_or_name returns the alias when there is one. In the query above, the physical table edits_raw is aliased x, and the scope has a CTE named x. The lookup says "this is a CTE" and the node is skipped. No allowlist check, no policy rewrite. The physical table reaches the warehouse as written.
The fix commit states the impact: raw PII, undeclared columns, and rows outside the role filter.
I had fixed a neighbouring case before. In WITH t AS (SELECT * FROM t) the inner t is the real table, so a global name match would wave it through. I moved to scope-based resolution and thought that was the end of it. I had swapped a name match for an alias match, and the alias match has the same flaw.
The fix
A table refers to a CTE only if its name is a CTE visible in its own scope. If the name does not match but the alias does, the query is refused.
for scope in scopes:
visible = {name.lower() for name in scope.cte_sources}
for table in scope.tables:
if table.args.get("db") or table.args.get("catalog"):
continue # qualified: never a CTE; rejected later
if table.name.lower() in visible:
cte_refs.add(id(table))
elif table.alias and table.alias.lower() in visible:
raise QueryRejected(f"Alias {table.alias!r} hides the CTE of the same name; "
"use a different alias.")
Refusing is simpler than resolving. No legitimate query needs to alias a real table to the name of a CTE in scope, so the guard removes the ambiguity instead of interpreting it.
The commit adds three cases that must be rejected:
"WITH x AS (SELECT wiki FROM edits) SELECT editor FROM edits_raw x, x",
"WITH x AS (SELECT wiki FROM edits) SELECT x.editor FROM edits x JOIN x y ON true",
"WITH x AS (SELECT wiki FROM edits) SELECT * FROM salaries AS x",
It also adds a positive test: a CTE referenced by its own name still works, including a self-join like FROM a AS b JOIN a. A guard that rejects everything is easy to write and useless to ship.
What else the red team caught
The adversarial suite is separate from the review. It runs attempts through the real service on a snapshot of about 100k Wikipedia edits. Each attempt must be refused, or come back with no raw editor names and no rows outside the role's filter. It found three gaps outside the SQL guard.
ReDoS in redaction. The email regex used unbounded repeats and backtracked quadratically on a long string with no @. The README's estimate: a 1 MB string from repeat('x', 1000000) would have held the server for about 12 minutes. Redaction ran after the warehouse timeout, so no timeout covered it. The fix bounds the repeats to RFC 5321 lengths and cuts cells before redaction:
_EMAIL = re.compile(r"[A-Za-z0-9._%+-]{1,64}@[A-Za-z0-9.-]{1,255}\.[A-Za-z]{2,24}")
The regression test asserts that a 40,000 character cell finishes in under half a second.
A single huge row skipped the result cap. The cap trimmed rows, so a result of one oversized row went out whole. The fix halves the longest text cells until the row fits, and the test checks that a 5,000 character cell comes back under a 1,000 character cap.
Pseudonyms relinked through titles. On Wikipedia, editing your own User: page produces a row where the editor column holds a pseudonym and the page title holds the real name. That pairs the two. In the snapshot where I found it, this re-identified 5.7% of editors. The fix adds conditional PII, pii_when: "namespace IN (2, 3)", so user-page titles are pseudonymized, and edit summaries are pseudonymized too. The regression test takes ground truth from a cleared role and checks that no pseudonymized row still carries the name in its title.
The independent review also found a leak the suite had missed.
Engine errors quoted data. An error like Could not convert string '<value>' skipped result redaction, because errors take a different path than rows. It reached both the agent and the audit log. The fix scrubs quoted values, redacts what is left, and caps the length:
_QUOTED = re.compile(r"'(?:[^']|'')*'|\"(?:[^\"]|\"\")*\"")
def scrub_error(message: str, max_chars: int = 300) -> str:
text = _QUOTED.sub("'…'", message)
text, _ = redact(text, phones=True)
return text if len(text) <= max_chars else text[:max_chars] + "…"
The agent still sees the error kind and where it happened, which is enough to fix a query. My earlier red-team test had asserted that a cast error showed the pseudonym, and I had treated that as acceptable. The test now asserts that no value comes back, for the analyst role and for the cleared role.
The numbers
- 104 red-team cases per engine, 0 leaks. 103 are attacks; one is a positive control that checks the cleared role still sees real names, so the PII checks are not passing vacuously. The attacks cover exfiltration, row-filter escapes, PII probes, file and system access, statement smuggling and resource abuse. Locally the suite runs on DuckDB, and on Postgres when a DSN is set. CI sets the DSN, so every push runs all 208 cases.
-
Parity. The same 15 queries return identical results on DuckDB, Postgres and BigQuery. The BigQuery run was live and is not part of CI, because it needs credentials.
docs/bigquery-run.mdhas the details, including a query that would scan 4.65 GB. The server refused it before billing: "above the 1.00 GB cap". - Agent eval. A Claude Haiku 5.5 agent answered 26 cases, 3 trials each, graded against ground-truth SQL with no LLM judge. Result: 78/78 correct, including 6 governance cases, at $0.0016 per question, with p95 latency of 6.3 s. All 10 rejected tool calls were self-corrected from the error message.
The BigQuery run also found one more bug. It was a correctness problem, not a leak: time literals were cast to DATETIME against TIMESTAMP columns.
Lessons
Layers matter when one fails. The engines were locked down separately, and the Postgres tests prove it with the guard bypassed. I do not want to overstate that. The alias bypass still skipped the row filter and the pseudonymization, because those live in the guard. Defense in depth limited what a parser gap could do. It did not make the gap harmless.
Every finding gets a regression test. Each fix above came with a test that fails without it. Those regressions are part of the 104 cases above, so a fix cannot quietly come undone.
Independent review finds what your own suite cannot. I wrote the guard and the red-team suite with the same assumptions. The reviewer did not share them, and it found the alias gap and the error leak. A passing suite tells you nothing about attempts you never thought of.
Prefer refusing over interpreting. When a construct is ambiguous and never needed, reject it. The alias rule is a few lines and removes a whole class of confusion.
To try it, the repo has a demo that needs no keys: uvx warehouse-mcp --demo.
Questions or findings are welcome on Telegram: @tanngoctr.
Top comments (0)