security: close a read-only-guard bypass via a welded quoted identifier

CHANGELOG.md

@@ -10,6 +10,29 @@ is the source of truth a host installs against — bumping it is what invalidate
 user's plugin cache (see [CONTRIBUTING.md](CONTRIBUTING.md)). Each released section
 below corresponds to one such version.
 
+## [Unreleased]
+
+### Security
+
+- **Closed a read-only-guard bypass via a welded quoted identifier.** A double-quoted
+  identifier is self-delimiting in SQL on **both** ends, so `SELECT*FROM"pg_read_file"(…)`
+  and `SELECT "x"INTO evil FROM t` are valid statements with no whitespace either side of
+  the quote. The guard's lexer dropped the quote characters without re-supplying those
+  boundaries, fusing neighbouring tokens into one (`FROM"pg_class"` → `FROMpg_class`,
+  `"x"INTO` → `xINTO`) and destroying the word-boundary anchor every deny-list pattern
+  matches on — so the gate stopped *seeing* the token rather than allowing it, and returned
+  no rejection. Verified against PostgreSQL 16: the leading form reads a server-side file
+  and the trailing form creates a table from `SELECT … INTO`, both while the guard passed
+  them. Row locks (`FOR SHARE`) were reachable the same way. The lexer now re-supplies a
+  separator on either side, when and only when the quote was actually separating two word
+  characters — so a qualified name (`t."current_user"`) still neutralizes to one token,
+  `t.current_user`, rather than being split. This restores an invariant the lexer already
+  documented for comments and literals ("never empty"); the identifier branch was the one
+  place not honouring it. Prior corpus cases all happened to carry a space before the
+  quote, which is why this stayed invisible; the corpus now pins both weld directions and
+  asserts the neutralized token structure directly, so neither a one-sided fix nor a
+  blanket separator can pass.
+
 ## [0.5.0] — 2026-07-25
 
 ### Changed

packages/agami-core/src/sql_guard.py

@@ -74,6 +74,18 @@ def _neutralize(sql: str) -> str:
     deliberately NOT an escape here — engines disagree (MySQL yes, standard PG no),
     and not honoring it can only stop a literal *early* (fail safe), never late.
     """
+    def _last_emitted(chunks: list[str]) -> str:
+        """The last character actually emitted, skipping empty chunks.
+
+        Peeking at `chunks[-1]` alone is wrong: a zero-length identifier (`""`) appends an
+        empty string, which would hide the real preceding character and drop a separator
+        that is needed.
+        """
+        for chunk in reversed(chunks):
+            if chunk:
+                return chunk[-1]
+        return ""
+
     out: list[str] = []
     i, n = 0, len(sql)
     while i < n:
@@ -129,7 +141,29 @@ def _neutralize(sql: str) -> str:
                 j += 1
             # A pathological identifier like "a;b" reduces to a;b and trips the
             # multi-statement check — a deliberate, safe-direction hardening choice.
+            #
+            # The quotes are ALSO the token delimiters, on BOTH ends: a delimited
+            # identifier is self-delimiting, so `FROM"pg_class"` and `"x"INTO` are both
+            # valid SQL the engine runs. Dropping the quotes without re-supplying those
+            # boundaries fuses neighbouring tokens into one — `FROM"pg_class"` ->
+            # `FROMpg_class`, `"x"INTO` -> `xINTO` — which defeats the `\b` anchors every
+            # deny-list pattern below relies on. The gate then stops *seeing* the token
+            # rather than allowing it, so it returns no rejection at all. This is the same
+            # invariant the docstring states for comments and literals ("never empty");
+            # the identifier branch is simply the one place that was not honouring it.
+            #
+            # Re-supply a boundary only where the quote was actually separating two word
+            # chars, so a qualified name stays one token: `t."col"` must neutralize to
+            # `t.col`, not `t. col`. That structural fidelity is the neutralizer's job —
+            # it removes hiding places without re-tokenizing the statement — and it is
+            # pinned by an explicit output test, since no current rule distinguishes the
+            # two spellings on its own.
+            if _last_emitted(out).isalnum() or _last_emitted(out) == "_":
+                out.append(" ")
             out.append("".join(buf))
+            nxt = sql[j] if j < n else ""
+            if nxt.isalnum() or nxt == "_":
+                out.append(" ")
             i = j
         elif sql[i] == "$":  # dollar-quoted string literal ($$...$$ or $tag$...$tag$)
             # Only a `$tag$` with a MATCHING close delimiter is a literal we can blank.

plugins/agami/lib/sql_guard.py

@@ -74,6 +74,18 @@ def _neutralize(sql: str) -> str:
     deliberately NOT an escape here — engines disagree (MySQL yes, standard PG no),
     and not honoring it can only stop a literal *early* (fail safe), never late.
     """
