diff --git a/CHANGELOG.md b/CHANGELOG.md index f9244525..44f93766 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,28 @@ below corresponds to one such version. ### Security +- **Gated PII masking on a whole-output proof; an output that cannot be proven clean is + now refused.** The mask decision quantified over the projections the detector *found* + (`all_maskable`), which proves "every offending projection we saw is maskable" but never + "no sensitive value reaches the output". While any detected projection refused the whole + query that gap was harmless; once a maskable projection let the query **run**, one seen + column unblocked the entire result set and every co-projected value the name-based + detector missed came back raw, beside a `***` implying the row had been protected. + `SensitiveCheckResult.output_provable` is the missing half: it is False when an + output-bearing arm draws through a source whose body is not walked (a derived table, a CTE + the arm selects from, a `VALUES`/`LATERAL` source), projects a scalar subquery, or projects + a qualifier naming none of that arm's own sources. `certainty="provable"` now requires it, + so an unprovable output falls back to a refusal. Two further binding bugs made the proof + itself unsound and are closed with it: a subquery alias could **shadow** an outer alias of + the same name (`FROM customers t` outside, `FROM orders t` inside an `EXISTS` body), silently + rebinding the outer `t.ssn` to a table declaring no `ssn` so it was never detected while the + qualifier still "resolved"; and two aliases folding to the same name (`FROM x "AB", customers + ab` with `AB.ssn`) resolved by exact-match-first to the quoted one, where the engine folds + onto the other. Alias binding is now shadow-aware and fails closed on a genuinely ambiguous + fold. The proof is scoped to what can actually put a value in the output, so a subquery in + `WHERE`/`HAVING`/`ORDER BY`, a JOIN's `ON` condition, or a CTE no arm selects from no longer + downgrades an ordinary maskable query to a refusal. + - **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 diff --git a/packages/agami-core/src/execute_sql.py b/packages/agami-core/src/execute_sql.py index 6f0e2285..04fa54b9 100644 --- a/packages/agami-core/src/execute_sql.py +++ b/packages/agami-core/src/execute_sql.py @@ -1296,14 +1296,27 @@ def _model_safety( mask_plan: MaskPlan | None = None sens = RT.check_sensitive_projection(sql, org, ctx=ctx) if sens.action == "refuse": + # Two conditions, and BOTH are required. `all_maskable` quantifies over the projections + # the detector FOUND, so on its own it proves only "every offending projection we saw is + # maskable" — never "no sensitive value reaches the output". Under the earlier refuse-only + # behaviour that gap leaked only when nothing was detected (anything detected blocked the + # whole query); once a maskable projection let the query RUN, one seen column unblocked the + # entire result set and every co-projected value the name-based detector missed came back + # raw — beside a `***` implying the row had been protected. `output_provable` is the + # missing half: the detector could see the whole output, not just part of it. all_maskable = bool(sens.projections) and all( p.maskable and p.output_index is not None for p in sens.projections ) + # `getattr`, not attribute access: this file is VENDORED into the plugin while + # semantic_model/runtime.py resolves from the separately-versioned installed package, so a + # newer plugin can meet an older `SensitiveCheckResult`. A missing field then means "this + # build cannot prove the output", which is exactly the fail-closed answer. + provable = all_maskable and getattr(sens, "output_provable", False) verdict = Verdict( cls="data_protection", rule="sensitive_projection", severity="high", - certainty="provable" if all_maskable else "uncertain", + certainty="provable" if provable else "uncertain", detail=sens.reason, remediation=sens.suggestion or "", ) diff --git a/packages/agami-core/src/semantic_model/runtime.py b/packages/agami-core/src/semantic_model/runtime.py index 4e8a7ef1..59e861cb 100644 --- a/packages/agami-core/src/semantic_model/runtime.py +++ b/packages/agami-core/src/semantic_model/runtime.py @@ -31,6 +31,7 @@ from __future__ import annotations import re +from collections.abc import Mapping from dataclasses import dataclass, field from difflib import SequenceMatcher from typing import Any, Callable, Optional @@ -457,11 +458,23 @@ class SensitiveCheckResult: reason: str = "" suggestion: Optional[str] = None # Per-offending-projection traceability (ACE-041): the maskable-vs-must-refuse decision and, for - # maskable ones, the output-column index. Latent in THIS slice (the caller still refuses on any - # sensitive projection); a later masking slice reads it. Deterministic order: arm order, then - # projection index; duplicates and multi-arm collisions on `output_index` are preserved (unlike - # `columns`, which is the de-duplicated sorted set the refuse message is built from). + # maskable ones, the output-column index. LOAD-BEARING, not informational — `_model_safety` + # reads `maskable` / `output_index` off this list to build `MaskPlan.indices`, i.e. the exact + # output columns that get redacted, so order and duplicates are significant and must not be + # re-sorted or de-duplicated. Deterministic order: arm order, then projection index; duplicates + # and multi-arm collisions on `output_index` are preserved (unlike `columns`, which is the + # de-duplicated sorted set the refuse message is built from). projections: list["SensitiveProjection"] = field(default_factory=list) + # Whether the WHOLE output was resolvable, not just whether each FOUND projection was + # maskable (ACE-041 review). `projections` can only describe what the detector saw; this + # says whether it could have seen everything. False when an output-bearing arm draws through + # a source whose body is not walked (a derived table, a CTE the arm selects from, or a + # non-table source such as VALUES / LATERAL), or projects a scalar subquery, or projects a + # column whose qualifier names none of that arm's own sources — i.e. exactly where the + # name-based match is blind. See `_output_lineage_provable` for why a bare column needs no + # such test. Masking a partially-understood output is worse than refusing it: the caller gets + # a `***` that certifies the other columns. + output_provable: bool = False def as_dict(self) -> dict[str, Any]: return { @@ -470,6 +483,7 @@ def as_dict(self) -> dict[str, Any]: "reason": self.reason, "suggestion": self.suggestion, "projections": [p.as_dict() for p in self.projections], + "output_provable": self.output_provable, } @@ -521,6 +535,108 @@ def _direct_from_tables(tree: "exp.Select") -> set[str]: return names +def _arm_sources(sel: "exp.Select") -> list["exp.Expression"]: + """The FROM / JOIN clauses belonging to THIS select, never a nested one. + + Read off `sel`'s own args by NODE TYPE rather than by arg key: sqlglot renamed the FROM key + (`from` -> `from_`) inside the `sqlglot>=20` range this package accepts, and a missing key + would silently degrade to "no sources", which reads as provable. Matching on `exp.From` / + `exp.Join` is stable across that range.""" + out: list["exp.Expression"] = [] + for value in sel.args.values(): + for item in value if isinstance(value, list) else [value]: + if isinstance(item, (exp.From, exp.Join)): + out.append(item) + return out + + +def _direct_scope(sel: "exp.Select") -> dict[str, str]: + """alias (or table name) -> table, for THIS arm's own FROM/JOIN sources only. + + Used by the whole-output proof to decide whether a projected qualifier names one of the + sources the arm actually draws from. Deliberately narrower than `_sensitive_scope`.""" + scope: dict[str, str] = {} + for src in _arm_sources(sel): + node = src.this + if isinstance(node, exp.Table): + scope[node.alias_or_name] = node.name + return scope + + +def _sensitive_scope(sel: "exp.Select") -> dict[str, Optional[str]]: + """alias -> table for the sensitive gate, with a SHADOWED alias resolved to None. + + Same whole-subtree walk as `_tables_in_scope`, so a bare column still resolves through a + derived-table or CTE body exactly as it did before. The one difference is what happens when a + single alias is bound to two different tables — `FROM customers t` outside and `FROM orders t` + inside an EXISTS body: + + SELECT c.ssn, t.ssn FROM customers c, customers t WHERE EXISTS (SELECT 1 FROM orders t) + + A last-one-wins map silently rebinds the OUTER `t.ssn` to `orders`, which declares no `ssn`, + so the column reads as un-sensitive and is never detected — while the qualifier still + "resolves", so the output reads as provable and the query masks one column and hands back its + twin raw. A subquery alias is scoped to its own body and must not capture an outer qualifier. + Mapping the conflict to None makes the binding UNRESOLVED, which `_sensitive_col_ref` already + handles by falling back to the conservative folded bare name (offending, and maskable).""" + bound: dict[str, set[str]] = {} + for tbl in sel.find_all(exp.Table): + bound.setdefault(tbl.alias_or_name, set()).add(tbl.name) + return {alias: (next(iter(t)) if len(t) == 1 else None) for alias, t in bound.items()} + + +def _output_lineage_provable(tree: "exp.Expression") -> bool: + """Could the sensitive-projection detector have seen EVERY value reaching the output? + + The detector matches sensitive columns by name against the tables in scope. That is sound + only while every output value traces back to a declared table it can name. Two things break + the trace: + + * an OUTPUT-BEARING source whose body is not walked — a derived table, a CTE the arm + selects from, or a non-table source (VALUES / LATERAL / table function). `(SELECT ssn + FROM customers) t` presents an opaque `t` whose columns cannot be attributed, and a + rename inside makes even the name useless; + * a qualifier that names nothing among the arm's own sources, so the column cannot be + bound to a table at all. + + Both tests are deliberately scoped to what can actually PUT A VALUE IN THE OUTPUT: an arm's + own FROM/JOIN sources, and its projection list. A subquery in WHERE / HAVING / ORDER BY, or a + JOIN's ON condition, yields a filter rather than a column, and a CTE no arm selects from + contributes nothing — withholding the proof for those refuses ordinary analytics SQL and buys + no safety. (This mirrors the distinction `check_column_scope` already draws.) + + A BARE column needs no test here. The sensitive match is by NAME against the whole model, so + it does not depend on which table the column binds to: either the name is declared sensitive + somewhere and `_sensitive_col_ref` flags it (falling back to the folded bare name when the + table is ambiguous), or it is declared sensitive nowhere and no binding could make it + sensitive. Refusing on ambiguity alone would reject every unqualified column in a join. + + Returning False here does NOT refuse the query. It only withholds the *proof* that the output + is fully understood, which downgrades a mask decision to a refusal. This runs only after an + offending projection has already been found, so a query that projects nothing sensitive never + reaches it — see the caller.""" + cte_names = {cte.alias_or_name.lower() for cte in tree.find_all(exp.CTE)} + for sel in _output_selects(tree): + for src in _arm_sources(sel): + node = src.this + if not isinstance(node, exp.Table): + return False # derived table / VALUES / LATERAL / table function: body not walked + if node.name.lower() in cte_names: + return False # a CTE body is not walked either, and a rename inside hides the name + scope = _direct_scope(sel) + # Case-fold the scope keys: SQL identifiers are case-insensitive unless quoted, so + # `FROM customers C` + `c.ssn` is one table, and a case-sensitive miss would silently + # read as "not a sensitive column" rather than "could not resolve". + lowered = {alias.lower() for alias in scope} + for proj in sel.expressions: + if proj.find(exp.Subquery) is not None: + return False # a scalar subquery in the PROJECTION does reach the output + for col in proj.find_all(exp.Column): + if col.table and col.table.lower() not in lowered: + return False # qualifier names nothing among this arm's own sources + return True + + def _output_selects(node: "exp.Expression") -> list["exp.Select"]: """The SELECTs whose projection reaches the query OUTPUT: the top-level SELECT, or — for a set operation (UNION / INTERSECT / EXCEPT) — every arm. @@ -553,7 +669,7 @@ def _output_selects(node: "exp.Expression") -> list["exp.Select"]: def _sensitive_col_ref( col: "exp.Column", - scope: dict[str, str], + scope: Mapping[str, Optional[str]], by_table: dict[str, dict[str, str]], allnames: set[str], ) -> Optional[str]: @@ -579,7 +695,7 @@ def _sensitive_col_ref( def _classify_projection( proj: "exp.Expression", index: int, - scope: dict[str, str], + scope: Mapping[str, Optional[str]], direct: set[str], by_table: dict[str, dict[str, str]], allnames: set[str], @@ -664,7 +780,10 @@ def check_sensitive_projection( offending: set[str] = set() projections: list[SensitiveProjection] = [] for sel in _output_selects(tree): - scope = _tables_in_scope(sel) + # `_sensitive_scope`, NOT `_tables_in_scope`: a last-one-wins alias map lets an alias + # declared inside a subquery overwrite the outer alias of the same name, which binds an + # outer qualified column to the wrong table and hides it from the match entirely. + scope = _sensitive_scope(sel) direct = _direct_from_tables(sel) for index, proj in enumerate(sel.expressions): _classify_projection( @@ -683,6 +802,7 @@ def check_sensitive_projection( suggestion="Aggregate it (e.g. COUNT(DISTINCT )) for a count, or omit it and " "select the entity's non-sensitive key (e.g. id) instead.", projections=projections, + output_provable=_output_lineage_provable(tree), ) @@ -1689,9 +1809,32 @@ def collect(node): return referenced -def _resolve_col_table(col: "exp.Column", scope: dict[str, str]) -> Optional[str]: +def _resolve_col_table(col: "exp.Column", scope: Mapping[str, Optional[str]]) -> Optional[str]: + """The table `col` binds to, or None when that cannot be decided. + + `scope` values are Optional because the sensitive gate passes `_sensitive_scope`, which maps a + SHADOWED alias (one bound to two different tables) to None on purpose. `Mapping` rather than + `dict` so the fan/chasm and aggregation callers can keep passing a plain `dict[str, str]` — a + Mapping's value type is covariant, a dict's is not.""" if col.table: - return scope.get(col.table, col.table) + # SQL identifiers are case-insensitive unless quoted, so `FROM customers C` and `c.ssn` + # name the same table. A case-sensitive miss does not read as "unresolved" downstream — + # it falls through to the raw qualifier, which the sensitive gate then fails to match + # against its table index and treats as NOT sensitive. ACE-041 slice 3 (`4bd39bb`) folded + # the COLUMN name for exactly this reason; the alias lookup is the other half of that fix. + # + # Resolve by FOLDED match rather than exact-match-first. When two aliases in scope fold + # together — `FROM x "AB", customers ab` with `AB.ssn` — an exact hit on the quoted `"AB"` + # would bind to `x`, while the engine folds the unquoted `AB` onto `ab` and reads + # `customers`. The binding is genuinely ambiguous, so return None (unresolved) and let the + # caller fail closed; picking either one is a coin flip that can read as un-sensitive. + folded = col.table.lower() + matches = {name for alias, name in scope.items() if alias.lower() == folded} + if len(matches) == 1: + return next(iter(matches)) + if matches: + return None # ambiguous binding -> unresolved, never a guess + return col.table # unqualified column: ambiguous; only safe to attribute if single table if len(scope) == 1: return next(iter(scope.values())) diff --git a/plugins/agami/lib/execute_sql.py b/plugins/agami/lib/execute_sql.py index 6f0e2285..04fa54b9 100644 --- a/plugins/agami/lib/execute_sql.py +++ b/plugins/agami/lib/execute_sql.py @@ -1296,14 +1296,27 @@ def _model_safety( mask_plan: MaskPlan | None = None sens = RT.check_sensitive_projection(sql, org, ctx=ctx) if sens.action == "refuse": + # Two conditions, and BOTH are required. `all_maskable` quantifies over the projections + # the detector FOUND, so on its own it proves only "every offending projection we saw is + # maskable" — never "no sensitive value reaches the output". Under the earlier refuse-only + # behaviour that gap leaked only when nothing was detected (anything detected blocked the + # whole query); once a maskable projection let the query RUN, one seen column unblocked the + # entire result set and every co-projected value the name-based detector missed came back + # raw — beside a `***` implying the row had been protected. `output_provable` is the + # missing half: the detector could see the whole output, not just part of it. all_maskable = bool(sens.projections) and all( p.maskable and p.output_index is not None for p in sens.projections ) + # `getattr`, not attribute access: this file is VENDORED into the plugin while + # semantic_model/runtime.py resolves from the separately-versioned installed package, so a + # newer plugin can meet an older `SensitiveCheckResult`. A missing field then means "this + # build cannot prove the output", which is exactly the fail-closed answer. + provable = all_maskable and getattr(sens, "output_provable", False) verdict = Verdict( cls="data_protection", rule="sensitive_projection", severity="high", - certainty="provable" if all_maskable else "uncertain", + certainty="provable" if provable else "uncertain", detail=sens.reason, remediation=sens.suggestion or "", ) diff --git a/tests/e2e/harness.py b/tests/e2e/harness.py index d58d8d8b..7a5c11c8 100644 --- a/tests/e2e/harness.py +++ b/tests/e2e/harness.py @@ -120,7 +120,7 @@ def seed_db_model(url: str, ds: str = "acme") -> None: s = Store.connect(url) s.run_migrations() - model_store.write_organization(s, ds, build_org()) + model_store.write_datasource(s, ds, build_org()) s.close() diff --git a/tests/test_ace041_masking.py b/tests/test_ace041_masking.py index dc569cfb..1e34c9a4 100644 --- a/tests/test_ace041_masking.py +++ b/tests/test_ace041_masking.py @@ -456,11 +456,121 @@ def test_mask_note_helper_is_accurate_and_deterministic(): assert execute_sql._mask_note(("customers.ssn",), (0, 9), ["ssn"]) == ("customers.ssn",) +# --------------------------------------------------------------------------- +# CO-PROJECTION FAIL-CLOSED — the mask verdict must be a WHOLE-OUTPUT proof. +# +# `all_maskable` quantifies over the projections the DETECTOR FOUND, which proves "every +# offending projection we saw is maskable" — not "no sensitive value reaches the output". +# Under the old refuse-only behaviour a detector miss leaked only when NOTHING was detected, +# because anything detected blocked the whole query. Once a detected-and-maskable projection +# started letting the query RUN, one seen column began unblocking the entire result set — so +# every co-projected value the name-based detector missed came back raw, next to a `***` that +# makes it look like masking worked. +# +# These are the co-projection forms of the alias-lineage gap below. The standalone forms are +# genuinely pre-existing; these are NOT — each returned a raw SSN only after the refuse->mask +# flip. Refusing is the correct outcome: when the output cannot be proven clean, fail closed. +# --------------------------------------------------------------------------- + +_CO_PROJECTION_LEAK = [ + # a detected `ssn` beside the same column laundered through a derived table + "SELECT ssn, t.ssn FROM customers, (SELECT ssn FROM customers) t", + # ...through a CTE with a rename + "WITH q AS (SELECT ssn AS z FROM customers) SELECT c.ssn, q.z FROM customers c, q", +] + + +@pytest.mark.parametrize("sql", _CO_PROJECTION_LEAK) +def test_co_projected_unprovable_output_is_refused_not_partially_masked(guarded, sql): + """A raw sensitive value must never ride along beside a masked one. + + Each of these ran and returned `('***', '111-22-3333')` before the whole-output proof: + one column redacted, its twin in the clear. + """ + spy = _SpyExecutor(_R(["ssn", "ssn_2"], [("111-22-3333", "111-22-3333")])) + with pytest.raises(execute_sql.GuardRefused) as ei: + guarded(sql, spy) + assert ei.value.refusal.kind == "sensitive_columns" + assert not spy.calls, "an unprovable output must be refused BEFORE the executor runs" + + +@pytest.mark.parametrize( + "sql,columns,row", + [ + ("SELECT ssn FROM customers", ["ssn"], ("111-22-3333",)), + ("SELECT c.ssn FROM customers c", ["ssn"], ("111-22-3333",)), + ('SELECT "SSN" FROM customers', ["SSN"], ("111-22-3333",)), + ("SELECT name, ssn FROM customers", ["name", "ssn"], ("Alice", "111-22-3333")), + ("SELECT ssn FROM customers UNION ALL SELECT ssn FROM customers", ["ssn"], + ("111-22-3333",)), + # Case-mismatched alias: `FROM customers C` + `c.ssn`. ACE-041 slice 3 (`4bd39bb`) folded + # the COLUMN name; the table ALIAS lookup was the other half. Once it resolves, this is an + # ordinary provable projection, so it masks rather than refusing — which restores the + # intended ACE-041 outcome. Before the alias fold, `SELECT C.ssn FROM customers c` + # returned the raw value with no mask at all. + ("SELECT ssn, c.ssn FROM customers C", ["ssn", "ssn_2"], + ("111-22-3333", "111-22-3333")), + ("SELECT C.ssn FROM customers c", ["ssn"], ("111-22-3333",)), + ], +) +def test_provable_output_still_masks(guarded, sql, columns, row): + """The whole-output proof must not collapse ACE-041 back into refuse-everything. + + Every projection here resolves to a declared table, so the output IS provable and the + feature works as specified: the query runs and the sensitive column is redacted. + """ + # The fixture mirrors each query's real projection shape, with the raw secret ONLY in cells the + # engine would fill with a sensitive value. That is what makes "no raw value survives" + # assertable — a fixture that puts the secret in every cell can only ever check that a token + # appeared SOMEWHERE, which passes even when the plan masks the wrong column. + spy = _SpyExecutor(_R(columns, [tuple(row)])) + result = guarded(sql, spy) + assert spy.calls, f"a provable maskable projection must still RUN: {sql!r}" + assert result.masked_columns, f"a provable projection must be MASKED, not passed through: {sql!r}" + flat = [v for r in result.rows for v in r] + assert execute_sql.REDACTION_TOKEN in flat, f"must be masked: {sql!r}" + assert "111-22-3333" not in flat, f"raw value survived: {result.rows!r} for {sql!r}" + assert execute_sql.REDACTION_TOKEN in [v for row in result.rows for v in row] + + +@pytest.mark.parametrize( + "sql", + [ + "SELECT COUNT(ssn) FROM customers", + "SELECT COUNT(*) FROM customers WHERE ssn IS NOT NULL", + "SELECT COUNT(*) FROM customers c, (SELECT id FROM orders) o WHERE c.ssn IS NOT NULL", + ], +) +def test_aggregate_or_filter_use_returns_before_the_proof(guarded, sql): + """Aggregate/filter use of a sensitive column stays allowed, derived table or not. + + Note what this does and does NOT cover. Every case here ends at `action="allow"`, so it + returns BEFORE the whole-output proof is ever consulted — it pins the early-return path, not + the proof. The over-refusal direction (a query that DOES project a sensitive column and also + contains unrelated nesting) is pinned separately by + `test_unrelated_nesting_still_masks`, which is the test that would catch the proof + being widened back out. + """ + spy = _SpyExecutor(_R(["n"], [(3,)])) + result = guarded(sql, spy) + assert spy.calls, f"aggregate/filter use must not be refused: {sql!r}" + assert result.rows == [(3,)] + + # --------------------------------------------------------------------------- # KNOWN LIMITATION (ACE-041 review, finding 2): a sensitive column RENAMED inside a derived-table / # CTE body escapes both mask and refuse, because `_output_selects` excludes subquery/CTE bodies and # the sensitive match is name-based with no alias/lineage tracking. This is PRE-EXISTING (not -# introduced by ACE-041). These xfail tests assert the DESIRED secure behaviour (the outer projection +# introduced by ACE-041) in its STANDALONE form — nothing sensitive is detected at all, so the +# whole-output proof above never engages. +# +# Its CO-PROJECTION form WAS new, and is closed above — but closed by REFUSAL, not by detection. +# The detector still never sees the laundered column (it reports only the plainly-projected +# `customers.ssn`); what stops the leak is that an arm drawing on a derived table or a CTE cannot +# be proven, so the statement is refused. That coupling matters: narrowing the opaque-source test +# in `_output_lineage_provable` would silently re-open these leaks while every test here still +# passes. `test_opaque_output_source_is_still_refused` is what pins it. +# These xfail tests assert the DESIRED secure behaviour (the outer projection # is masked OR refused); they xfail today and will flip to xpass when a future alias-lineage fix # lands — that flip is the tracking signal. Do NOT "fix" them by weakening the assertion. See the # `# ACE-041 known limitation` comment in semantic_model/runtime.py::_output_selects. @@ -634,3 +744,149 @@ def test_real_subprocess_fork_masks_ssn_in_the_emitted_csv(tmp_path): masked = [json.loads(x)["masked"] for x in proc.stderr.splitlines() if x.strip().startswith("{") and '"masked"' in x] assert masked == [["customers.ssn"]] + + +# --------------------------------------------------------------------------- +# WHOLE-OUTPUT PROOF — adversarial matrix (PR #155 review). +# +# The proof answers one question: could the detector have seen EVERY value reaching the output? +# Two failure directions, and both must be pinned, because a fix for either one silently breaks +# the other: +# +# * UNSOUND — `output_provable` True while a sensitive value reaches the output unseen. That is +# the `('***', raw)` receipt this whole feature exists to prevent, so it is the direction that +# must never regress. Caused by the detector RESOLVING A QUALIFIER TO THE WRONG TABLE. +# * OVER-BROAD — the proof withholds itself for a construct that cannot contribute an output +# value at all (a WHERE/HAVING subquery, a CTE nothing selects from), refusing an ordinary +# query that ACE-041 is supposed to mask. +# +# The fixtures below put the raw secret ONLY in cells the engine would really fill with a sensitive +# value, and a distinct sentinel elsewhere, so "no raw value survives" is directly assertable +# instead of being inferred from the presence of a token somewhere in the row. +# --------------------------------------------------------------------------- + +_SECRET = "111-22-3333" +_SAFE = "not-sensitive" + + +def _assert_no_raw_value(guarded, sql, columns, row): + """Run `sql` and assert the raw sensitive value never reaches the caller. + + Either outcome is secure: refusing the statement, or running it with every sensitive cell + redacted. What is NOT acceptable is a raw secret in the result — with or without a `***` + beside it, which is the partial-mask receipt that reads as "this row was protected". + """ + spy = _SpyExecutor(_R(columns, [tuple(row)])) + try: + result = guarded(sql, spy) + except execute_sql.GuardRefused: + return # fail-closed is a secure outcome + flat = [v for r in result.rows for v in r] + assert _SECRET not in flat, f"raw sensitive value survived: {result.rows!r} for {sql!r}" + + +# An alias declared in a subquery must not capture a qualifier belonging to the OUTER query. The +# detector resolves `t.ssn` through its alias->table map; if the map is built over the whole tree, +# the inner `FROM orders t` overwrites the outer `t -> customers` and the outer `t.ssn` is judged a +# column of `orders` (which declares no `ssn`), so it is never detected. Every qualifier still +# "resolves", so the proof reports provable and the query masks ONE column and returns the other raw. +_ALIAS_SHADOWING_LEAK = [ + # `EXISTS (...)` parses to exp.Exists wrapping a bare Select — NO exp.Subquery node — so a + # derived-table check keyed on that node type never fires here. + ("SELECT c.ssn, t.ssn FROM customers c, customers t WHERE EXISTS (SELECT 1 FROM orders t)", + ["ssn", "ssn_2"], (_SECRET, _SECRET)), + ("SELECT c.ssn, t.ssn FROM customers c, customers t WHERE NOT EXISTS (SELECT 1 FROM x t)", + ["ssn", "ssn_2"], (_SECRET, _SECRET)), + # Same root cause with no co-projection to unblock the query: the single projection resolves to + # the wrong table, nothing is detected at all, and the raw value flows straight through. + ("SELECT t.ssn FROM customers t WHERE EXISTS (SELECT 1 FROM orders t)", + ["ssn"], (_SECRET,)), + # A JOIN body, not a WHERE body — same shadowing, different clause. + ("SELECT c.ssn, t.ssn FROM customers c JOIN customers t ON t.id = c.id " + "WHERE EXISTS (SELECT 1 FROM x t)", ["ssn", "ssn_2"], (_SECRET, _SECRET)), +] + + +@pytest.mark.parametrize("sql,columns,row", _ALIAS_SHADOWING_LEAK) +def test_subquery_alias_must_not_shadow_an_outer_qualifier(guarded, sql, columns, row): + _assert_no_raw_value(guarded, sql, columns, row) + + +# A quoted alias must not capture an unquoted qualifier that the ENGINE folds onto a different +# table. `FROM x "AB", customers ab` + `AB.ssn`: Postgres folds the unquoted `AB` to `ab` and reads +# `customers`, but an exact-match-first lookup binds it to the quoted `"AB"` -> `x`, whose `ssn` is +# not declared sensitive. When two aliases in scope fold together the binding is genuinely +# ambiguous, and an ambiguous binding must fail closed, not pick one. +_QUOTED_ALIAS_COLLISION = [ + ('SELECT c.ssn, AB.ssn FROM customers c, x "AB", customers ab', + ["ssn", "ssn_2"], (_SECRET, _SECRET)), + ('SELECT AB.ssn FROM x "AB", customers ab', ["ssn"], (_SECRET,)), +] + + +@pytest.mark.parametrize("sql,columns,row", _QUOTED_ALIAS_COLLISION) +def test_case_colliding_aliases_fail_closed(guarded, sql, columns, row): + _assert_no_raw_value(guarded, sql, columns, row) + + +# The proof must engage ONLY for constructs that can actually put a value in the output. A subquery +# in WHERE/HAVING/ORDER BY yields a filter, not a column; a CTE nothing selects from contributes +# nothing at all. Refusing these buys no safety and rolls back the feature for ordinary analytics +# SQL, which is what LLM-generated queries look like. +_MASKABLE_DESPITE_UNRELATED_NESTING = [ + # WHERE subquery — the archetypal false refusal. + ("SELECT ssn FROM customers WHERE id IN (SELECT cust_id FROM orders)", ["ssn"], (_SECRET,)), + ("SELECT c.ssn FROM customers c WHERE c.id IN (SELECT cust_id FROM orders)", ["ssn"], (_SECRET,)), + ("SELECT ssn FROM customers c WHERE EXISTS (SELECT 1 FROM orders o WHERE o.cust_id = c.id)", + ["ssn"], (_SECRET,)), + # HAVING / ORDER BY subqueries. + ("SELECT ssn FROM customers GROUP BY ssn HAVING COUNT(*) > (SELECT COUNT(*) FROM orders)", + ["ssn"], (_SECRET,)), + ("SELECT ssn FROM customers ORDER BY (SELECT COUNT(*) FROM orders)", ["ssn"], (_SECRET,)), + # A CTE that no output arm selects from cannot launder anything. + ("WITH unused AS (SELECT id FROM orders) SELECT ssn FROM customers", ["ssn"], (_SECRET,)), + # An ordinary two-table join with an unqualified projection. A bare column is matched by NAME + # against the sensitive set, which does not depend on which table it binds to, so ambiguity here + # cannot hide a sensitive value — `_sensitive_col_ref` already falls back to the folded bare name. + ("SELECT name, ssn FROM customers JOIN orders ON customers.id = orders.cust_id", + ["name", "ssn"], (_SAFE, _SECRET)), + ("SELECT ssn FROM customers a, customers b", ["ssn"], (_SECRET,)), +] + + +@pytest.mark.parametrize("sql,columns,row", _MASKABLE_DESPITE_UNRELATED_NESTING) +def test_unrelated_nesting_still_masks(guarded, sql, columns, row): + """Nesting that cannot reach the output must not downgrade a maskable projection to a refusal.""" + spy = _SpyExecutor(_R(columns, [tuple(row)])) + result = guarded(sql, spy) + assert spy.calls, f"must RUN, not refuse: {sql!r}" + flat = [v for r in result.rows for v in r] + assert execute_sql.REDACTION_TOKEN in flat, f"must be masked: {sql!r}" + assert _SECRET not in flat, f"raw value survived: {result.rows!r} for {sql!r}" + + +# The narrowing above must not re-open what the whole-output proof closed. Each of these puts an +# output-bearing arm behind a body the detector does not walk, so the statement must still refuse. +_OUTPUT_BEARING_OPAQUE_SOURCE = [ + # derived table in FROM + "SELECT ssn, t.ssn FROM customers, (SELECT ssn FROM customers) t", + # CTE actually selected from by the output arm + "WITH q AS (SELECT ssn AS z FROM customers) SELECT c.ssn, q.z FROM customers c, q", + # a set-operation arm whose source is opaque, the other arm clean + "SELECT ssn FROM customers UNION ALL SELECT z FROM (SELECT ssn AS z FROM customers) q", + "WITH q AS (SELECT ssn AS z FROM customers) SELECT ssn FROM customers UNION ALL SELECT z FROM q", + # nested two deep (no `*` — the star ban would refuse that one first, for a different reason) + "SELECT ssn, t.z FROM customers, (SELECT z FROM (SELECT ssn AS z FROM customers) i) t", + # a scalar subquery in the PROJECTION list can contribute an output value, and a rename inside + # it defeats the name-based match, so it stays opaque even though it is not a FROM source + "SELECT ssn, (SELECT z FROM (SELECT ssn AS z FROM customers) i LIMIT 1) AS leaked FROM customers", +] + + +@pytest.mark.parametrize("sql", _OUTPUT_BEARING_OPAQUE_SOURCE) +def test_opaque_output_source_is_still_refused(guarded, sql): + spy = _SpyExecutor(_R(["a", "b"], [(_SECRET, _SECRET)])) + with pytest.raises(execute_sql.GuardRefused) as ei: + guarded(sql, spy) + assert ei.value.refusal.kind == "sensitive_columns" + assert not spy.calls, "an unprovable output must be refused BEFORE the executor runs"