Harden ValidateSQLRequest as the single SQL-safety chokepoint (move SHOW/RBAC gating off raw-regex)

[xe-nvdk]

Problem

The query API enforces its SQL-safety invariants — RBAC table extraction, SHOW-command gating, cross-database rejection, dangerous-keyword denylist, multi-statement rejection, read_parquet/arc_partition_agg blocking — with regex and string-scanning over the raw SQL, not a real parser. This is an inherently leaky abstraction: each invariant has to independently re-derive "what is a token / literal / comment / statement boundary," and every divergence from real SQL tokenization is a potential bypass.

PR #489 (a small SHOW-gating change) made this concrete. It took 7 Gemini rounds, each surfacing a distinct tokenizer edge case in the same gate:

1. Comment hides the command — /* x */ SHOW DATABASES (regex on raw SQL).
2. Quoted db name not recognized — SHOW TABLES FROM "db" (char class too narrow).
3. Path traversal — SHOW TABLES FROM .. (char class too wide; no identifier validation).
4. Multi-statement smuggling — SHOW DATABASES; SELECT 1 (anchored $ assumes one statement).
5. Whitespace before paren in TVF detection — generate_series (...).
6. Endpoint parity — Arrow endpoint had no SHOW gate at all.
7. Comment marker inside a quoted identifier — SHOW TABLES FROM "my--db" (stripped comments before masking literals → truncation → wrong db authorized).

Each was individually fixed and is covered by a regression test, and the defensive posture held throughout (Arc databases are virtual / never ATTACHed, so a missed SHOW returns DuckDB's empty catalog, not tenant data; the denylist blocks destructive statements). So these were defense-in-depth hardening, not live tenant-data leaks. But the rate of new edge cases in one small PR is the signal: the approach doesn't converge.

Proposed direction

Treat ValidateSQLRequest as the single SQL-safety chokepoint and make the SQL handling robust rather than regex-by-feature:

1. Normalize once, centrally. All endpoints already call ValidateSQLRequest. Do the literal-masking + comment-stripping there once, and have it expose the normalized form (and/or a small set of derived facts: statement count, leading keyword, whether it's a SHOW/metadata command) so downstream gates don't each re-scan raw SQL. The normalizeSQLForShow helper added in fix(api): add SHOW command RBAC-gating to estimateQuery endpoint #489 is a step toward this — generalize it.
2. Move SHOW / metadata detection off raw-regex onto the normalized form (or a lightweight statement classifier). The anchored-regex approach keeps sprouting edge cases (rounds 1, 2, 4, 7 were all "the regex saw the wrong string").
3. Evaluate a real parser/lexer for the safety layer. DuckDB's own parser, a Go SQL tokenizer, or EXPLAIN-based statement classification would replace the hand-rolled token scanning that RBAC depends on. Scope/perf to be assessed — the hot path matters (current extraction is 6–30 µs).
4. Property/fuzz test the chokepoint. A fuzz target over ValidateSQLRequest + the SHOW gate (comments, quotes, mixed quoting, statement separators, unicode, whitespace) would surface these classes before review does.

Relationship to other work

• Complements the [SQL parser modernization initiative] and Make duckdb_arrow the default build path (drop the tag) so default go test exercises shipped code #490 (make duckdb_arrow the default so default go test exercises the shipped path — round 6's Arrow-parity gap was invisible to untagged tests).
• This is the durable fix for the "whack-a-mole with Gemini on tokenizer nits" pattern. Not urgent (no known live bypass), but it's the root cause behind a recurring review cost.

Acceptance criteria (direction-setting; refine when scheduled)

• SQL-safety invariants (RBAC extraction, SHOW gating, statement-count, denylist) operate on a single, centrally-produced normalized representation — no gate re-scans raw req.SQL.
• SHOW/metadata-command detection no longer depends on anchored regex over raw SQL.
• A fuzz/property test covers the normalization + gating against comment/quote/separator/whitespace permutations.
• No regression in the existing query_rbac_test.go / TestValidateSQLRequest_* / TestShowTablesPattern / TestNormalizeSQLForShow suites.

[xe-nvdk]

New data point: GHSA-93cm-2v4m-c56c (merged in #495)

The read_parquet/arc_partition_agg blocking listed in this issue's problem statement just produced two more bypasses in the same ValidateSQLRequest chokepoint — exactly the "regex-by-feature keeps sprouting edge cases" pattern this issue tracks. Incomplete-fix of CVE-2026-47735; an external reporter found the first, internal review and Gemini found the other two during #495.

Three tokenizer-shaped misses, all in the I/O-function gate:

1. Alias / family incompleteness — the denylist matched only the literal read_parquet( and arc_partition_agg(, missing the parquet_scan alias and the rest of DuckDB's file-reading family (glob, read_blob, read_csv*, read_json*, parquet_metadata/schema, …). A name-denylist has to enumerate every spelling DuckDB accepts.
2. Position anchoring — the first replacement allowlist anchored to the FROM/JOIN regexes (the same ones RBAC's extractTableReferences uses), so it inherited their comma cross-join blind spot: FROM cpu, parquet_scan('…/other-db/…') slipped through (and was a regression vs the prior whole-string scan). Caught in internal review.
3. Quoted-identifier masking — the denylist matched against the shared normalised string, which masks double-quoted/backtick identifiers as string literals. So "parquet_scan"('…') — which DuckDB executes identically to the unquoted call — had its name hidden inside a __STR__ placeholder and bypassed the check. Caught by Gemini.

Severity note that raises the stakes for this issue: unlike the #489 SHOW cases (defense-in-depth — Arc DBs are virtual, so a missed SHOW returns an empty catalog, not tenant data), miss #1 and miss #3 were live cross-tenant data reads, reproduced end-to-end: a token scoped to attackerdb:read read another database's secret rows via parquet_scan(...) and via "parquet_scan"(...). This gate sits directly on tenant data, so its tokenizer divergences are exploitable, not just theoretical. The "no known live bypass" framing in the issue body is now outdated for the I/O-gate specifically.

#495 fixed all three at the regex layer (whole-string name denylist + a dedicated quote-stripping normalisation ioDenylistNormalise that exposes quoted identifiers while still masking string literals). That is another point-patch, not the durable fix — it reinforces this issue rather than closing it. Acceptance criterion 1 (single centrally-produced normalised representation) is directly relevant: misses #2 and #3 were both "this gate re-derived/reused the wrong normalisation." A real lexer that distinguishes identifier-quoting from string-quoting and tracks FROM-clause position would have made all three structurally impossible.