+    def _last_emitted(chunks: list[str]) -> str:
+        """The last character actually emitted, skipping empty chunks.
+
+        Peeking at `chunks[-1]` alone is wrong: a zero-length identifier (`""`) appends an
+        empty string, which would hide the real preceding character and drop a separator
+        that is needed.
+        """
+        for chunk in reversed(chunks):
+            if chunk:
+                return chunk[-1]
+        return ""
+
     out: list[str] = []
     i, n = 0, len(sql)
     while i < n:
@@ -129,7 +141,29 @@ def _neutralize(sql: str) -> str:
                 j += 1
             # A pathological identifier like "a;b" reduces to a;b and trips the
             # multi-statement check — a deliberate, safe-direction hardening choice.
+            #
+            # The quotes are ALSO the token delimiters, on BOTH ends: a delimited
+            # identifier is self-delimiting, so `FROM"pg_class"` and `"x"INTO` are both
+            # valid SQL the engine runs. Dropping the quotes without re-supplying those
+            # boundaries fuses neighbouring tokens into one — `FROM"pg_class"` ->
+            # `FROMpg_class`, `"x"INTO` -> `xINTO` — which defeats the `\b` anchors every
+            # deny-list pattern below relies on. The gate then stops *seeing* the token
+            # rather than allowing it, so it returns no rejection at all. This is the same
+            # invariant the docstring states for comments and literals ("never empty");
+            # the identifier branch is simply the one place that was not honouring it.
+            #
+            # Re-supply a boundary only where the quote was actually separating two word
+            # chars, so a qualified name stays one token: `t."col"` must neutralize to
+            # `t.col`, not `t. col`. That structural fidelity is the neutralizer's job —
+            # it removes hiding places without re-tokenizing the statement — and it is
+            # pinned by an explicit output test, since no current rule distinguishes the
+            # two spellings on its own.
+            if _last_emitted(out).isalnum() or _last_emitted(out) == "_":
+                out.append(" ")
             out.append("".join(buf))
+            nxt = sql[j] if j < n else ""
+            if nxt.isalnum() or nxt == "_":
+                out.append(" ")
             i = j
         elif sql[i] == "$":  # dollar-quoted string literal ($$...$$ or $tag$...$tag$)
             # Only a `$tag$` with a MATCHING close delimiter is a literal we can blank.

tests/test_sql_guard.py

@@ -29,7 +29,7 @@
 from typing import Any
 
 import pytest
-from sql_guard import _MAX_SQL_CHARS, check_read_only
+from sql_guard import _MAX_SQL_CHARS, _neutralize, check_read_only
 
 # ---------------------------------------------------------------------------
 # Accept — valid single read-only statements
@@ -373,6 +373,96 @@ def test_red_team_quoted_dangerous_fn_bypass(sql: str) -> None:
     assert check_read_only(sql) is not None, f"Quoted-fn bypass NOT blocked: {sql!r}"
 
 
