From c7f010fbbc78e76d4df973f907969dbeba33ba8b Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Thu, 6 Aug 2026 23:04:04 +0200 Subject: [PATCH] feat(guard): the changed-path classifier cannot fail open (#384) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Detect changed paths` decides whether 10 of the 18 required status checks run; the same logic cloned into proofs.yml gates an 11th. It could be made to answer "no code changed" two ways, and both had the error path produce the permissive verdict with the step exiting 0. `skipped` satisfies a required context, so a skipped required check and a passed one are the same green on the merge button. Mechanism A — the allow-list was scoped by prefix, not file type. `safety/.*` and `docs/.*` admitted anything under those trees including .rs and .sh. `docs/research/forge-replication/` already holds run_trial.sh, tasks.json, descriptor.sexp and descriptor.aadl, so this was one `git mv` from live rather than theoretical. Mechanism B — the error path failed open. A grep failure wrote nothing, `n_code` became empty, `[ "" -gt 0 ]` exited 2, and `set -e` is inert inside an `if`, so bash read that as false and took the `code=false` branch. The `|| true` was not removable: `grep -c` also exits 1 on a legitimate zero count, so success and failure arrived identically. Replaced by one script both workflows call. Every allow-list entry names its extensions; an unmatched path is code, because a classifier that cannot recognise a file has not shown it is inert. No error path emits a verdict at all — it raises, prints, and exits non-zero. The script appends to $GITHUB_OUTPUT itself. That is its only emission mode on purpose: `echo "code=$(script ...)"` would restore mechanism B verbatim in the glue — script dies, echo succeeds, `code=` lands empty, every `== 'true'` dependent skips, exit 0. Neither `changes` job now contains a subshell, `||`, `-gt` or `grep`. Exiting non-zero is safe only because both classifier jobs are themselves required contexts — read from the branch-protection API, not assumed. A failing `changes` job otherwise skips its dependents, and skipped satisfies a required context. Also verified rather than trusted: every consumer of `outputs.code` tests `== 'true'`, and python3 is present on [self-hosted, linux, x64, light] because ci.yml::fmt already runs check_fmt_workspaces.py there. One deliberate loosening: the two copies had already drifted — ci.yml gained `rivet.yaml` in #379 and proofs.yml never did. Single-sourcing gives the proofs side that entry, safe for the same reason #379 gave, and it is exactly the divergence single-sourcing exists to end. 25 self-test cases, run as the first step of both `changes` jobs. The detector is the property rather than the cases: for every directory prefix in the allow-list, no pattern may match /evil.rs, .sh, .toml or Makefile — so a future re-broadening fails without anyone remembering to add a case. Mutation-tested with 5 mutants, all caught, each by the test that should catch it; the harness asserts each substitution changed the file first, since a mutation that fails to apply reports as "not caught". Measured on real history: 7db7f0e6 (two artifact YAMLs) still emits code=false, so the classifier was tightened without becoming always-true; c8c0450e (8 files, 6 code) emits code=true. Refs #384. REQ-GUARD-GATE-EVIDENCE-002 (e). The self-test's fixture repo pins core.hooksPath=/dev/null. It is step 2 of a required context gating 11 others and it shells out to git init/commit, so a machine with a global core.hooksPath would run foreign hooks inside the fixture and turn that context red for every PR in the repo. Verified load-bearing: under a simulated hostile global hooksPath the unpinned copy fails 3 of 25, the pinned one passes 25. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 75 +++--- .github/workflows/proofs.yml | 36 +-- artifacts/verification.yaml | 66 +++++ tools/classify_changed_paths.py | 434 ++++++++++++++++++++++++++++++++ 4 files changed, 551 insertions(+), 60 deletions(-) create mode 100755 tools/classify_changed_paths.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93e7806..19d7915 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -41,50 +41,41 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 + # Runs FIRST, for the same reason as the other guardrail self-tests: a + # classifier whose own tests fail must not be trusted to decide which + # required checks run. + - name: Changed-path classifier self-test + run: tools/classify_changed_paths.py --self-test + + # The allow-list, the fail-closed behaviour and the rationale all live in + # tools/classify_changed_paths.py, which proofs.yml calls too — the two + # copies of this logic had already drifted (#379's `rivet.yaml` entry + # existed here and not there), which is why it is single-sourced now. + # + # This step is ONE command on purpose. The old body computed a count in a + # subshell and let `[ "$n_code" -gt 0 ]` decide; when grep failed for any + # reason the count was empty, the test exited 2, bash read that as false + # inside `if`, and the step emitted `code=false` and exited 0 — skipping + # 10 required checks (#384). The script therefore appends to + # $GITHUB_OUTPUT itself rather than returning a value for the shell to + # interpolate. Any `$(...)`, `||` or `-gt` reappearing in this step is + # that bug coming back. + # + # The two event values go through `env:` rather than being interpolated + # into the command text. Neither is attacker-controlled (`event_name` is a + # GitHub enum, `base.sha` a computed 40-hex sha), but `${{ }}` substitutes + # into the script *before* bash parses it, while an env var is only ever a + # value. Same reason the rest of this repo's workflows do it. - id: filter name: Classify diff as code vs artifacts/docs - run: | - set -euo pipefail - if [ "${{ github.event_name }}" != "pull_request" ]; then - echo "Non-PR event (${{ github.event_name }}) -> full suite." - echo "code=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - base="${{ github.event.pull_request.base.sha }}" - changed=$(git diff --name-only "$base"...HEAD) - echo "Changed files:" - printf '%s\n' "$changed" - if [ -z "$changed" ]; then - echo "Empty diff -> full suite (fail-safe)." - echo "code=true" >> "$GITHUB_OUTPUT" - exit 0 - fi - # Non-code allow-list: rivet artifacts, STPA files, docs, markdown. - # Any path outside it makes the WHOLE diff "code" — a PR that touches - # both artifacts and Rust must still run the full suite. - # Counted explicitly rather than relying on `grep -q` exit semantics, - # so the gate's decision is readable in the job log. - # - # `rivet.yaml` is on the list (#379) because it is a rivet artifact by - # every other statement in this file — the header above says "rivet - # artifacts", and the rivet-validate job says it catches regressions - # "in artifacts/, safety/stpa/, and rivet.yaml" — but it lives at the - # repo root, so `artifacts/.*` never matched it and a rivet.yaml-only - # PR ran the entire heavy suite. Safe to skip those jobs for it: - # nothing that compiles reads the file (no .rs, no build script, no - # Cargo.toml references it; only AGENTS.md, docs/, this workflow, and - # artifacts themselves), and it stays gated regardless because - # rivet-validate has no `changes` gate at all. - allow='^(artifacts/.*\.ya?ml|safety/.*|docs/.*|rivet\.yaml|[^/]*\.md|.*/.*\.md)$' - n_code=$(printf '%s\n' "$changed" | grep -cvE "$allow" || true) - echo "code-ish files in diff: $n_code" - if [ "$n_code" -gt 0 ]; then - echo "-> code change: running the full suite." - echo "code=true" >> "$GITHUB_OUTPUT" - else - echo "-> artifacts/docs only: skipping the heavy jobs." - echo "code=false" >> "$GITHUB_OUTPUT" - fi + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: >- + tools/classify_changed_paths.py + --event-name "$EVENT_NAME" + --base-sha "$BASE_SHA" + --github-output "$GITHUB_OUTPUT" # ── Fast checks ─────────────────────────────────────────────────────── # fmt / audit / deny / rivet-validate deliberately have NO `changes` gate: diff --git a/.github/workflows/proofs.yml b/.github/workflows/proofs.yml index fbfd303..15ef418 100644 --- a/.github/workflows/proofs.yml +++ b/.github/workflows/proofs.yml @@ -42,26 +42,26 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 + # This job gates the required `Lean proof typecheck (lake build)` context, + # so it is the 11th check the #384 fail-open could skip. It was a verbatim + # clone of ci.yml's classifier and had already drifted from it — ci.yml + # gained `rivet.yaml` in #379 and this copy never did. Both now call the + # same script; see tools/classify_changed_paths.py for the allow-list and + # the fail-closed rationale, including why picking up the `rivet.yaml` + # entry here is a deliberate (and safe) loosening. + - name: Changed-path classifier self-test + run: tools/classify_changed_paths.py --self-test + - id: filter name: Classify diff as code vs artifacts/docs - run: | - set -euo pipefail - if [ "${{ github.event_name }}" != "pull_request" ]; then - echo "code=true" >> "$GITHUB_OUTPUT"; exit 0 - fi - changed=$(git diff --name-only "${{ github.event.pull_request.base.sha }}"...HEAD) - printf '%s\n' "$changed" - if [ -z "$changed" ]; then - echo "code=true" >> "$GITHUB_OUTPUT"; exit 0 - fi - allow='^(artifacts/.*\.ya?ml|safety/.*|docs/.*|[^/]*\.md|.*/.*\.md)$' - n_code=$(printf '%s\n' "$changed" | grep -cvE "$allow" || true) - echo "code-ish files in diff: $n_code" - if [ "$n_code" -gt 0 ]; then - echo "code=true" >> "$GITHUB_OUTPUT" - else - echo "code=false" >> "$GITHUB_OUTPUT" - fi + env: + EVENT_NAME: ${{ github.event_name }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: >- + tools/classify_changed_paths.py + --event-name "$EVENT_NAME" + --base-sha "$BASE_SHA" + --github-output "$GITHUB_OUTPUT" lean: name: Lean proof typecheck (lake build) diff --git a/artifacts/verification.yaml b/artifacts/verification.yaml index 6ed038f..f18acff 100644 --- a/artifacts/verification.yaml +++ b/artifacts/verification.yaml @@ -4169,6 +4169,72 @@ artifacts: release: v0.36.0 tags: [process, guardrail, ci, tooling] + - id: TEST-GUARD-CHANGED-PATHS + type: feature + title: The changed-path classifier cannot answer "no code" by failing + description: > + Verifies REQ-GUARD-GATE-EVIDENCE-002 (e). Closes #384. + + `tools/classify_changed_paths.py --self-test` runs 25 cases and is the + FIRST step of the `changes` job in both `ci.yml` and `proofs.yml` — a + classifier whose own tests fail must not be trusted to decide which + required checks run. + + Both original failure modes are regression cases, at the unit level and + end-to-end over a real `git diff`: + + MECHANISM A — the allow-list was scoped by PREFIX. `safety/.*` and + `docs/.*` admitted any file under those trees, `.rs` and `.sh` included. + Every entry is now type-scoped, and `docs/research/forge-replication/` + (which already holds `run_trial.sh`, `tasks.json`, `descriptor.sexp` and + `descriptor.aadl`) is classified as code. + + MECHANISM B — the error path failed OPEN. A `grep` failure left the count + empty, `[ "" -gt 0 ]` exited 2, `set -e` is inert inside an `if`, so bash + read that as false and emitted `code=false` with the step exiting 0. Five + induced failures — bad base sha, empty base sha, unwritable output, + missing arguments — now each assert non-zero AND that no verdict was + written. + + The case that makes it a detector rather than an assertion is the + PROPERTY, not the cases: for every directory prefix in the allow-list, + no pattern may match `/evil.rs`, `.sh`, `.toml`, `Makefile`. An + editor who re-broadens an entry back to prefix scope fails without anyone + having remembered to add a case for them. + + MUTATION-TESTED 2026-08-06, 5 mutants, all caught, each by the test that + should catch it: unknown→non-code, `emit()` ignoring its argument, + restoring the #384 exception handler, re-broadening `safety/`, re-adding + `docs/.*`. The harness asserts each substitution changed the file before + drawing a conclusion — a mutation that silently fails to apply reports as + "not caught" and is worthless. + + MEASURED on real history: commit 7db7f0e6 (two artifact YAMLs) still + emits `code=false`, so the fix tightened the classifier without making it + uselessly always-true; commit c8c0450e (8 files, 6 code) emits + `code=true`. + + The script owns `$GITHUB_OUTPUT` and appends to it directly. That is its + only emission mode on purpose: `echo "code=$(script ...)"` would put + mechanism B back in the glue verbatim — script dies, `echo` succeeds, + `code=` lands empty, every `== 'true'` dependent skips, exit 0. + + SAFETY ARGUMENT, and its dependency: exiting non-zero is safe only + because `Detect changed paths` and `Detect changed paths (proofs)` are + themselves required contexts (read from the branch-protection API, not + assumed) — a failing `changes` job otherwise skips its dependents, and + `skipped` satisfies a required context. If either is ever dropped from + the required set this design becomes unsafe; REQ-GUARD-GATE-EVIDENCE-001 + and `check_required_contexts.py` are what keep that from happening + quietly. + + NOT claimed: that skipping is correct for any particular file. This only + ensures a skip is caused by a match against a declared, type-scoped + pattern and never by a crash. + status: implemented + release: v0.36.0 + tags: [process, guardrail, ci, tooling] + - id: TEST-GUARD-RELEASE-PLANE type: feature title: Release-plane guardrail is proven able to fail before it is allowed to judge diff --git a/tools/classify_changed_paths.py b/tools/classify_changed_paths.py new file mode 100755 index 0000000..3b5a51f --- /dev/null +++ b/tools/classify_changed_paths.py @@ -0,0 +1,434 @@ +#!/usr/bin/env python3 +"""Decide whether a PR diff is code, and never answer "no" by accident. + +REQ-GUARD-GATE-EVIDENCE-002 (e). Closes #384. + +WHY THIS EXISTS +=============== + +`Detect changed paths` gates 10 of the 18 required status checks; the identical +logic cloned into `proofs.yml` gates an 11th (`Lean proof typecheck`). It could +be made to answer "no code changed" two different ways, and in both the error +path produced the *permissive* verdict with the step exiting 0. + +`skipped` satisfies a required context. A skipped required check and a passed +one are the same green on the merge button. + +Mechanism A — the allow-list was scoped by PREFIX, not by file type +------------------------------------------------------------------ + + allow='^(artifacts/.*\\.ya?ml|safety/.*|docs/.*|rivet\\.yaml|[^/]*\\.md|.*/.*\\.md)$' + +`artifacts/` and the two `*.md` entries name the extensions they admit. +`safety/.*` and `docs/.*` do not — they admit **anything** under those trees, +including `.rs`, `.sh`, `.toml`. `docs/research/forge-replication/` already +holds `run_trial.sh`, `tasks.json`, `descriptor.sexp` and `descriptor.aadl`, so +this was one `git mv` away from live rather than purely theoretical. + +Mechanism B — the error path failed OPEN +---------------------------------------- + + n_code=$(printf '%s\\n' "$changed" | grep -cvE "$allow" || true) + if [ "$n_code" -gt 0 ]; then ... else echo "code=false" ... + +If `grep` failed for any reason other than "zero matches" — an unbalanced group +after an edit, `grep` missing from `PATH` — it wrote nothing, `n_code` became +the empty string, and `[ "" -gt 0 ]` exited 2. Inside an `if` condition `set -e` +is inert, so bash read that non-zero as *false* and took the `code=false` +branch. Two error messages printed to the log and the step still exited 0, +having skipped the suite. + +The `|| true` was not simply removable: `grep -c` exits 1 on a zero count, which +is the legitimate "everything was allow-listed" outcome. That is the trap — the +suppression was load-bearing for the success path and catastrophic for the +failure path, because both arrive as a non-zero exit. + +WHAT REPLACES IT +================ + +One script, called from both workflows. + +* **Type-scoped allow-list.** Every entry names its extensions. A path that + matches nothing is CODE. Unknown is not a licence to skip — a classifier that + cannot recognise a file has not shown it is inert. +* **No error path can emit `code=false`.** Any failure raises, `main` catches, + prints, and exits non-zero *without emitting a verdict at all*. +* **The script owns `$GITHUB_OUTPUT`.** It opens and appends the file itself. + This is the only emission mode on purpose. Had the workflow done + + echo "code=$(python3 tools/classify_changed_paths.py ...)" >> "$GITHUB_OUTPUT" + + mechanism B would be back verbatim in the glue: the script dies, `echo` + succeeds, `code=` lands empty, every `== 'true'` dependent skips, exit 0. The + decision and its emission must not be separated by a shell that fails open. +* **Every file prints with its verdict**, on every path including success, so + "these 4 files were skipped" is readable rather than inferred from silence. + +WHY EXITING NON-ZERO IS SAFE HERE, AND WHY THAT IS NOT OBVIOUS +-------------------------------------------------------------- + +A failing `changes` job means its dependents (`if: needs.changes.outputs.code == +'true'`) are **skipped**, and skipped satisfies a required context. So exiting +non-zero would normally be *worse* than failing open. + +It is safe only because `Detect changed paths` and `Detect changed paths +(proofs)` are themselves required contexts (verified against the branch +protection API, not assumed). The classifier going red therefore blocks the +merge on its own. Two further facts were checked rather than trusted: + +* every consumer of `outputs.code` tests `== 'true'`, so no job runs on the + inverse and an absent output cannot trigger anything; +* `python3` is present on `[self-hosted, linux, x64, light]` — `ci.yml::fmt` + already runs `check_fmt_workspaces.py` on that exact label. + +**If `Detect changed paths` is ever removed from the required contexts, this +file becomes unsafe.** `check_required_contexts.py` is what keeps that honest. + +ONE DELIBERATE BEHAVIOUR CHANGE +------------------------------- + +The two copies had already drifted: `ci.yml` gained `rivet\\.yaml` (#379), +`proofs.yml` never did. Single-sourcing gives the proofs side that entry, so a +rivet.yaml-only PR now also skips the Lean job. That is safe for the same reason +#379 gave for the CI side — nothing under `proofs/` reads `rivet.yaml` — and it +is exactly the class of divergence single-sourcing exists to end. It is a +loosening, so it is called out here rather than absorbed silently. + +NOT CLAIMED: that skipping is right for any particular file. This only ensures +the *reason* for a skip is a match against a declared, type-scoped pattern, and +never a crash. +""" + +from __future__ import annotations + +import argparse +import io +import re +import subprocess +import sys +import tempfile +from pathlib import Path + +# The allow-list: a changed file is non-code only if it FULL-matches one of +# these. Every entry names the extensions it admits — a bare directory prefix is +# mechanism A and must never reappear here. `_no_pattern_admits_code` in the +# self-test enforces that mechanically for any pattern added later. +_ALLOW: list[tuple[str, str]] = [ + ("rivet artifact YAML", r"artifacts/(?:.*/)?[^/]+\.ya?ml"), + ("STPA safety YAML", r"safety/(?:.*/)?[^/]+\.ya?ml"), + # Root-level rivet manifest (#379). Not under artifacts/, so it needs its + # own entry; nothing that compiles reads it. + ("rivet manifest", r"rivet\.yaml"), + # Markdown anywhere. Checked: no .md in the tree is consumed as build or + # test data — the `docs/design/*.md` filters in spar-codegen's golden tests + # match GENERATED output paths, not repo files. + ("markdown", r"(?:.*/)?[^/]+\.md"), +] + +_ALLOW_RE: list[tuple[str, re.Pattern[str]]] = [(k, re.compile(p)) for k, p in _ALLOW] + + +def classify_one(path: str) -> str | None: + """Return the allow-list label that admits `path`, or None if it is code.""" + for label, rx in _ALLOW_RE: + if rx.fullmatch(path): + return label + return None + + +def classify(paths: list[str]) -> tuple[bool, list[tuple[str, str | None]]]: + """(is_code, [(path, label_or_None)]). is_code once ANY path is unmatched.""" + rows = [(p, classify_one(p)) for p in paths] + return any(label is None for _, label in rows), rows + + +def changed_paths(base: str, head: str, cwd: str | None = None) -> list[str]: + """`git diff --name-only base...head`. Raises — never returns a short list.""" + proc = subprocess.run( + ["git", "diff", "--name-only", f"{base}...{head}"], + capture_output=True, + text=True, + cwd=cwd, + ) + if proc.returncode != 0: + raise RuntimeError( + f"git diff {base}...{head} failed (exit {proc.returncode}): " + f"{proc.stderr.strip() or '(no stderr)'}" + ) + return [line for line in proc.stdout.splitlines() if line.strip()] + + +def emit(is_code: bool, github_output: str) -> None: + """Append the verdict. The ONLY way a verdict leaves this process.""" + with open(github_output, "a", encoding="utf-8") as fh: + fh.write(f"code={'true' if is_code else 'false'}\n") + + +def run( + event_name: str, + base_sha: str, + head: str, + github_output: str, + cwd: str | None = None, + out=sys.stdout, +) -> int: + print("== changed-path classifier ==", file=out) + print(f"event: {event_name}", file=out) + + if event_name != "pull_request": + print(f"non-PR event -> full suite.", file=out) + emit(True, github_output) + return 0 + + if not base_sha: + raise RuntimeError( + "pull_request event with an empty base sha — the diff cannot be " + "computed, so no file can be shown to be inert." + ) + + paths = changed_paths(base_sha, head, cwd=cwd) + if not paths: + # A PR with a genuinely empty diff is odd; so is a base sha that makes + # the diff look empty. Both get the full suite. + print("empty diff -> full suite (fail-safe).", file=out) + emit(True, github_output) + return 0 + + is_code, rows = classify(paths) + for path, label in rows: + print(f" {'CODE' if label is None else 'skip'} {path}" + + (f" [{label}]" if label else ""), file=out) + + n_code = sum(1 for _, label in rows if label is None) + print(f"\n{len(rows)} changed, {n_code} classified CODE.", file=out) + print(f"-> code={'true' if is_code else 'false'}", file=out) + emit(is_code, github_output) + return 0 + + +# -------------------------------------------------------------------------- +# self-test +# -------------------------------------------------------------------------- + +# Extensions that must never be admitted by a directory-scoped pattern. +_HOSTILE = ["evil.rs", "evil.sh", "Cargo.toml", "evil.py", "evil.json", "Makefile"] + + +def _allow_prefixes() -> list[str]: + """Literal leading directory of each pattern, e.g. 'artifacts', 'safety'.""" + out = set() + for _, pattern in _ALLOW: + m = re.match(r"^([A-Za-z0-9_.-]+)/", pattern) + if m: + out.add(m.group(1)) + return sorted(out) + + +def self_test() -> int: + passed = failed = 0 + + def ok(cond: bool, desc: str, detail: str = "") -> None: + nonlocal passed, failed + if cond: + passed += 1 + print(f" ok {desc}") + else: + failed += 1 + print(f" FAIL {desc}" + (f"\n {detail}" if detail else "")) + + print("classify_changed_paths self-test") + + # -- pure classification ------------------------------------------------- + def verdict(paths): + return classify(paths)[0] + + ok(verdict(["docs/architecture.md"]) is False, "a docs .md alone is not code") + ok(verdict(["README.md"]) is False, "a root .md alone is not code") + ok(verdict(["artifacts/requirements.yaml"]) is False, "artifact YAML is not code") + ok(verdict(["safety/stpa/hazards.yaml"]) is False, "STPA YAML is not code") + ok(verdict(["rivet.yaml"]) is False, "rivet.yaml is not code (#379)") + + # MECHANISM A. These are the exact paths from #384's executed repro. + ok(verdict(["docs/nested/evil.rs"]) is True, + "REGRESSION #384-A: a .rs under docs/ is CODE") + ok(verdict(["safety/scripts/deploy.sh"]) is True, + "REGRESSION #384-A: a .sh under safety/ is CODE") + # The live instance of the same hole. + ok(verdict(["docs/research/forge-replication/run_trial.sh"]) is True, + "the real docs/research/*.sh in the tree is CODE") + ok(verdict(["docs/research/forge-replication/descriptor.aadl"]) is True, + "a .aadl under docs/ is CODE (spar parses .aadl)") + ok(verdict(["artifacts/evil.rs"]) is True, + "a .rs under artifacts/ is CODE (that entry was already type-scoped)") + + # Mixed diffs must run the suite — one code file poisons the whole diff. + ok(verdict(["artifacts/requirements.yaml", "crates/spar-cli/src/main.rs"]) is True, + "artifacts + code together is CODE") + ok(verdict(["crates/spar-analysis/src/scheduling.rs"]) is True, "plain .rs is CODE") + ok(verdict([".github/workflows/ci.yml"]) is True, + "a workflow YAML is CODE (only artifacts/ and safety/ YAML are exempt)") + + # -- PROPERTY: no allow-list pattern admits code ------------------------- + # A future editor who re-broadens `safety/.*\.ya?ml` back to `safety/.*` + # fails here without having to remember to add a case. + bad = [] + for prefix in _allow_prefixes(): + for name in _HOSTILE: + for path in (f"{prefix}/{name}", f"{prefix}/deep/nested/{name}"): + label = classify_one(path) + if label is not None: + bad.append(f"{path} admitted by [{label}]") + ok(not bad, + f"PROPERTY: no allow pattern admits code under {_allow_prefixes()}", + "; ".join(bad[:4])) + + # -- emission honours its argument --------------------------------------- + # Without this, an emit() that ignored `is_code` would pass every test + # above: distinct inputs must produce distinct outputs. + with tempfile.TemporaryDirectory() as td: + f_true, f_false = Path(td) / "t", Path(td) / "f" + emit(True, str(f_true)) + emit(False, str(f_false)) + ok(f_true.read_text().strip() == "code=true", "emit(True) writes code=true") + ok(f_false.read_text().strip() == "code=false", "emit(False) writes code=false") + + # -- PROPERTY: no error path emits code=false ---------------------------- + # This is the requirement's own detector turned on this script: a guard + # whose ERROR path yields its IDEAL reading. Every induced failure must + # exit non-zero AND leave no verdict behind. + def induce(desc: str, argv_fn, expect_written: str | None = None) -> None: + nonlocal passed, failed + with tempfile.TemporaryDirectory() as td: + gh = Path(td) / "gh_output" + gh.write_text("") + argv = argv_fn(str(gh)) + err = io.StringIO() + real_err, sys.stderr = sys.stderr, err + try: + try: + rc = main(argv) + except SystemExit as e: # argparse + rc = e.code if isinstance(e.code, int) else 1 + finally: + sys.stderr = real_err + written = gh.read_text() if gh.exists() else "" + if expect_written is None: + good = rc != 0 and "code=false" not in written + detail = f"rc={rc}, wrote {written!r}" + else: + good = rc == 0 and written.strip() == expect_written + detail = f"rc={rc}, wrote {written!r}" + if good: + passed += 1 + print(f" ok {desc}") + else: + failed += 1 + print(f" FAIL {desc}\n {detail}") + + induce("ERROR: nonexistent base sha -> non-zero, no code=false", + lambda gh: ["--event-name", "pull_request", "--base-sha", "0" * 40, + "--github-output", gh]) + induce("ERROR: empty base sha on a PR -> non-zero, no code=false", + lambda gh: ["--event-name", "pull_request", "--base-sha", "", + "--github-output", gh]) + induce("ERROR: unwritable --github-output -> non-zero, no code=false", + lambda _: ["--event-name", "push", "--github-output", + "/nonexistent-dir-84f2/out"]) + induce("ERROR: missing --github-output -> non-zero, no code=false", + lambda _: ["--event-name", "push"]) + induce("ERROR: missing --event-name -> non-zero, no code=false", + lambda gh: ["--github-output", gh]) + # ...and the success control, so the above is not passing because the tool + # is simply always broken. + induce("push event emits code=true and exits 0", + lambda gh: ["--event-name", "push", "--github-output", gh], + expect_written="code=true") + + # -- end-to-end over a real git diff -------------------------------------- + # Closes the seam between classify() and emit(): a run() that emitted a + # constant would satisfy every test above. + def e2e(desc: str, files: list[str], want: str) -> None: + nonlocal passed, failed + with tempfile.TemporaryDirectory() as td: + g = ["git", "-C", td, "-c", "user.name=t", "-c", "user.email=t@t", + # A throwaway fixture repo, not a spar commit: signing is + # switched OFF in this repo's own config rather than skipped on + # the commit, and identity is supplied so a runner with no + # global git config still works. + "-c", "commit.gpgsign=false", + # This self-test is step 2 of a required context that gates 11 + # others. A machine with a global `core.hooksPath` would + # otherwise run foreign hooks inside the fixture repo and could + # stall CI for every PR in the repo. + "-c", "core.hooksPath=/dev/null"] + try: + subprocess.run(g + ["init", "-q", "-b", "main"], check=True, + capture_output=True) + Path(td, "seed").write_text("seed") + subprocess.run(g + ["add", "seed"], check=True, capture_output=True) + subprocess.run(g + ["commit", "-qm", "seed"], check=True, + capture_output=True) + base = subprocess.run(g + ["rev-parse", "HEAD"], check=True, + capture_output=True, text=True).stdout.strip() + for rel in files: + p = Path(td, rel) + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text("x") + subprocess.run(g + ["add", rel], check=True, capture_output=True) + subprocess.run(g + ["commit", "-qm", "change"], check=True, + capture_output=True) + except (subprocess.CalledProcessError, OSError) as e: + failed += 1 + print(f" FAIL {desc}: fixture repo could not be built: {e}") + return + gh = Path(td, "gh_output") + gh.write_text("") + rc = run("pull_request", base, "HEAD", str(gh), cwd=td, out=io.StringIO()) + written = gh.read_text().strip() + if rc == 0 and written == want: + passed += 1 + print(f" ok {desc}") + else: + failed += 1 + print(f" FAIL {desc}: rc={rc}, wrote {written!r}, want {want!r}") + + e2e("E2E: a docs-only diff really does emit code=false", + ["docs/guide.md"], "code=false") + e2e("E2E: a .rs diff emits code=true", ["crates/x/src/lib.rs"], "code=true") + e2e("E2E: REGRESSION #384-A end-to-end — docs/*.rs emits code=true", + ["docs/nested/evil.rs"], "code=true") + + print(f"\n{passed} passed, {failed} failed") + return 1 if failed else 0 + + +def main(argv: list[str] | None = None) -> int: + argv = list(sys.argv[1:] if argv is None else argv) + if "--self-test" in argv: + return self_test() + + ap = argparse.ArgumentParser(description=__doc__.split("\n")[0]) + ap.add_argument("--event-name", required=True, help="github.event_name") + ap.add_argument("--base-sha", default="", + help="github.event.pull_request.base.sha") + ap.add_argument("--head", default="HEAD") + # Required, and the only emission mode. See the module docstring: an + # optional stdout mode is a mode a workflow will interpolate through a + # shell, which is how #384 mechanism B comes back. + ap.add_argument("--github-output", required=True, + help="path to $GITHUB_OUTPUT; this script appends to it") + ap.add_argument("--self-test", action="store_true") + a = ap.parse_args(argv) + + try: + return run(a.event_name, a.base_sha, a.head, a.github_output) + except Exception as exc: # noqa: BLE001 — every failure must land here + print(f"::error::changed-path classifier failed: {exc}", file=sys.stderr) + print("::error::No verdict was emitted. This job is RED and it is a " + "required context, so the merge is blocked rather than the suite " + "silently skipped (#384).", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main())