diff --git a/.github/required-contexts.txt b/.github/required-contexts.txt index a65bf94b..1b424322 100644 --- a/.github/required-contexts.txt +++ b/.github/required-contexts.txt @@ -3,9 +3,28 @@ # WHY THIS FILE EXISTS. Branch protection lives on the server, so "is this check blocking?" was # unanswerable from a clone -- and five places in this repo answered it differently: docs/CI.md # listed 8 contexts, .github/workflows/manifest-lint.yml said 4, docs/design/freethread.md said 7, -# tests/test_push_guard.py said 12. The live API says 12. With `required_approving_review_count: 0` -# and auto-merge armed, required-set membership is the ONLY thing separating "reviewed" from -# "merged unread", so a wrong answer here mis-ranks which gates a change can safely touch. +# tests/test_push_guard.py said 12. With `required_approving_review_count: 0` and auto-merge armed, +# required-set membership is the ONLY thing separating "reviewed" from "merged unread", so a wrong +# answer here mis-ranks which gates a change can safely touch. +# +# It stopped being hypothetical within hours of this file landing. On 2026-07-29 branch protection was +# briefly reduced to a SINGLE required context -- the backlog-hygiene check -- dropping all twelve +# gates, every test leg and security scanner among them, while auto-merge stayed armed and approvals +# stayed at 0. Several PRs merged in that window. What surfaced it was reconciling a CI wait against +# `required_status_checks.contexts` and seeing the count come back as 1: a monitor that had waited for +# "nothing pending" would have reported green without ever waiting for a test leg, because with one +# required context there was nothing to wait for. Count what SHOULD gate the merge, not what does. +# +# The set then moved four more times the same day (1 -> 13 -> 12 -> 15 -> 12). This snapshot is the +# state it settled on, read from the API and unchanged for six minutes before being written down. Two +# non-context settings moved with it and are recorded because they change merge semantics: +# +# strict = TRUE -- a PR must be up to date with `main` before it can merge. This closes the +# stale-green hole: previously a PR validated against an older base could merge +# without revalidation, so a semantic conflict with a newer `main` (a test that +# reads a file the other PR changed) could land red on main. +# enforce_admins = FALSE -- the documented escape hatch in scripts/hooks/push_guard.py, for when a +# permanently red required check blocks every merge. It was ON from 2026-07-28. # # This file is not the enforcement -- the server is. It is the CHECKED-IN CLAIM that every in-repo # statement must agree with, which is what `tests/test_required_contexts.py` asserts. When branch @@ -25,7 +44,12 @@ # it, no `if:` gated on a non-PR event) BEFORE it lands here or in branch protection. # --- ci.yml --------------------------------------------------------------------------------------- -CI gate +# NOTE: `CI gate` is NOT in this list. It was removed from branch protection on 2026-07-29 and the +# reason is not recorded here -- this file states what the server enforces, not why. It matters, +# because that roll-up is what gated ci.yml's CONDITIONAL/matrix legs (ci.yml, job `ci-gate`, +# `if: always()` over its `needs:`). The three `test` legs below are required in their own right and +# are unaffected; what no longer gates a merge is everything the roll-up covered beyond them. If the +# removal was incidental, restoring it is one POST to the contexts endpoint plus a line here. test (ubuntu-latest, py3.14) test (windows-2022, py3.14) test (windows-2025, py3.14) @@ -43,23 +67,28 @@ forbidden-content (customer/PHI leak guard) # --- cla.yml -------------------------------------------------------------------------------------- cla -# PENDING PROMOTION -- owner's decision, NOT yet applied. This file mirrors the LIVE server, so the -# context below is deliberately absent until branch protection actually changes; adding it here first -# would make the file lie in the one direction it exists to prevent. +# --- backlog-hygiene.yml --------------------------------------------------------------------------- +# PROMOTED 2026-07-29 (verified against the live API the same day). Before that it hard-failed and +# nothing waited for it, so it was decoration -- auto-merge blocks only on REQUIRED contexts. It exists +# because #60 shipped while its banner still read "PRE-RESERVED", and that stale banner was then +# repeated as fact in a merged PR; in a repo where the ledger is the only memory shared across isolated +# sessions, a stale banner makes another session rebuild work that is already merged. # -# "a PR that implements BACKLOG #N must update BACKLOG.md" (backlog-hygiene.yml) -# -# WHY PROMOTE: it hard-fails today and nothing waits for it, so it is decoration -- auto-merge only -# blocks on REQUIRED contexts. It exists because #60 shipped while its banner still read -# "PRE-RESERVED", and that stale banner was then repeated as fact in a merged PR. In a repo where the -# ledger is the only memory shared across isolated sessions, a stale banner makes another session -# rebuild work that is already merged. -# SAFE TO PROMOTE: it triggers on `pull_request: branches: [main]` with NO paths filter and no `if:`, -# so it always reports -- no required-but-absent trap. (Its STRUCTURAL half, "every item declares +# The quoting in backlog-hygiene.yml is load-bearing: the job name contains " #", which YAML would +# truncate at the comment marker, and the job NAME is the context string. +a PR that implements BACKLOG #N must update BACKLOG.md + +# (Historical note, kept because it is the reasoning that made the promotion safe: it triggers on +# `pull_request: branches: [main]` with NO paths filter and no `if:`, so it always reports -- no +# required-but-absent trap. Its STRUCTURAL half, "every item declares # exactly one status", is already enforced inside the required test legs by -# tests/test_backlog_status_check.py; only the behavioural half is unguarded.) -# TO APPLY: add the context to branch protection, then move the line above into the ci/security list -# and update the count pinned in tests/test_required_contexts.py. +# tests/test_backlog_status_check.py; only the behavioural half was unguarded.) +# +# ADDING ANOTHER ONE LATER: add the context to branch protection FIRST, then add the line here and +# update the count pinned in tests/test_required_contexts.py, in the same PR. In that order, because +# this file mirrors the live server -- a line added here first is exactly the lie the file exists to +# prevent. Adding to protection is all-or-nothing: the REST endpoint 422s with `already_exists` if any +# context in the request is already required, and then adds NONE of them, so send only the missing ones. # DELIBERATELY NOT REQUIRED (do not add without the owner's decision; each has a recorded reason): # codeql.yml, scorecard.yml -- their SARIF upload needs `security-events: write`, which fork-PR diff --git a/docs/CI.md b/docs/CI.md index bac6f8b3..01f8c228 100644 --- a/docs/CI.md +++ b/docs/CI.md @@ -34,7 +34,6 @@ would otherwise wedge the PR — see the gotcha below). The stable contexts required on `main` are — mirroring [`.github/required-contexts.txt`](../.github/required-contexts.txt), which is the file to edit: -- `CI gate` - `test (ubuntu-latest, py3.14)` - `test (windows-2022, py3.14)` - `test (windows-2025, py3.14)` @@ -45,6 +44,7 @@ The stable contexts required on `main` are — mirroring - `semgrep (project SAST rules)` - `crypto-inventory (ASVS 11.1.3 discovery gate)` - `forbidden-content (customer/PHI leak guard)` +- `a PR that implements BACKLOG #N must update BACKLOG.md` - `cla` That last string is the **job key** in `cla.yml`, whose job declares no `name:`. Branch protection diff --git a/scripts/hooks/push_guard.py b/scripts/hooks/push_guard.py index b12ba5d7..452ad8d9 100644 --- a/scripts/hooks/push_guard.py +++ b/scripts/hooks/push_guard.py @@ -6,9 +6,11 @@ publication, immediately and irreversibly (deleting a ref later does not un-publish content that was fetched, mirrored or indexed in between). -Branch protection on the server requires a PR and 12 status checks. ``enforce_admins`` is now TRUE -(enabled 2026-07-28), so a direct push to ``main`` is refused server-side and ``gh pr merge --admin`` -no longer works. This hook is therefore DEFENCE-IN-DEPTH rather than the only guard -- it still earns +Branch protection on the server requires a PR and 12 status checks, and `strict` is ON (a PR must be +up to date with ``main`` to merge). ``enforce_admins`` was enabled 2026-07-28 and DISABLED again on +2026-07-29 via the escape hatch in the HISTORY note below, so ``gh pr merge --admin`` works once more +and a direct push to ``main`` is refused only by this hook. That makes it the ONLY guard again for +that one path, not merely defence-in-depth -- it still earns its place by failing FAST and LOCALLY, with an explanation, instead of after a round-trip; and it covers ``cla-signatures``, which branch protection does not. diff --git a/tests/test_push_guard.py b/tests/test_push_guard.py index b628b084..0b15a188 100644 --- a/tests/test_push_guard.py +++ b/tests/test_push_guard.py @@ -2,8 +2,8 @@ Since the MEFORORG cutover this repository IS the published artifact — a push to ``main`` is publication, immediately, with no publish step left to catch anything. Server-side branch protection -requires a PR and 12 status checks, and since 2026-07-28 ``enforce_admins`` is TRUE, so the owner no -longer bypasses it either. This hook is now defence-in-depth: it fails FAST and LOCALLY with an +requires a PR and 12 status checks. ``enforce_admins`` was ON from 2026-07-28 but is OFF again as of +2026-07-29 (the documented escape hatch in the hook's own HISTORY note), so the owner CAN bypass. This hook is now defence-in-depth: it fails FAST and LOCALLY with an explanation rather than after a round-trip, and it covers ``cla-signatures``, which branch protection does not. (It was written while ``enforce_admins`` was false and the hook WAS the only guard.) The realistic trigger is unchanged: one click on VS Code's Sync button while the current branch happens to diff --git a/tests/test_security_posture.py b/tests/test_security_posture.py index 950030ff..06f854ee 100644 --- a/tests/test_security_posture.py +++ b/tests/test_security_posture.py @@ -79,6 +79,35 @@ (re.compile(r"--fail-under=0\b"), "--fail-under=0"), ) +#: A `$( ... )` command substitution on one line. What is inside CANNOT set the step's exit status — +#: it sets the substitution's, which the surrounding assignment then consumes. +_SUBSTITUTION = re.compile(r"\$\([^()]*\)") + +#: A trailing shell comment. Approximate on purpose (a `#` inside a quoted string is not a comment), +#: and safe in this direction: stripping a comment can only REMOVE text from the scan, and a real +#: `cmd || true # note` still reads as `cmd || true` afterwards. +_TRAILING_COMMENT = re.compile(r"(?:^|\s)#.*$") + + +def _gating_text(line: str) -> str: + """A step line reduced to the part whose exit status can actually reach the step. + + Two things are removed before the neutering patterns are applied, because both produced false + positives that would have been "fixed" by weakening a real gate: + + * **Command substitutions.** `claim="$(… | grep -oiE 'BACKLOG #[0-9]+' | head -1 || true)"` is the + idiomatic guard for `grep` exiting 1 on no-match under `set -euo pipefail`; without it the script + aborts on the ordinary "no claim in this PR" path. It cannot mask the step's status — the gating + decision is the explicit `exit 0` / `exit 1` further down. Flagging it suggested "confine it to + its own step", which is impossible for a variable assignment and would have meant deleting the + guard from a correct script. + * **Comments.** The rationale comments in these workflows quote the very idiom being prohibited, so + a raw scan reports the explanation as the offence — a detector counting itself. + + A standalone `gating_cmd || true`, which is the actual hazard, survives both strips and is caught. + """ + return _TRAILING_COMMENT.sub("", _SUBSTITUTION.sub("", line)) + def _required_jobs() -> dict[tuple[str, str], dict]: """Every job backing a required context, keyed by (workflow file, job key).""" @@ -173,6 +202,11 @@ def test_required_jobs_carry_no_continue_on_error() -> None: # contexts are ONE matrix job, so 12 contexts collapse to 10 jobs. Pinned rather than derived so # that a change in the collapse — a matrix split, or a context that quietly stops resolving — # forces a look here instead of passing on a self-consistent count. + # + # 13/11 since 2026-07-29, when backlog-hygiene was promoted to a required context. Promoting one + # brings its job under every rule in this module for the first time, which is the point: it + # immediately surfaced a `|| true` in that job (a false positive, as it turned out — see + # _gating_text — but the review it forced is exactly what promotion should trigger). print( f"[security-posture] examined {examined} distinct jobs backing " f"{len(required_contexts())} required contexts" @@ -205,10 +239,17 @@ def test_required_jobs_have_no_neutered_steps() -> None: if not run: continue scanned_steps += 1 - for pattern, label in _NEUTERING: - if pattern.search(run): - name = (step or {}).get("name") or "" - offenders.append(f"{wf}:{key} — step {name!r} contains {label}") + # Per LINE, against the gating text only — see _gating_text for why a whole-body scan + # reported two false positives, each of which would have been "fixed" by deleting a + # correct guard or a rationale comment. + for raw_line in run.splitlines(): + gating = _gating_text(raw_line) + for pattern, label in _NEUTERING: + if pattern.search(gating): + name = (step or {}).get("name") or "" + offenders.append( + f"{wf}:{key} — step {name!r} contains {label}: {raw_line.strip()!r}" + ) print(f"[security-posture] scanned {scanned_steps} run steps across required jobs") assert scanned_steps > 0, "scanned ZERO run steps — the parser stopped seeing steps, not a pass" assert not offenders, (