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 7d5afab9e..022e81259 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -16919,3 +16919,33 @@ cpu_util_cores_mean 0 **Related:** #1211 (limb one shipped the channel this reuses; limb two blocked), #1101 (the per-message form these annotate), #320 (windows-2025 leg timing, one of the explanations these fields separate). **Source:** scoped by the Dispatcher, corrected by this lane's scouting -- the original scoping described artifact-upload work already done and a selection bias already fixed by #1211 limb one; what remained was the band-less fields and the constraint above. + +## 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})