From 4ca6ee4b06689009fcf163e59f53c9c3a66131ab Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 27 Aug 2026 05:55:54 -0500 Subject: [PATCH] feat(security): assert the leak-gate floor still tracks the real token list (BACKLOG #1368, SEC-04) `MEFOR_MIN_DETECTORS` is a per-section FLOOR and `token_floor_failure` fails when the token list drops below it. NOTHING ASKED THE OPPOSITE QUESTION. A floor of 7 against a list of 40 is satisfied by any 7 detectors surviving, so the gate stays green while constraining almost nothing. The counts have been printed on every run all along, and nothing has ever compared them to the floor -- which is why the drift is silent rather than merely unfixed. Measured against the live list (counts only; the list is a secret and appears nowhere): section floor live floor/live names 7 8 87.5% estate 13 14 92.9% site_prefixes 1 2 50.0% <- would FAIL a flat 80% rule THE SPEC'S 80% RATIO IS THE WRONG SHAPE AT SMALL N, AND BUILDING IT IS HOW THAT SURFACED. Growth from 1 to 2 scores 50% however healthy it is, so at that size a ratio measures the SECTION'S SIZE rather than the floor's staleness -- the rule as specified would have redded the gate on its first real run with nothing wrong. Shipped instead: a ratio above four loaded detectors, an absolute lag of at most one at or below four. Both arms fire on their own section and name both numbers. The deviation from the master test plan is routed to the owner via the Liaison rather than shipped quietly, and nothing here raises any floor -- the live list is within the rule today, and if it later drifts the gate says so and a human decides. THE COUNTS CANNOT TELL YOU WHICH TABLE YOU MEASURED. The SYNTHETIC example set and the REAL list both report 8/14/2 -- three identical numbers from two tables, one of which matches nothing real. Anyone verifying a floor from counts alone would assert it against a set that cannot fire and read the pass as evidence. I made exactly that error an hour before finding it, assuming my own local run was synthetic; the scanner's mode marker is what settled it. So `--print-detector-counts` emits `mode=` FIRST, and a test pins that the count lines are byte-identical between synthetic and real -- if they ever differ, that test has stopped demonstrating the hazard. THE NEW CHECK IS GUARDED ON THE FLOOR VARIABLE, NOT ON THE SECRET. Only the secret-present branch exports `MEFOR_MIN_DETECTORS`, so fork PRs and structural-only runs skip it entirely. It must not run against the synthetic set: that set populates every section and would fail the freshness rule on a contributor's machine, and a gate that fires on everyone gets switched off by the third person who hits it. For the same reason the check is NOT folded into `token_floor_failure` beside the existing floor test, which is where the floor and the counts already meet -- I tried that first and rejected it. A COMMENT THIS CHANGE FALSIFIES IS CORRECTED IN THE SAME COMMIT. security.yml promised "a FLOOR: adding tokens needs no CI change, losing them fails the build". After this, growth that outruns the floor DOES ask for one line. That is the trade SEC-04 requests, and leaving the old sentence standing would be a comment contradicting the code beside it. Verified: ruff check + format clean; 50 tests passing, 38 definitions, no duplicate names; 9 new tests confirmed present BY NAME. The freshness assertion was exercised end to end against the real floors (exit 0) and shown to FAIL on three separate drift shapes -- names 2/8 by ratio, estate 6/14 by ratio, site_prefixes 0-against-2 by absolute lag -- each naming its own section. security.yml still parses as YAML and the step is wired into the forbidden-content job. NO TOKEN CONTENT ANYWHERE, in the code, the tests, the ledger row or this message. Every figure is a count or a mode marker, both of which the gate already prints to CI logs. Co-Authored-By: Claude Opus 5 --- .github/workflows/security.yml | 22 +++++- docs/BACKLOG.md | 29 ++++++++ scripts/security/scan_forbidden.py | 107 +++++++++++++++++++++++++++++ tests/test_scan_forbidden.py | 104 ++++++++++++++++++++++++++++ 4 files changed, 261 insertions(+), 1 deletion(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 47865d71f..0f29bb243 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -663,7 +663,15 @@ jobs: # passed with a green tick, and losing just the final line silently disabled every site-code # detector. The floor is PER-SECTION, not a bare total -- a total is a SUM, so growth in a cheap # section masks collapse in an expensive one (names 7->1 alongside estate 13->19 still totals 21). - # It is a FLOOR: adding tokens needs no CI change, losing them fails the build. + # It is a FLOOR: losing tokens fails the build. + # + # ADDING them USED TO need no CI change, and that is no longer quite true (BACKLOG #1368, + # SEC-04). A floor only constrains the list while it stays near it: 7 against a list of 40 is + # satisfied by any 7 detectors surviving, so the gate goes green while guarding almost nothing, + # and NOTHING NOTICED because the counts print and nothing compared them. The step below now + # fails when the floor falls behind, so growth is free until it outruns the floor and then asks + # for one line. That is a deliberate trade of convenience for a floor that keeps meaning + # something. # # zizmor: the secret is never interpolated into this run: body. It arrives as the step-level env # var (an opaque value, not an Actions-expression sink). @@ -689,3 +697,15 @@ jobs: exit 2 fi python scripts/security/scan_forbidden.py --path . + + # SEC-04 / BACKLOG #1368 -- is the FLOOR still tracking the list? The scan above asks whether + # the LIST fell below the FLOOR; this asks the opposite, which nothing did before. + # + # GUARDED ON THE FLOOR VARIABLE, NOT ON THE SECRET, and only the secret-present branch above + # exports it -- so a fork PR and a structural-only run skip this entirely. It must NOT run + # against the synthetic example set: that set populates every section and would fail the + # freshness rule on a contributor's machine, and a gate that fires on everyone gets switched + # off by the third person who hits it. + if [ -n "$MEFOR_MIN_DETECTORS" ]; then + python scripts/security/scan_forbidden.py --assert-floor-fresh + fi diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 9d4e3f3d0..1a39cab73 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -16713,3 +16713,32 @@ raised message via `ast` and reds on that revert. confusing rather than broken, and churning that many message strings in one change buys less than the guard that stops the next one. Anyone taking them should take the `[egress]` six as one job -- they share a shape. +## 1368. the leak-gate detector floor is never checked against the real token list, so it can silently stop meaning anything + +> 🔢 **Filed 2026-08-26 (builder 2) - BUILT IN THIS COMMIT, not yet landed.** Implements **SEC-04** from [`16-security-phi-and-supply-chain`](testing/master-test-plan/16-security-phi-and-supply-chain.md), which had no ledger row. +> Verdict: build +> Closing-act: code + +**Cluster:** Security testing. **Priority:** P0 (per the spec row). **Verdict:** build. +**Severity:** no engine effect and no deployment axis (sec. 0). The cost is that the leak gate's fail-closed floor can drift into meaninglessness with nothing reporting it -- a control resting on a number nobody re-checks. + +**What:** `MEFOR_MIN_DETECTORS` is a per-section FLOOR, and `token_floor_failure` fails when the token list drops below it. **Nothing asked the opposite question.** A floor of 7 against a list of 40 is satisfied by any 7 detectors surviving, so the gate stays green while constraining almost nothing. The counts are printed on every run and **nothing has ever compared them to the floor**, which is why the drift is silent rather than merely unfixed. + +**Measured against the live list, 2026-08-26** (counts only -- the list is a secret and appears nowhere): + +``` +section floor live floor/live +names 7 8 87.5% +estate 13 14 92.9% +site_prefixes 1 2 50.0% <- would FAIL a flat 80% rule +``` + +**THE SPEC'S 80% RATIO IS THE WRONG SHAPE AT SMALL N, and the item found that by building it.** Growth from 1 to 2 scores 50% however healthy it is, so at that size a ratio measures the SECTION'S SIZE rather than the floor's staleness, and the rule as written would red the gate on its first real run while nothing was wrong. Shipped instead: **a ratio above four loaded detectors, an absolute lag of at most one at or below four.** Both arms fire on their own section and name both numbers. Deviation from the spec routed to the owner via the Liaison rather than shipped quietly. + +**AND THE COUNTS CANNOT TELL YOU WHICH TABLE YOU MEASURED.** On 2026-08-26 the SYNTHETIC example set and the REAL list both reported **8/14/2** -- three identical numbers from two tables, one of which matches nothing real. Anyone verifying a floor from counts alone would assert it against a set that cannot fire and read the pass as evidence. `scan_forbidden` already distinguishes three load states, and only the MODE marker discriminates them, so `--print-detector-counts` emits `mode=` first and a test pins that the count lines are byte-identical between the two. + +**Not in scope:** raising any floor. The live list is within the shipped rule today, so no threshold moves in this change; if it later drifts, the gate says so and a human decides. `branch-leak-scan.yml` pins the same floor and is NOT wired here -- same rule, different job, and it deserves its own decision rather than being swept in. + +**Related:** #1080 (the setup script reporting a loaded set as if it were the installed one -- the same family of "the gate reported something adjacent to what you asked"), SEC-05. + +**Source:** SEC-04 in the master test plan, routed by the Liaison; scoped down by this lane after measuring that half the brief's build already existed. diff --git a/scripts/security/scan_forbidden.py b/scripts/security/scan_forbidden.py index 9ee902189..e271dc79c 100644 --- a/scripts/security/scan_forbidden.py +++ b/scripts/security/scan_forbidden.py @@ -1026,8 +1026,115 @@ def _parse_require_flag(argv: list[str]) -> tuple[bool, int | dict[str, int] | N _TRUTHY = frozenset({"1", "true", "yes", "on"}) +#: A section small enough that a RATIO cannot express drift. Growth from 1 to 2 is 50% however +#: healthy it is, so at these sizes a ratio measures the section's SIZE rather than the floor's +#: staleness. Measured 2026-08-26 against the live list: site_prefixes floor 1 / loaded 2 scores 50% +#: and would fail an 80% rule on its first run while nothing is actually wrong. +_SMALL_SECTION_MAX = 4 +#: For a small section, the floor may lag the loaded count by at most this many detectors. One is +#: deliberate: it admits today's 1-against-2 and refuses 1-against-3. +_MAX_ABSOLUTE_LAG = 1 +#: For every other section, the floor must be at least this share of what actually loaded. +_MIN_FLOOR_RATIO = 0.8 + + +def floor_freshness_failure( + min_detectors: dict[str, int] | None, counts: dict[str, int] | None = None +) -> str | None: + """Has the FLOOR fallen behind the token list it is supposed to guard? (BACKLOG #1368, SEC-04) + + ``token_floor_failure`` answers the opposite question -- did the LIST fall below the FLOOR -- and + catches a token source that is truncated or misconfigured. It cannot see the other direction: a + list that grows while the floor stays put still passes, and the floor quietly stops meaning + anything. A floor of 7 against a list of 40 is satisfied by any 7 detectors surviving. + + TWO RULES, BECAUSE ONE SHAPE DOES NOT FIT BOTH SIZES. Above ``_SMALL_SECTION_MAX`` a ratio is the + honest measure. At or below it a ratio is dominated by the section's size -- see the constant -- + so the rule is an absolute lag instead. + + Returns None when the floor is still fresh, else a sentence naming the section and both numbers. + NEVER returns or logs token CONTENT: every value here is a count. + """ + if not min_detectors: + return None + counts = loaded_token_counts() if counts is None else counts + stale: list[str] = [] + for section, floor in sorted(min_detectors.items()): + loaded = counts.get(section, 0) + if loaded <= floor: + continue # at or below the floor is token_floor_failure's question, not this one + if loaded <= _SMALL_SECTION_MAX: + if loaded - floor > _MAX_ABSOLUTE_LAG: + stale.append(f"{section} floor {floor} lags {loaded} loaded by {loaded - floor}") + elif floor < _MIN_FLOOR_RATIO * loaded: + stale.append( + f"{section} floor {floor} is {floor / loaded:.0%} of {loaded} loaded " + f"(needs {_MIN_FLOOR_RATIO:.0%})" + ) + if not stale: + return None + return ( + f"detector floor has fallen behind the token list ({'; '.join(stale)}). The floor still " + "passes, which is the problem: it no longer constrains the list it guards. Raise " + "MEFOR_MIN_DETECTORS in .github/workflows/security.yml and branch-leak-scan.yml to match " + "what actually loads, in the same change that grew the list." + ) + + +def _detector_count_report(counts: dict[str, int]) -> list[str]: + """Machine-readable counts AND the load MODE, one ``key=value`` per line. + + THE MODE IS THE POINT AND THE COUNTS ARE NOT SUFFICIENT. The synthetic example set and the real + list can report the SAME three numbers -- measured 2026-08-26, both 8/14/2 -- so a caller checking + counts alone cannot tell which table it just measured, and would happily assert a floor against a + set that matches nothing real. `mode` is the only field that discriminates. + """ + if not TOKENS_PRESENT: + mode = "none" + elif is_synthetic_token_set(): + mode = "synthetic" + else: + mode = "real" + return [f"mode={mode}"] + [f"{k}={v}" for k, v in sorted(counts.items())] + + def main(argv: list[str]) -> int: reload_tokens() + # SEC-04 / BACKLOG #1368. Both are read BEFORE the require/floor parsing below, because each is a + # terminal mode that reports and exits rather than scanning. + if "--print-detector-counts" in argv: + for line in _detector_count_report(loaded_token_counts()): + print(line) + return 0 + if "--assert-floor-fresh" in argv: + spec = os.environ.get("MEFOR_MIN_DETECTORS", "").strip() + if not spec: + print( + "scan_forbidden: --assert-floor-fresh needs MEFOR_MIN_DETECTORS set; refusing rather " + "than passing vacuously.", + file=sys.stderr, + ) + return 2 + try: + parsed = parse_min_spec(spec) + except ValueError as exc: + print(f"scan_forbidden: MEFOR_MIN_DETECTORS: {exc}", file=sys.stderr) + return 2 + if not isinstance(parsed, dict): + print( + "scan_forbidden: --assert-floor-fresh needs a PER-SECTION floor " + "(names=N,estate=N,site_prefixes=N); a single total cannot say which section drifted.", + file=sys.stderr, + ) + return 2 + for line in _detector_count_report(loaded_token_counts()): + print(line) + why = floor_freshness_failure(parsed) + if why is not None: + print(f"scan_forbidden: {why}", file=sys.stderr) + return 1 + return 0 + show_context = "--show-context" in argv rest = [a for a in argv if a != "--show-context"] try: diff --git a/tests/test_scan_forbidden.py b/tests/test_scan_forbidden.py index e7b166da3..01cad2c79 100644 --- a/tests/test_scan_forbidden.py +++ b/tests/test_scan_forbidden.py @@ -516,3 +516,107 @@ def test_the_record_detector_reports_no_value(sf, tmp_path: Path) -> None: assert verdict not in h.split("security-record content")[0], ( f"reason leaked the verdict: {h!r}" ) + + +# -------------------------------------------------------------------------------------------------- +# BACKLOG #1368 / SEC-04 -- is the FLOOR still tracking the list it guards? +# +# `token_floor_failure` asks whether the LIST fell below the FLOOR. This asks the opposite: whether the +# floor fell behind the list. A floor of 7 against a list of 40 is satisfied by any 7 detectors +# surviving, so the gate goes green while constraining almost nothing -- and nothing anywhere noticed, +# because the counts print and nothing compares them. +# +# EVERY TEST HERE INJECTS COUNTS. No token source is loaded, so these run identically on a fork with no +# secret, and this file continues to carry no real token. +# -------------------------------------------------------------------------------------------------- + +#: The shape measured against the LIVE list on 2026-08-26, recorded so the tests below are anchored to a +#: real observation rather than to invented numbers. Counts only; the list itself is a secret. +_LIVE_2026_08_26 = {"names": 8, "estate": 14, "site_prefixes": 2} +_FLOOR_IN_CI = {"names": 7, "estate": 13, "site_prefixes": 1} + + +def test_the_floor_CI_pins_today_is_still_fresh_against_the_live_shape(sf) -> None: + """The rule must not fail on the day it lands. If this reds, the floor needs raising and that is a + decision for a human, not a test to be relaxed.""" + assert sf.floor_freshness_failure(_FLOOR_IN_CI, _LIVE_2026_08_26) is None + + +def test_THE_80_PERCENT_RATIO_ALONE_WOULD_HAVE_FAILED_site_prefixes(sf) -> None: + """WHY THE RULE IS SPLIT, pinned as a test rather than left in a comment. + + site_prefixes is floor 1 against 2 loaded. As a ratio that is 50% and always will be -- ANY growth + from 1 to 2 scores 50%, however healthy. At that size a ratio measures the SECTION'S SIZE, not the + floor's staleness, so a flat 80% rule would red the gate on its first real run while nothing is + wrong. + """ + floor, loaded = _FLOOR_IN_CI["site_prefixes"], _LIVE_2026_08_26["site_prefixes"] + assert floor / loaded < sf._MIN_FLOOR_RATIO, "the premise of the split rule stopped holding" + # ...and the shipped rule passes it anyway, via the absolute arm. + assert sf.floor_freshness_failure({"site_prefixes": floor}, {"site_prefixes": loaded}) is None + + +def test_a_small_section_fails_on_an_ABSOLUTE_lag(sf) -> None: + """1 against 3 is drift; 1 against 2 is not. The rule has to separate them.""" + assert sf.floor_freshness_failure({"site_prefixes": 1}, {"site_prefixes": 2}) is None + why = sf.floor_freshness_failure({"site_prefixes": 1}, {"site_prefixes": 3}) + assert why and "site_prefixes" in why and "lags" in why, why + + +def test_a_large_section_fails_on_the_RATIO(sf) -> None: + why = sf.floor_freshness_failure({"names": 2}, {"names": 8}) + assert why and "names" in why and "25%" in why, why + assert sf.floor_freshness_failure({"names": 7}, {"names": 8}) is None + + +def test_the_message_names_the_section_AND_BOTH_NUMBERS(sf) -> None: + """A drift warning that does not say which section, and by how much, sends the reader to grep.""" + why = sf.floor_freshness_failure({"estate": 6}, {"estate": 14}) + assert why is not None + for fragment in ("estate", "6", "14"): + assert fragment in why, f"{fragment!r} missing from: {why}" + + +def test_a_section_AT_OR_BELOW_its_floor_is_not_this_function_s_question(sf) -> None: + """That is `token_floor_failure`'s job -- the list falling below the floor. Answering it here too + would double-report one defect and, worse, imply this check covers it when it does not.""" + assert sf.floor_freshness_failure({"names": 9}, {"names": 8}) is None + assert sf.floor_freshness_failure({"names": 8}, {"names": 8}) is None + + +def test_no_floor_configured_is_not_a_freshness_failure(sf) -> None: + """Absence of a floor is `MEFOR_REQUIRE_TOKENS`'s question. Failing here would make the gate fire + on every contributor machine, and a gate that fires on everyone gets switched off.""" + assert sf.floor_freshness_failure(None, _LIVE_2026_08_26) is None + assert sf.floor_freshness_failure({}, _LIVE_2026_08_26) is None + + +def test_THE_COUNTS_CANNOT_DISCRIMINATE_SYNTHETIC_FROM_REAL_BUT_THE_MODE_CAN( + sf, monkeypatch +) -> None: + """THE FINDING THAT SHAPED THIS ITEM, pinned so nobody verifies a floor from counts alone. + + On 2026-08-26 the SYNTHETIC example set and the REAL list both reported 8/14/2. Three identical + numbers from two tables, one of which matches nothing real. A caller checking counts would assert a + floor against a set that cannot fire, and read the pass as evidence. + + `mode` is the only field that separates them, so the report must carry it. + """ + counts = dict(_LIVE_2026_08_26) + monkeypatch.setattr(sf, "TOKENS_PRESENT", True) + monkeypatch.setattr(sf, "is_synthetic_token_set", lambda: True) + synthetic = sf._detector_count_report(counts) + monkeypatch.setattr(sf, "is_synthetic_token_set", lambda: False) + real = sf._detector_count_report(counts) + + assert "mode=synthetic" in synthetic and "mode=real" in real + # ...and the COUNT lines are byte-identical between them, which is the whole point. + assert [x for x in synthetic if not x.startswith("mode=")] == [ + x for x in real if not x.startswith("mode=") + ], "if the counts ever differ here, this test has stopped demonstrating the hazard" + + +def test_no_token_source_reports_mode_none_rather_than_a_clean_zero(sf, monkeypatch) -> None: + """ "loaded nothing" and "loaded a real list that happens to be small" must not render alike.""" + monkeypatch.setattr(sf, "TOKENS_PRESENT", False) + assert "mode=none" in sf._detector_count_report({"names": 0})