+@pytest.mark.parametrize(
+    "sql",
+    [
+        # WELDED quoted identifier — no whitespace between the preceding keyword and the
+        # opening `"`. A delimited identifier is self-delimiting in SQL (the quote IS the
+        # token boundary), so these are valid statements the engine happily runs; verified
+        # on PostgreSQL 16 (`SELECT"pg_read_file"('/tmp/x')` returns the file contents).
+        # Dropping the quotes without re-supplying a separator fuses two tokens into one
+        # (`FROM"pg_class"` -> `FROMpg_class`), which destroys the `\b` anchor every
+        # deny-list pattern relies on and silently blinds the gate.
+        "SELECT\"pg_read_file\"('/etc/passwd')",
+        "SELECT*FROM\"pg_read_file\"('/etc/passwd')",
+        'SELECT"pg_sleep"(10)',
+        'SELECT"dblink"(\'host=evil\', \'select 1\')',
+        'SELECT"pg_terminate_backend"(123)',
+        'SELECT"set_config"(\'statement_timeout\', \'0\', false)',
+        'SELECT 1 FROM"pg_class"WHERE"pg_sleep"(10) IS NULL',
+        # The weld can also appear mid-statement, after a non-keyword word char.
+        'SELECT a FROM t WHERE b="pg_sleep"(1)',
+        # The CLOSING quote delimits too, so a keyword can hide on the trailing side.
+        # `SELECT ... INTO <table>` is a write that opens with SELECT, which is exactly
+        # why INTO is in the DML/DDL list; `\bINTO\b` does not match `xINTO`. Verified on
+        # PostgreSQL 16: each of these creates a table holding the source rows.
+        'SELECT "x"INTO evil FROM t',
+        'SELECT"x"INTO evil FROM t',
+        'SELECT t."x"INTO evil FROM t',
+        'SELECT 1 AS"a"INTO evil',
+        'WITH c AS (SELECT 1 AS "x")SELECT "x"INTO evil FROM c',
+        # Row-lock rule, same root cause. FOR UPDATE survives only because bare UPDATE is
+        # independently in the DML list; FOR SHARE has no such backstop.
+        'SELECT * FROM t AS"a"FOR SHARE',
+        'SELECT * FROM t AS"a"FOR KEY SHARE',
+    ],
+)
+def test_red_team_welded_quoted_dangerous_fn_bypass(sql: str) -> None:
+    """A quoted identifier welded to the preceding token must not escape the gate.
+
+    Regression for the `_neutralize` weld: every pre-existing case in the corpus above
+    happens to carry a space before the `"`, so the missing separator was invisible.
+    """
+    assert check_read_only(sql) is not None, f"Welded quoted-fn bypass NOT blocked: {sql!r}"
+
+
+@pytest.mark.parametrize(
+    "sql",
+    [
+        # The separator must be re-supplied ONLY where the quote was actually delimiting
+        # two word chars. A blanket space would break qualified names — `t."col"` must stay
+        # `t.col`, not become `t. col` — so these legitimate reads must still pass.
+        'SELECT t."current_user" FROM t',
+        'SELECT "order id" FROM orders',
+        'SELECT "name", "email" FROM customers',
+        'SELECT c."email" FROM customers c',
+        'SELECT "schema"."table" FROM "schema"."table"',
+        'SELECT a."b" FROM x a',
+    ],
+)
+def test_welded_fix_does_not_over_block_legitimate_quoted_identifiers(sql: str) -> None:
+    """The weld fix must not turn a qualified quoted column into a false positive."""
+    assert check_read_only(sql) is None, f"Legitimate quoted identifier wrongly blocked: {sql!r}"
+
+
+@pytest.mark.parametrize(
+    ("sql", "expected"),
+    [
+        # A separator is re-supplied ONLY where the quote actually separated two word
+        # chars — so a qualified name survives as ONE token.
+        ('SELECT t."current_user" FROM t', "SELECT t.current_user FROM t"),
+        ('SELECT "schema"."table" FROM "schema"."table"', "SELECT schema.table FROM schema.table"),
+        ('SELECT c."email" FROM customers c', "SELECT c.email FROM customers c"),
+        # ...and IS re-supplied on both sides where it was separating word chars.
+        ('SELECT * FROM"pg_class"', "SELECT * FROM pg_class"),
+        ('SELECT "x"INTO evil FROM t', "SELECT x INTO evil FROM t"),
+        ('SELECT 1 AS"a"INTO evil', "SELECT 1 AS a INTO evil"),
+    ],
+)
+def test_neutralize_preserves_token_structure(sql: str, expected: str) -> None:
+    """Pin the *shape* of the neutralized text, not just the gate's yes/no.
+
+    Every rule here is `\\b`-anchored, so a blanket separator on both sides would block the
+    same attacks and pass the same negatives — the gate-level tests alone cannot tell the
+    two designs apart, and a future refactor could silently swap one for the other. What a
+    blanket space would change is token *structure*: `t."col"` would become `t. col`, two
+    tokens where the statement meant one. Any rule that reasons about qualification (a
+    pattern anchored on a preceding `.`, say) would then read a qualified column as a bare
+    identifier. Asserting the neutralized string keeps that decision checkable.
+    """
+    assert _neutralize(sql) == expected
+
+
 @pytest.mark.parametrize(
     "sql",
     [