diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0dfda7aa..ddc72ea8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,7 +139,7 @@ jobs: # contradictions, no duplicates — is satisfied just as easily by scanning a remnant of the corpus # as the whole of it. Without a floor, a change that stopped the archive being read would go green. # Raise the number when the total legitimately grows; it must never be lowered to make CI pass. - # ⚠️ THE FLOOR LIVES IN TWO PLACES AND NOTHING COMPARES THEM: here, and `_MIN_TOTAL_ITEMS` in + # WARNING: THE FLOOR LIVES IN TWO PLACES AND NOTHING COMPARES THEM: here, and `_MIN_TOTAL_ITEMS` in # tests/test_backlog_status_check.py. Raise BOTH, or the lower one becomes the only floor that # binds. Found on 2026-08-05 at 277 against a corpus of 300 — 23 items of accumulated slack, in a # guard whose entire purpose is to notice the corpus shrinking. @@ -167,7 +167,7 @@ jobs: # docs-only PR pays a base editable install and ~5s of tests rather than the full extras install. # The gated steps below are deliberately UNTOUCHED, so a code PR is byte-identical to before. # - # ⚠️ 89 of these tests SKIP here and that is structural, not a gap to fix in this step: + # WARNING: 89 of these tests SKIP here and that is structural, not a gap to fix in this step: # `tests/test_threat_model_doc_drift.py` asserts against `docs/security/THREAT-MODEL.md`, which is # vault-only and absent from this tree. ADR 0156 records that class (six `*_doc_drift` modules # assert against documents `git ls-files docs/security/` shows are not present) and ASVS 15.1.3 is @@ -295,7 +295,7 @@ jobs: # Step-level watchdog UNDER the job cap (#55): the windows-2022 leg intermittently # wedges ~25% in (a Windows ProactorEventLoop listener-teardown / socket wait that the shared # session event loop can't get past), emits no output for ~12 min, then the JOB cap CANCELS it - # — a red ✗ with no stack and no named test. Two belts make that fail FAST and NAMED instead: + # — a red X with no stack and no named test. Two belts make that fail FAST and NAMED instead: # * `--timeout-method=thread` (the ONLY method on Windows — SIGALRM is POSIX-only) dumps ALL # thread stacks at the per-test cap (matrix `pytest_timeout`, 120s on these Windows legs), # naming the stuck frame. @@ -834,7 +834,11 @@ jobs: # A path appears in `changed` even if only deleted/renamed, so a pure doc rename still short- # circuits, and a code deletion still runs. # Allowlisted (docs-only) paths: any *.md anywhere, docs/**, top-level LICENSE/NOTICE/AUTHORS, - # .editorconfig, .gitignore/.gitattributes, .github/{ISSUE_TEMPLATE,PULL_REQUEST_TEMPLATE,...}.md. + # .editorconfig, .gitattributes, .github/{ISSUE_TEMPLATE,PULL_REQUEST_TEMPLATE,...}.md. + # NOT .gitignore -- #327 removed it from the regex and this line went on listing it for + # weeks, the THIRD instance in this file of a comment stating the opposite of its code. + # `.gitattributes` IS still listed here and is nonetheless CODE: `alwayscodepath` below is + # checked first and wins. Precedence, not deletion -- see the reasoning there. # Be conservative: when in doubt a path is CODE. Empty diff (shouldn't happen on a PR) => code=true. # `.gitignore` is NOT in this allowlist, deliberately (BACKLOG #327). Six of its rules are the # sole control keeping maintainer-internal material out of a public commit, and @@ -845,8 +849,52 @@ jobs: # ungated backlog guards above were added for. Treating it as code costs one suite run on a # rare PR; the alternative costs the publishing boundary, silently. noncode='(\.md$|^docs/|^LICENSE$|^NOTICE$|^AUTHORS$|^\.editorconfig$|^\.gitattributes$|^\.github/(ISSUE_TEMPLATE/|PULL_REQUEST_TEMPLATE))' + # EXTENSIONLESS CONFIG THAT IS ALWAYS CODE (BACKLOG #1200). The extension rule below cannot + # reach a file with no extension, and `.gitattributes` is exactly that: it was still in the + # docs-only allowlist above, so a `.gitattributes`-only PR skipped lint, mypy and the whole + # suite. It is not cosmetic -- the vault's own asvs-scorecard.yml records that a change here + # "silently alters how the corpus is materialized, which is exactly what makes the digest + # differ", so it is an input to the ASVS corpus pin. + # + # Stated as a POSITIVE list and CHECKED FIRST, rather than by deleting the entry from + # `noncode`, for two reasons. "Is code" should not depend on the ABSENCE of a line somewhere + # else -- a future edit re-adding a path to `noncode` would silently undo this with nothing + # to say so. And keeping the historical `noncode` intact is what lets + # tests/test_ci_docs_only_detector.py assert the regression in BOTH directions: it can still + # reconstruct the old classification by disabling this rule alone. `.gitignore` is named for + # the first reason even though #327 already removed it -- it was code only by falling through. + alwayscodepath='^(\.gitattributes|\.gitignore)$' + # EXTENSION OVERRIDE, EVALUATED FIRST (BACKLOG #1200). An executable file is CODE wherever it + # lives, including under docs/. + # + # The paragraph above states the intent exactly -- "any *.py ... counts as CODE" -- and the + # regex did not implement it, because `^docs/` is an alternation branch that matches a .py + # under docs/ and short-circuits before the *.py rule is ever reached. An auditor reads the + # comment, agrees with it, and moves on. Measured on 2026-08-09: + # docs/security/asvs-apply-cells.py classified NON-CODE -- the tool that WRITES the ASVS + # record of record, able to silently un-close an owner-closed cell, exempt from lint, mypy + # and the entire pytest suite by virtue of its directory. Two mypy errors had been sitting in + # it since it was written; they could not have survived a single check. + # + # THE PRECEDENT IS FOUR LINES ABOVE THIS ONE. BACKLOG #327 fixed exactly this shape for + # `.gitignore` and wrote the lesson down -- and the identical defect for docs/**/*.py sat in + # the regex immediately below the paragraph explaining it. The instance was fixed and the + # class left open, with the reasoning that would have closed it preserved in place. Hence an + # EXTENSION rule rather than another one-path exception: the next executable file someone + # puts under docs/ must not need this discovered a third time. + # + # The docs-only optimisation is deliberately preserved for actual documents -- deleting + # `^docs/` outright would run the full suite on every prose edit, which is the cost this + # short-circuit exists to avoid. Order matters: this is checked BEFORE `noncode`. + alwayscode='\.(py|ps1|sh|ts|js|yml|yaml|toml|lock|cfg|ini)$' if [ -z "$changed" ]; then echo "code=true" >> "$GITHUB_OUTPUT" + elif echo "$changed" | grep -qE "$alwayscode"; then + # An executable/config file changed, wherever it lives -> run the full suite. + echo "code=true" >> "$GITHUB_OUTPUT" + elif echo "$changed" | grep -qE "$alwayscodepath"; then + # Extensionless config the extension rule cannot see -> run the full suite. + echo "code=true" >> "$GITHUB_OUTPUT" elif echo "$changed" | grep -qvE "$noncode"; then # At least one changed path is NOT docs-only -> run the full suite. echo "code=true" >> "$GITHUB_OUTPUT" diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index e9dfc4a1..053630c1 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -8714,6 +8714,64 @@ filing. **Source:** filed 2026-08-08 from the ASVS ledger-coverage sweep of the partial and fail cells that carried no backlog item at all; this cell was one of them. The scorecard is the record of record for the verdict; this item tracks the research toward changing it. +## 1200. the CI docs-only detector exempts EXECUTABLE files under `docs/` from the entire suite + +> 🔢 **Filed 2026-08-09 - FIXED in the same change. Reproduced with the workflow's own regex under real `grep -E`, with a negative control.** Value **7/10** · Difficulty **2/10**. `ci.yml`'s `changes` job short-circuits the required `test` legs when every changed path is docs-only. `^docs/` is an alternation branch in that allowlist, so it matches a **`.py` under `docs/`** and short-circuits before the stated `*.py` rule is ever reached. A PR touching only such a file set `code=false` and skipped install, lint, type-check and the whole of pytest. + +**Cluster:** CI correctness / gate blindness. **Priority:** P2. **Verdict:** build (done). +**Severity:** no product effect and no PHI effect. The cost is that a defect here does not fail loudly +- it REMOVES the thing that would have failed, which is the worst failure mode a gate has. + +**Measured, not reasoned.** Extracting the live regex from `ci.yml` and running real `grep -E`: + +``` +PRE-FIX (noncode only): + docs/security/asvs-apply-cells.py -> NON-CODE (suite skipped) + docs/benchmarks/.../b5_microbench.py -> NON-CODE (suite skipped) +POST-FIX (alwayscode checked first): + docs/security/asvs-apply-cells.py -> code + docs/SECURITY.md -> NON-CODE (still short-circuits) + .gitignore -> code (via the noncode branch, BACKLOG #327) +``` + +**Blast radius.** Engine: 2 files, both benchmark scripts under +`docs/benchmarks/results/2026-07-04-adr0071-b5-executor-marshaling/` - low risk. Vault: 3 files, +including `docs/security/asvs-apply-cells.py`, the tool that WRITES the ASVS record of record and can +silently un-close an owner-closed cell. **Two mypy errors had been sitting in that file since it was +written; they could not have survived a single check.** That is the corroboration that the exemption +was real and not theoretical. + +**TWO THINGS MAKE THIS WORSE THAN A MISSING TEST.** + +**The comment and the regex disagree, and the comment is what people read.** `ci.yml` states the intent +in as many words: *"Anything outside the allowlist - any `*.py`, `ide/**`, config, lockfiles, OTHER +workflows, scripts, samples, harness - counts as CODE and runs the full suite."* The regex does not +implement that sentence. An auditor reads the comment, agrees with it, and moves on. + +**The precedent sits four lines above the defect.** `#327` fixed exactly this shape for `.gitignore` - +allowlisted as docs-only, so a `.gitignore`-only PR skipped `tests/test_private_paths_stay_ignored.py`, +*"the one guard that would catch the rule being deleted DID NOT RUN, on exactly the PR shape it exists +to catch"* - and the lesson was written down in place. The identical defect for `docs/**/*.py` was in +the regex immediately below that paragraph. **The instance was fixed and the class was left open, with +the reasoning that would have closed it preserved alongside.** That is the recurring shape: a fix that +does not generalise is the one that comes back. + +**The fix.** An `alwayscode` EXTENSION check evaluated BEFORE the `noncode` allowlist: +`\.(py|ps1|sh|ts|js|yml|yaml|toml|lock|cfg|ini)$`. An executable file is code wherever it lives. The +docs-only optimisation is deliberately preserved for actual documents - simply deleting `^docs/` would +have run the full suite on every prose edit, which is the cost the short-circuit exists to avoid. + +**The test drives the DETECTOR, and reads its regexes OUT of `ci.yml`.** A test carrying its own copy +of the pattern passes forever while the workflow drifts underneath it, reproducing this very defect one +level up. It asserts the regression in BOTH directions in a single test - the pre-fix logic classifies +`docs/x.py` as non-code AND the post-fix logic does not - because asserting only the new behaviour +cannot distinguish a fixed detector from a deleted one (`return True` passes that). It carries a +negative control, so a regex that accidentally matched everything cannot make every assertion pass +vacuously. + +**Source:** found 2026-08-09 while promoting the ASVS writer out of `docs/security/` (BACKLOG #1200's +sibling work), and escalated from instance to class by the parallel `asvs-tracking-rework` session, +which measured the blast radius in both repos and identified the `#327` precedent. ## 1201. `redacted_settings` served credential-bearing HTTP headers outside a five-name list > 🔢 **Filed 2026-08-09 - FIXED IN THE SAME CHANGE, and the entry is published WITH the fix rather than ahead of it.** Value **8/10** · Difficulty **2/10**. Header redaction was `str(k).lower() in _SECRET_HEADER_NAMES` -- an exact-membership test against **five** strings (`authorization`, `proxy-authorization`, `x-api-key`, `api-key`, `cookie`). Header names are **operator-authored free text**, typed into `connections.toml` or a Handler, so an exhaustive list cannot exist even in principle. Measured against the shipped list: `X-Auth-Token`, `X-Amz-Security-Token` and `Private-Token` were all returned VERBATIM. diff --git a/tests/test_ci_docs_only_detector.py b/tests/test_ci_docs_only_detector.py new file mode 100644 index 00000000..61218bbc --- /dev/null +++ b/tests/test_ci_docs_only_detector.py @@ -0,0 +1,211 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The CI docs-only short-circuit, driven directly (BACKLOG #1200). + +`ci.yml`'s `changes` job decides whether a PR runs the suite at all. When it says `code=false`, +install, lint, type-check and the whole of pytest are skipped — so a defect in this detector does not +fail loudly, it removes the thing that would have failed. That is the worst failure mode a gate has, +and until now the detector had no test of its own. + +**The regexes are READ OUT OF `ci.yml`, never copied here.** A test carrying its own copy of the +pattern passes forever while the workflow drifts underneath it, which would reproduce the defect this +file exists to prevent one level up. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +CI = Path(__file__).resolve().parent.parent / ".github" / "workflows" / "ci.yml" + + +def _extract(var: str, *, required: bool = True) -> str: + """Pull a single-quoted shell assignment (`name='...'`) out of the workflow.""" + text = CI.read_text(encoding="utf-8") + m = re.search(rf"^\s*{re.escape(var)}='([^']*)'\s*$", text, re.M) + if not m and not required: + return "" + assert m, f"{var}= not found in {CI.name}; the detector was renamed or restructured" + return m.group(1) + + +def classify( + paths: list[str], *, alwayscode: str | None, noncode: str, alwayscodepath: str = "" +) -> bool: + """Mirror of the shell decision in `ci.yml`'s `changes` step. Returns `code`. + + `grep -qE P` -> any line matches P. + `grep -qvE P` -> any line does NOT match P. + + `alwayscode=None` reproduces the PRE-#1200 logic, which is what makes the regression test below + able to tell a fixed detector from a deleted one. + """ + if not paths: + return True + if alwayscode is not None and any(re.search(alwayscode, p) for p in paths): + return True + # Extensionless config the extension rule cannot reach (BACKLOG #1200 amendment). + if alwayscodepath and any(re.search(alwayscodepath, p) for p in paths): + return True + # The shell's final `elif ... else`: any path outside the docs allowlist means CODE, otherwise the + # diff is docs-only and short-circuits. + return any(not re.search(noncode, p) for p in paths) + + +@pytest.fixture(scope="module") +def pats() -> tuple[str, str, str]: + return _extract("alwayscode"), _extract("noncode"), _extract("alwayscodepath") + + +# --- the defect, and proof the probe can see it --------------------------------------------------- + + +@pytest.mark.parametrize( + "path", + [ + "docs/security/asvs-apply-cells.py", # the ASVS record WRITER, measured exempt 2026-08-09 + "docs/benchmarks/results/2026-07-04-adr0071-b5-executor-marshaling/b5_microbench.py", + "docs/anything/script.sh", + "docs/anything/module.ts", + "docs/anything/config.toml", + "docs/anything/workflow.yml", + ], +) +def test_an_executable_under_docs_is_code_and_was_not_before( + path: str, pats: tuple[str, str, str] +) -> None: + """THE REGRESSION, asserted in both directions in one test. + + Asserting only that the new detector says CODE cannot distinguish a fixed detector from a deleted + one — `return True` passes that. So this also asserts the OLD logic said NON-CODE for the same + path. If someone reverts the fix, the first assertion fails; if someone guts the detector into a + constant, the second fails. + """ + alwayscode, noncode, alwayscodepath = pats + assert classify([path], alwayscode=None, noncode=noncode) is False, ( + "the pre-#1200 detector should classify this as docs-only; if this fails the test has lost " + "its grip on the historical behaviour and proves nothing about the fix" + ) + assert ( + classify([path], alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) + is True + ) + + +# --- the optimisation this fix must NOT destroy --------------------------------------------------- + + +@pytest.mark.parametrize( + "path", + [ + "docs/SECURITY.md", + "docs/adr/0156-asvs-scorecard-as-data.md", + "README.md", + "LICENSE", + "NOTICE", + ], +) +def test_a_real_document_still_short_circuits(path: str, pats: tuple[str, str, str]) -> None: + """Deleting `^docs/` would have fixed the defect and run the full suite on every prose edit. + + The short-circuit exists to avoid exactly that cost, so preserving it for actual documents is part + of the requirement, not a nicety. + """ + alwayscode, noncode, alwayscodepath = pats + assert ( + classify([path], alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) + is False + ) + + +@pytest.mark.parametrize("path", [".gitattributes", ".gitignore"]) +def test_extensionless_config_is_code_and_gitattributes_was_not_before( + path: str, pats: tuple[str, str, str] +) -> None: + """The gap the EXTENSION rule structurally cannot close (BACKLOG #1200 amendment). + + `#1200` made an executable file code wherever it lives -- keyed on the extension. `.gitattributes` + has none, and it was still sitting in the docs-only allowlist, so a `.gitattributes`-only PR + skipped lint, mypy and the entire suite. Not cosmetic: the vault's `asvs-scorecard.yml` records + that a change here "silently alters how the corpus is materialized, which is exactly what makes + the digest differ", so it is an input to the ASVS corpus pin. + + Asserted in BOTH directions for `.gitattributes`, because asserting only the new behaviour cannot + distinguish a fixed detector from a deleted one. `.gitignore` is code under both rules -- #327 + removed it from `noncode` -- so only the forward direction is asserted for it; it is named in + `alwayscodepath` so that "is code" stops depending on the ABSENCE of a line elsewhere. + """ + alwayscode, noncode, alwayscodepath = pats + if path == ".gitattributes": + assert ( + classify([path], alwayscode=alwayscode, noncode=noncode, alwayscodepath="") is False + ), "pre-amendment, .gitattributes should classify docs-only; the test has lost its grip" + assert ( + classify([path], alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) + is True + ) + + +# --- ordinary code stays code ---------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "path", + [ + "messagefoundry/api/app.py", + "scripts/asvs/apply.py", + "tests/test_asvs_apply.py", + ".github/workflows/ci.yml", + "pyproject.toml", + ".gitignore", # BACKLOG #327 — deliberately code, and it has no code-ish extension + ], +) +def test_code_paths_run_the_suite(path: str, pats: tuple[str, str, str]) -> None: + alwayscode, noncode, alwayscodepath = pats + assert ( + classify([path], alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) + is True + ) + + +# --- mixed diffs, and the probe's own liveness ------------------------------------------------------ + + +def test_one_executable_among_many_documents_still_runs_the_suite( + pats: tuple[str, str, str], +) -> None: + """The realistic shape: a docs PR that also touches one script. Conservative means CODE.""" + alwayscode, noncode, alwayscodepath = pats + paths = ["docs/a.md", "docs/b.md", "docs/security/helper.py", "README.md"] + assert ( + classify(paths, alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) + is True + ) + + +def test_an_empty_diff_is_treated_as_code(pats: tuple[str, str, str]) -> None: + """`ci.yml` defaults to running everything when it cannot tell. Pinned so that stays true.""" + alwayscode, noncode, alwayscodepath = pats + assert ( + classify([], alwayscode=alwayscode, noncode=noncode, alwayscodepath=alwayscodepath) is True + ) + + +def test_the_probe_is_not_matching_everything(pats: tuple[str, str, str]) -> None: + """NEGATIVE CONTROL. A regex that accidentally matched every path would make every assertion above + pass while proving nothing — the same class of blindness the ASVS absence claims guard against with + a positive control.""" + alwayscode, noncode, alwayscodepath = pats + assert not re.search(alwayscode, "docs/SECURITY.md") + assert not re.search(noncode, "messagefoundry/api/app.py") + + +def test_both_patterns_are_still_read_from_the_workflow(pats: tuple[str, str, str]) -> None: + """If either assignment is renamed or restructured, `_extract` raises and this file goes red rather + than silently testing nothing.""" + alwayscode, noncode, alwayscodepath = pats + assert alwayscode and noncode + assert "py" in alwayscode and "docs/" in noncode