fix(security): close eight PII-denylist bypasses (v0.2.0) - #1
Merged
Conversation
The denylist could be defeated eight ways. Two were reported from an
adversarial review of a deployed agent; the other six surfaced while
fixing them. Each was confirmed against 0.1.1 with a reproducing query
before being fixed, and each has a regression test.
Scope aliasing. PiiProjectionRule checked only the outermost select list
via outermost_projection_names(), so any inner scope that renamed a
denied column laundered it:
WITH c AS (SELECT BillingCity AS city FROM t) SELECT city FROM c
Derived tables, UNION arms and multi-hop alias chains worked the same
way. Checking now runs over every SELECT scope and matches the
underlying column names in the scope that names them, so an alias cannot
launder a denied column.
Value probing. The denylist gated projection only, so WHERE, GROUP BY,
HAVING and ORDER BY references passed. None return the column, but each
answers a yes/no question about its value, and enough queries
reconstruct it. New pii_mode config ("reference" | "project", default
"reference") denies any reference to a denied column. "project" is the
documented loosening path and is still all-scope.
Found while fixing the above:
- SELECT c FROM tbl AS c returned every column of every row, PII
included, and the guard auto-executed it. A bare table alias in a
value position expands to the whole row; it parses as an ordinary
column, so the denylist had nothing to match. Strictly worse than the
SELECT * already blocked. New NoUnresolvableColumnsRule.
- NATURAL JOIN joins on whichever columns the tables share, which the
guard cannot enumerate without a schema. Now denied.
- JOIN ... USING (email) produced zero exp.Column nodes, so reference
mode never saw it - a working single-query value oracle. Same for
AS g(email) column aliases and STRUCT('x' AS email) field names; all
three carry names as bare exp.Identifier and are now harvested.
- The star check only inspected the projection's root node, so
OBJECT_CONSTRUCT(*), COLUMNS(*), * APPLY(f) and ROW(c.*) passed on
non-BigQuery dialects. It is now a deep walk with COUNT(*) as the
explicit carve-out. ClickHouse COLUMNS('regex'), which has no Star
node at all, is matched on node type.
- Qualified t.* bypassed the star rule even at top level: it parses as
an exp.Column wrapping a Star.
- Every exp.AggFunc counted as PII-neutralising, so MAX(email),
ARRAY_AGG(email) and STRING_AGG(email) returned real values through
project mode. Only aggregates reducing to a derived statistic qualify
now.
NoSelectStarRule replaces NoTopLevelStarRule, which stays importable as
an alias. default_rules ordering semantics are unchanged.
BREAKING: queries 0.1.1 allowed are now denied. Alias laundering,
inner-scope and nested stars, whole-row aliases, NATURAL JOIN,
identifier-only references and PII through value-preserving aggregates
have no supported way back - that is the point. Denied columns in
predicates can be restored with pii_mode="project", which re-opens the
value-probing oracle. The bundled Q1 identity-resolution query is now
denied in both modes: its CTE projects the denied columns, and
COUNTIF(email_norm = 'target') is itself an oracle.
Also: minimum Python is now 3.11. The package has imported enum.StrEnum
since 0.1.0 while advertising >=3.10, so import failed on 3.10 and that
CI job could never have passed. Metadata, classifiers, ruff
target-version, mypy python_version and the CI matrix now agree.
Cleared two pre-existing CI failures: a redundant int() around
math.floor in format_cost (RUF046), and a strict-mypy no-untyped-call on
sqlglot's unalias(), replaced with an equivalent .this read.
Tests: 74 -> 204.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes eight ways the PII denylist could be defeated. Two came from the
adversarial review of the deployed agent; six more surfaced while fixing those.
Every one was reproduced against 0.1.1 before being fixed, and every one has a
regression test.
The most severe was not in the original report:
SELECT c FROM tbl AS creturned every column of every row, PII included, with
auto_execute=True.The two reported issues
PiiProjectionRulechecked only the outermost selectlist, so a CTE / derived table / UNION arm / multi-hop chain could rename a
denied column and launder it. Now checked across every
SELECTscope,matching on the underlying column names in the scope that names them.
WHERE BillingCity = 'Columbus',GROUP BY,HAVINGandORDER BYpassed.New
pii_mode("reference"default |"project") denies any reference.Found while fixing them
SELECT c FROM tbl AS cNATURAL JOINJOIN ... USING (email)exp.Columnnodes → invisible to reference modeOBJECT_CONSTRUCT(*),COLUMNS(*),* APPLY(f),ROW(c.*)SELECT t.*ColumnwrappingStar; missed even at top levelMAX(email),ARRAY_AGG(email)AggFuncexemption returned real valuesBreaking changes
Queries 0.1.1 allowed are now denied. The bypasses have no supported way back.
Denied columns in predicates can be restored with
pii_mode="project"— whichre-opens the probing oracle. The bundled
Q1identity-resolution query is nowdenied in both modes; rationale is in the test and the CHANGELOG.
Minimum Python is now 3.11 (the code has required it since 0.1.0, so the
3.10 CI job could never have passed).
Review notes
NoUnresolvableColumnsRuleresolves ambiguity from the AST rather thandenying on a bare name collision.
WITH revenue AS (... SUM(x) AS revenue) SELECT revenue FROM revenueis correctly read as a column reference — anearlier cut denied it, which would have got the rule switched off in prod.
UNNESTaliases are deliberately not covered: indistinguishable at parsetime from the legitimate scalar-array form, and no worse than selecting the
struct column directly. Documented as a denylist-config concern.
re-identification, side channels.
Verification
ruff clean,
mypy srcclean, 204 tests passing (was 74), all three examplesrun. Consumer repos untouched — they pin sql-guard and pick this up via
uv lock --upgrade-package sql-guardseparately.Still open (pre-existing, not fixed here)
SelectOnlyRulerejects top-levelEXCEPT/INTERSECT(sqlglot derivesthem from
SetOperation, notUnion). Fails closed.AllowedTablesRuleruns last, so allowlist breaches are under-reported indecision.reasontelemetry.