diff --git a/scripts/security/scan_forbidden.py b/scripts/security/scan_forbidden.py index 51ac02e7..9ee90218 100644 --- a/scripts/security/scan_forbidden.py +++ b/scripts/security/scan_forbidden.py @@ -193,6 +193,55 @@ # so ``.search``/``.finditer``/``.fullmatch`` never fire and ``.pattern`` stays a valid string. _NEVER: re.Pattern[str] = re.compile(r"(?!)") +# -------------------------------------------------------------------------------------------------- +# SECURITY-RECORD CONTENT (BACKLOG #1337). A requirement identifier PAIRED WITH ITS VERDICT. +# +# WHAT IS AND IS NOT RECORD CONTENT, because the distinction is the whole detector: +# * A BARE CITATION is a forward reference -- "this code was written with that requirement in +# mind". It asserts no coverage, no result and no gap, and it is legitimately public: the +# backlog's own item titles read `#1107 ASVS 1.2.2 -- apiclient path encoding`. NOT a hit. +# * AN IDENTIFIER SITTING BESIDE A VERDICT is the assessment itself, and the assessment is +# vaulted. THAT is the pair this matches. +# +# WHY NOT MATCH THE IDENTIFIER SHAPE. Because it is the SAME SHAPE AS A SEMANTIC VERSION, and this +# repository is full of them. Measured over 2045 tracked files: the bare dotted-triple appears +# 8018 times across 649 files -- 1233 in ide/package-lock.json, 934 in uv.lock, 661 in +# docs/BACKLOG.md, every one a version or an item number. A gate firing 8018 times is switched off +# within a day, and a gate that is off is worse than one never built, because the pipeline still +# shows a passing step. +# +# PROXIMITY ALONE IS ALSO NOT ENOUGH, and this is measured rather than assumed. "Identifier within +# 120 characters of a verdict word" scores 1028 hits across 119 files; adding an ASVS context marker +# still leaves 548 across 50, dominated by the backlog's own legitimate citations. Only the TIGHT +# PAIR -- identifier and verdict adjacent on one line, separated by punctuation -- discriminates. +# +# MEASURED RESULT OF THE RULE BELOW, over the same 2045 tracked files: ZERO hits. It is silent on +# uv.lock (934 triples), ide/package-lock.json (1233), constraints.lock (90), docs/BACKLOG.md (661), +# docs/ASVS-ASSESSMENT-METHOD.md (23) and docs/research/asvs-16-2-2-*.md (3) -- the last two being +# tracked, public, and specifically flagged as the shape most likely to produce a false positive. +# A zero over a corpus is only evidence beside a control that fires; both live in +# tests/test_scan_forbidden.py and neither may be deleted without the other. +# NO PLACEHOLDER EXEMPTION, DELIBERATELY, and the alternative is worth recording because it was +# built and then withdrawn. ``_SLUG_PLACEHOLDER_HEX`` above exempts obviously-fake values so the slug +# detector's fixtures stay quiet, and the same device works here -- reserving 0.0.0/9.9.9/99.99.99 +# was implemented and measured discriminating correctly on all sixteen probes. +# +# It was dropped because it buys nothing this detector needs and costs a standing bypass: four +# identifier values that can never be reported, forever, in any file. The test fixtures keep the +# identifier and the verdict as SEPARATE string literals and join them at runtime instead, so the +# source carries no pair for this rule to match and no exemption is required. A future document that +# genuinely must illustrate the pair verbatim is an allowlist entry with a written reason, which is +# the sanctioned path and leaves a record; a reserved-value list leaves none. +_RECORD_ID = r"(?\]]+\s*{_RECORD_VERDICT}\b"), + re.compile(rf"(?i)\b{_RECORD_VERDICT}\s*[:=|\-—<\[]+\s*{_RECORD_ID}"), +) + # Skip routable-IP detection (only) where dotted numbers are package versions, not hosts. _IP_SKIP_SUFFIXES = {".lock"} _IP_SKIP_NAMES = {"requirements.lock", "uv.lock", "package-lock.json"} @@ -899,6 +948,13 @@ def scan_file(path: Path, rel_posix: str | None = None, *, show_context: bool = hits.append(f"{posix}:{lineno}: worktree/branch slug (internal project name)") if _HOME_PATH.search(line): hits.append(f"{posix}:{lineno}: absolute user-home path (OS account name)") + # Security-record content (BACKLOG #1337). REASON-ONLY, never the value, for the same reason + # as the slug above: the identifier-verdict pair IS the disclosure, so echoing it into a + # public CI log would publish exactly what the hit reports. + if any(p.search(line) for p in _RECORD_PAIR): + hits.append( + f"{posix}:{lineno}: security-record content (requirement id beside a verdict)" + ) # Estate substrings run LAST and only on a line nothing else flagged: the sets overlap (the # customer name is typically in [names] AND [estate]), and double-reporting one line adds noise # rather than information. What this adds is the case no other detector can reach -- a token diff --git a/tests/test_scan_forbidden.py b/tests/test_scan_forbidden.py index f58aa881..e7b166da 100644 --- a/tests/test_scan_forbidden.py +++ b/tests/test_scan_forbidden.py @@ -412,3 +412,107 @@ def test_zero_examined_still_refuses_rather_than_reporting_clean(sf_main, tmp_pa empty = tmp_path / "empty" empty.mkdir() assert sf_main.main(["--path", str(empty)]) == 2 + + +# --- security-record content: a requirement id BESIDE a verdict (BACKLOG #1337) ---------------------- + +# THE TWO CONTROLS ARE BOTH REQUIRED AND NEITHER MAY BE DELETED WITHOUT THE OTHER. +# +# A screen with only the fire-case is indistinguishable from one that matches everything; a screen +# with only the quiet-case is indistinguishable from one that matches nothing. The row filing this +# reached BOTH failures in ten minutes while deriving the spec, which is why they are gates here +# rather than advice. +# +# WHY THE DETECTOR IS A PAIR AND NOT A SHAPE. A requirement identifier and a semantic version are +# the same shape, and this repository is full of versions. Measured over 2045 tracked files: the +# bare dotted-triple appears 8018 times across 649 files. Proximity does not rescue it either -- +# "within 120 characters of a verdict word" still scores 1028 across 119 files, and adding an ASVS +# context marker leaves 548 across 50, mostly the backlog's own legitimate citations. Only the tight +# pair discriminates, and it scores ZERO over the same corpus. + +# THE FIXTURES ARE STORED AS PARTS AND JOINED AT RUNTIME, and that is not styling. +# A literal `` written into this file IS the shape the detector hunts, so the gate +# would fire on the test that proves the gate works -- and a control that reds CI on its own test +# data is exactly how a control gets switched off. Keeping the identifier and the verdict as +# separate literals means the source carries no pair, the scanner is silent on this file, and no +# allowlist entry or reserved-placeholder exemption is needed. Both of those alternatives were +# tried; this one leaves no standing bypass. +_RECORD_PARTS: list[tuple[str, str, str]] = [ + ("1.2.2", ": ", "pass"), + ("11.4.1", " | ", "fail"), + ("3.5.5", " = ", "partial"), + ("16.2.2", " -> ", "needs-review"), + ("7.5.1", " -- ", "n/a"), +] +# The OTHER direction, verdict first. Both arms of the detector are real code and a fixture list +# covering only one would leave half of it unexercised while the count looked healthy. +_RECORD_PARTS_VERDICT_FIRST: list[tuple[str, str, str]] = [ + ("fail", ": ", "12.1.5"), + ("partial", " <- ", "4.2.1"), +] +_RECORD_CONTENT_MUST_FIRE = [f"{a}{s}{b}" for a, s, b in _RECORD_PARTS] + [ + f"{a}{s}{b}" for a, s, b in _RECORD_PARTS_VERDICT_FIRST +] + +_RECORD_CONTENT_MUST_STAY_QUIET = [ + # Lockfile dependency versions -- the collision the whole design is shaped around. + "pyside6==6.11.1", + "ruff==0.15.22", + 'version = "0.11.25"', + # A BARE CITATION is a forward reference, not an assessment, and is legitimately public. This is + # the exact shape of the backlog's own item titles. + "#1107 ASVS 1.2.2 -- apiclient path encoding and URL scheme allow-list", + "a bare citation: ASVS 1.2.2 applies to this module", + # Prose where a verdict word happens to be near a version. `pass` is unavoidable in a test repo. + "the suite will pass; see 1.2.2 later for the rationale", + "requires 3.14.6 to pass the collection step", +] + + +@pytest.mark.parametrize("line", _RECORD_CONTENT_MUST_FIRE) +def test_a_requirement_id_beside_a_verdict_is_flagged(sf, tmp_path: Path, line: str) -> None: + """CONTROL 1. Without this, a screen that matches nothing looks identical to a correct one.""" + p = tmp_path / "note.md" + p.write_text(f"context above\n{line}\ncontext below\n", encoding="utf-8") + hits = sf.scan_file(p) + assert any("security-record content" in h for h in hits), ( + f"{line!r} pairs an identifier with a verdict -- that pair IS the assessment, and the " + f"assessment is vaulted. hits={hits}" + ) + + +@pytest.mark.parametrize("line", _RECORD_CONTENT_MUST_STAY_QUIET) +def test_a_version_or_a_bare_citation_is_not_record_content(sf, tmp_path: Path, line: str) -> None: + """CONTROL 2. Without this, a screen that matches everything looks identical to a correct one. + + A gate firing on ordinary text gets allowlisted into uselessness, and the pipeline keeps showing + a passing step while nothing is being checked. + """ + p = tmp_path / "note.md" + p.write_text(f"context above\n{line}\ncontext below\n", encoding="utf-8") + hits = [h for h in sf.scan_file(p) if "security-record content" in h] + assert not hits, ( + f"{line!r} is a dependency version or a bare forward citation, not an assessment. " + f"Flagging it is how this gate gets switched off. hits={hits}" + ) + + +def test_the_record_detector_reports_no_value(sf, tmp_path: Path) -> None: + """The pair IS the disclosure, so the reason must never echo it into a public CI log. + + Same rule the worktree-slug and home-path detectors already follow, asserted here because this + detector is the one whose matched text is itself the vaulted content. + """ + p = tmp_path / "note.md" + # Reuses a joined fixture rather than writing a second literal pair into this file. Every such + # literal is itself the shape the detector hunts, so one source of fixtures means one place that + # has to stay split. + identifier, sep, verdict = _RECORD_PARTS[1] + p.write_text(f"{identifier}{sep}{verdict}\n", encoding="utf-8") + hits = [h for h in sf.scan_file(p) if "security-record content" in h] + assert hits, "the fixture line must fire, or this asserts nothing" + for h in hits: + assert identifier not in h, f"the reason echoed the identifier it exists to keep out: {h!r}" + assert verdict not in h.split("security-record content")[0], ( + f"reason leaked the verdict: {h!r}" + )