diff --git a/.github/workflows/required-workflow-state.yml b/.github/workflows/required-workflow-state.yml new file mode 100644 index 00000000..cb43176d --- /dev/null +++ b/.github/workflows/required-workflow-state.yml @@ -0,0 +1,58 @@ +name: Required workflow state + +# Every REQUIRED status check must belong to a workflow GitHub will actually run. +# +# THE DEFECT THIS EXISTS FOR — measured on the sibling vault repo (wshallwshall/MessageFoundry), +# 2026-07-30: 10 required contexts whose workflows were ALL `disabled_manually`. A disabled workflow +# never dispatches, so those contexts never reported — NO pull request could merge, and every merge +# there had silently been riding admin bypass. Nobody noticed, because the symptom presents as "CI is +# stuck", not as "branch protection is misconfigured". +# +# `tests/test_required_contexts.py` cannot see this: a disabled workflow keeps its file and job name on +# disk, so resolving a context against YAML passes in the healthy AND the broken case. Workflow `state` +# is server-side, so this has to be an API check. +# +# SCHEDULED, not per-PR, deliberately. A workflow disabled AFTER the last pull request is invisible to +# any per-PR check — there is no PR to run it on. The failure arrives while the repo is idle, which is +# exactly when nobody is looking. +# +# It checks REACHABILITY (can the context ever report?), not outcome. Whether a check passes is CI's +# job; whether it is capable of running at all is this one's. +on: + schedule: + # 07:00 UTC — an hour after the nightly CI cron, so a workflow disabled overnight is reported the + # same morning rather than a day later. + - cron: "0 7 * * *" + workflow_dispatch: + # Also on a PR that edits the required set or the workflows it points at: this is the one moment a + # human is actively changing the mapping, and a typo'd context is cheapest to catch right then. + pull_request: + paths: + - ".github/required-contexts.txt" + - ".github/workflows/**" + +permissions: + contents: read + +jobs: + reachable: + name: required contexts belong to active workflows + runs-on: ubuntu-latest + permissions: + contents: read + actions: read # read workflow `state`; NOT the admin scope branch protection would need + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: "3.14" + - name: Install PyYAML (the resolver parses the workflow files) + run: | + # Hash-pinned from the CI toolchain lock, like every other scanner install (ADR 0034 §3). + python -m pip install --require-hashes -r ci/locks/ci-scanners.lock + - name: Reconcile required contexts against workflow state + env: + GH_TOKEN: ${{ github.token }} + run: python scripts/ci/check_required_workflow_state.py --repo "$GITHUB_REPOSITORY" diff --git a/scripts/ci/check_required_workflow_state.py b/scripts/ci/check_required_workflow_state.py new file mode 100644 index 00000000..bce9454c --- /dev/null +++ b/scripts/ci/check_required_workflow_state.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every REQUIRED status check must belong to a workflow GitHub will actually run. + +THE DEFECT THIS EXISTS FOR — measured, on a sibling repo, 2026-07-30. The private vault repo +(``wshallwshall/MessageFoundry``) had 10 required contexts whose workflows were all +``disabled_manually``. A disabled workflow never dispatches, so those contexts never reported: NO pull +request could ever merge, and every merge there had silently been riding admin bypass. It sat like +that long enough for nobody to notice, because the symptom presents to a human as *"CI is stuck"* — +not as *"branch protection is misconfigured"*. + +WHY THE EXISTING GUARD CANNOT SEE IT. ``tests/test_required_contexts.py`` resolves each required +context against the job names in ``.github/workflows/``. A ``disabled_manually`` workflow keeps its +file and its job name on disk, so that check passes cheerfully while the context can never report. The +property it measures — "a job with this name exists in YAML" — is true in both the healthy case and the +broken one. That is the same defect shape as a CI monitor polling for "nothing pending": sound about +the thing it looks at, blind to the thing that can fail. Workflow ``state`` is server-side and +invisible to any file-based test, which is why this lives in a scheduled job rather than in pytest. + +WHY SCHEDULED AND NOT PER-PR. A workflow disabled *after* the last pull request is invisible to any +per-PR check — there is no PR to run it on. The failure arrives while the repository is idle, which is +exactly when nobody is looking. A daily sweep is the only shape that catches it. + +SCOPE. This checks REACHABILITY (can the context ever report?), not outcome. Whether a check passes is +CI's job; whether it is capable of running at all is this one's. + +USAGE + python scripts/ci/check_required_workflow_state.py # uses gh's auth + python scripts/ci/check_required_workflow_state.py --repo owner/name + python scripts/ci/check_required_workflow_state.py --states-json states.json # offline/testing +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from pathlib import Path + +_ROOT = Path(__file__).resolve().parents[2] + +# Reuse the SAME resolver the doc-drift tests use, rather than re-deriving context -> workflow here. +# A second implementation of that mapping is exactly the drift this repo keeps writing parity tests to +# catch (ruff/bandit scan scope, the required-set prose). One resolver, two callers. +sys.path.insert(0, str(_ROOT)) +from tests._workflow_contexts import required_contexts, resolve # noqa: E402 + +#: GitHub reports a runnable workflow as ``active``. Everything else -- ``disabled_manually``, +#: ``disabled_inactivity`` (60 days idle on a fork/scheduled-only repo), ``disabled_fork`` -- means it +#: will not dispatch, so a required context it owns can never report. +_RUNNABLE = "active" + + +def _workflow_states(repo: str | None, states_json: Path | None) -> dict[str, str]: + """``{workflow filename: state}`` for every workflow in the repo. + + Keyed on the FILENAME rather than the display name: the display name is what ``workflow_run`` + matches on, but a required context resolves to a FILE, and two workflows may legitimately share a + display name (this repo has two called "CodeQL"). + """ + if states_json is not None: + payload = json.loads(states_json.read_text(encoding="utf-8")) + else: + cmd = ["gh", "api", "--paginate"] + cmd.append( + f"repos/{repo}/actions/workflows" if repo else "repos/{owner}/{repo}/actions/workflows" + ) + # B603 asks whether untrusted input reaches a subprocess. It cannot here: argv is a fixed + # literal list, there is no shell, and the only variable element is `--repo`, an operator-typed + # CLI argument on a CI runner — not message, config, or network data. Annotated per-line with a + # reason, the posture security.yml's bandit notes require. + out = subprocess.run( # noqa: S603 # nosec B603 — fixed argv, no shell, operator-supplied repo + cmd, capture_output=True, text=True, timeout=120 + ) + if out.returncode != 0: + raise RuntimeError(f"gh api failed ({out.returncode}): {out.stderr.strip()[:400]}") + payload = json.loads(out.stdout) + workflows = payload.get("workflows", payload if isinstance(payload, list) else []) + return {Path(str(w.get("path", ""))).name: str(w.get("state", "")) for w in workflows} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter + ) + parser.add_argument("--repo", default=None, help="owner/name; defaults to gh's current repo") + parser.add_argument( + "--states-json", type=Path, default=None, help="a saved API payload (testing)" + ) + args = parser.parse_args(argv) + + contexts = required_contexts() + if not contexts: + print( + "::error::.github/required-contexts.txt parsed to ZERO contexts — the format changed under " + "the parser. That is a broken check, not a clean sweep.", + file=sys.stderr, + ) + return 2 + + try: + states = _workflow_states(args.repo, args.states_json) + except (RuntimeError, json.JSONDecodeError, subprocess.SubprocessError, OSError) as exc: + # FAIL CLOSED. "We could not read workflow state" must never read as "every workflow is fine" — + # that is the same blindness this script exists to catch, one level up. + print( + f"::error::could not read workflow state ({exc!r}). Treating as a FAILURE.", + file=sys.stderr, + ) + return 2 + if not states: + print( + "::error::the API returned ZERO workflows — refusing to report success.", + file=sys.stderr, + ) + return 2 + + unreachable: list[str] = [] + unresolved: list[str] = [] + checked = 0 + for ctx in contexts: + where = resolve(ctx) + if where is None: + unresolved.append(ctx) + continue + workflow = where[0] + state = states.get(workflow) + if state is None: + unresolved.append(f"{ctx} (workflow {workflow} not present on the server)") + continue + checked += 1 + if state != _RUNNABLE: + unreachable.append(f"{ctx} -> {workflow} [state={state}]") + + # Liveness receipt: report what was EXAMINED. "no unreachable contexts" and "nothing was checked" + # are otherwise indistinguishable from the exit code. + print(f"required-workflow-state: checked {checked} of {len(contexts)} required contexts") + + if unresolved: + for item in unresolved: + print( + f"::error::required context resolves to no runnable workflow: {item}", + file=sys.stderr, + ) + return 1 + + if unreachable: + # THE VAULT SHAPE, called out by name. When EVERY required context is unreachable, no pull + # request can merge at all, and the only visible symptom is that PRs hang — which reads as + # flakiness, not misconfiguration. Say the diagnosis out loud so nobody spends a day on it. + if len(unreachable) == checked: + print( + "::error::EVERY required context belongs to a non-active workflow. NO pull request can " + "merge — each required check will hang as 'Expected — waiting for status to be " + "reported' forever. This presents as 'CI is stuck'; it is branch protection pointing at " + "workflows GitHub will not run. Measured in this exact state on the vault repo " + "(2026-07-30), where every merge had silently been riding admin bypass.", + file=sys.stderr, + ) + for item in unreachable: + print(f"::error::{item}", file=sys.stderr) + print( + "\nRe-enable the workflow (`gh workflow enable `), or remove the context from branch " + "protection AND .github/required-contexts.txt — deliberately, in a reviewed diff.", + file=sys.stderr, + ) + return 1 + + print(f"required-workflow-state: all {checked} required contexts belong to active workflows.") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/_workflow_contexts.py b/tests/_workflow_contexts.py index 51d41895..3772090b 100644 --- a/tests/_workflow_contexts.py +++ b/tests/_workflow_contexts.py @@ -22,13 +22,25 @@ class of bug both suites exist to catch. from pathlib import Path from typing import Any -import pytest - -# The importorskip lives HERE rather than in each importing test module. Done there, it would be a -# statement before the `from tests._workflow_contexts import ...` line and every caller would need an -# E402 dance; done here, both suites keep ordinary top-of-file imports and PyYAML stays optional -# exactly as tests/test_lint_scope_parity.py treats it. -yaml = pytest.importorskip("yaml") +# PyYAML stays OPTIONAL (as tests/test_lint_scope_parity.py treats it), and the skip lives HERE rather +# than in each importing test module: done there it would sit before the +# `from tests._workflow_contexts import ...` line and every caller would need an E402 dance. +# +# Import yaml DIRECTLY first, and only fall back to pytest's importorskip when it is genuinely absent. +# This module is no longer test-only — scripts/ci/check_required_workflow_state.py imports the same +# resolver so there is ONE context->workflow mapping rather than two that drift. An unconditional +# `import pytest` at module scope made that impossible: the CI job installs the scanner lock, not the +# test toolchain, so the script died on `ModuleNotFoundError: No module named 'pytest'` while passing +# locally, where a dev venv has pytest. Measured on PR #76. +# +# Behaviour under pytest is unchanged: with PyYAML present the try succeeds (pytest is never imported +# here); with it absent the importorskip still turns the whole importing module into a SKIP. +try: + import yaml +except ModuleNotFoundError: # pragma: no cover - exercised only on a venv without PyYAML + import pytest + + yaml = pytest.importorskip("yaml") ROOT = Path(__file__).resolve().parents[1] WORKFLOWS = ROOT / ".github" / "workflows" diff --git a/tests/test_required_workflow_state.py b/tests/test_required_workflow_state.py new file mode 100644 index 00000000..ad2ca373 --- /dev/null +++ b/tests/test_required_workflow_state.py @@ -0,0 +1,208 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Tests for the required-context reachability check. + +THE DEFECT IT GUARDS — measured on the sibling vault repo (``wshallwshall/MessageFoundry``), +2026-07-30: 10 required contexts whose workflows were all ``disabled_manually``. A disabled workflow +never dispatches, so NO pull request could merge and every merge had silently been riding admin +bypass — invisible because the symptom reads as "CI is stuck", not "protection is misconfigured". + +``tests/test_required_contexts.py`` cannot catch that: a disabled workflow keeps its file and job name +on disk, so resolving a context against YAML passes in the healthy *and* the broken case. + +WHY THESE TESTS DRIVE THE REAL ``main()``. Every case below runs the shipped entry point through its +``--states-json`` seam rather than re-implementing the rule. A guard whose test asserts a local copy of +the logic is a guard nobody has run — and this repo has been bitten by exactly that. + +MEASUREMENT, DATED so it cannot age into a false claim: on **2026-07-30** MEFORORG had 19 workflows, +all ``active``, and all 13 required contexts resolved to active workflows. The hazard is real but was +not live here on that date. ``test_the_live_repo_is_currently_clean`` is deliberately NOT part of this +module — that would make the suite depend on the network and on a mutable server state. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(_ROOT)) + +from scripts.ci.check_required_workflow_state import main # noqa: E402 +from tests._workflow_contexts import required_contexts, resolve # noqa: E402 + + +def _required_workflow_files() -> set[str]: + """The workflow FILES that own a required context — resolved by the shipped resolver.""" + out = set() + for ctx in required_contexts(): + where = resolve(ctx) + assert where is not None, ( + f"required context {ctx!r} resolves to no job. Fix that first — " + "tests/test_required_contexts.py owns that assertion." + ) + out.add(where[0]) + return out + + +def _payload(states: dict[str, str]) -> dict: + """An /actions/workflows payload with the given ``{filename: state}``.""" + return { + "total_count": len(states), + "workflows": [ + {"id": i, "name": f, "path": f".github/workflows/{f}", "state": s} + for i, (f, s) in enumerate(sorted(states.items()), start=1) + ], + } + + +def _write(tmp_path: Path, states: dict[str, str]) -> Path: + p = tmp_path / "states.json" + p.write_text(json.dumps(_payload(states)), encoding="utf-8") + return p + + +def test_all_active_passes(tmp_path: Path) -> None: + """The healthy case — and the baseline the failure cases are measured against.""" + states = dict.fromkeys(_required_workflow_files(), "active") + assert main(["--states-json", str(_write(tmp_path, states))]) == 0 + + +def test_one_disabled_workflow_is_caught( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A single disabled workflow makes its context permanently unreportable.""" + files = sorted(_required_workflow_files()) + assert len(files) >= 2, "expected the required set to span at least two workflows" + states = dict.fromkeys(files, "active") + states[files[0]] = "disabled_manually" + + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + err = capsys.readouterr().err + assert files[0] in err and "disabled_manually" in err, err + # A single disabled workflow is NOT the vault shape; the catastrophic message must not fire. + assert "EVERY required context" not in err, ( + "the all-unreachable message fired for a single disabled workflow — it would cry wolf and " + "train the reader to ignore the one case that means nothing can merge" + ) + + +def test_the_vault_shape_is_called_out_by_name( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """EVERY required context unreachable means NO PR can merge — say so, loudly. + + This is the case that cost the vault repo real time: it presents as PRs hanging, which reads as + flakiness. The diagnosis has to be in the output or someone spends a day on it. + """ + states = dict.fromkeys(_required_workflow_files(), "disabled_manually") + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + err = capsys.readouterr().err + assert "EVERY required context" in err, err + assert "NO pull request can merge" in err, err + + +def test_disabled_inactivity_counts_too(tmp_path: Path) -> None: + """``active`` is the only runnable state — not "anything that isn't disabled_manually". + + ``disabled_inactivity`` arrives on its own after 60 idle days, which is precisely the quiet period + this check exists to cover. Matching only the one state name seen on the vault would miss it. + """ + files = sorted(_required_workflow_files()) + states = dict.fromkeys(files, "active") + states[files[0]] = "disabled_inactivity" + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + + +def test_an_empty_workflow_list_fails_closed(tmp_path: Path) -> None: + """ "The API returned nothing" must never read as "everything is fine".""" + p = tmp_path / "empty.json" + p.write_text(json.dumps({"total_count": 0, "workflows": []}), encoding="utf-8") + assert main(["--states-json", str(p)]) == 2 + + +def test_an_unreadable_payload_fails_closed(tmp_path: Path) -> None: + """Same blindness, one level up: unable to measure is a FAILURE, not a pass.""" + p = tmp_path / "missing.json" + assert main(["--states-json", str(p)]) == 2 + + +def test_a_context_whose_workflow_is_absent_server_side_is_reported(tmp_path: Path) -> None: + """A required context pointing at a workflow the server does not have is equally unreportable. + + Distinct from the disabled case — the file may exist on a branch but never have landed on the + default branch, so GitHub has no workflow to run. + """ + files = sorted(_required_workflow_files()) + states = dict.fromkeys(files[1:], "active") # first workflow simply absent + assert main(["--states-json", str(_write(tmp_path, states))]) == 1 + + +def test_it_reuses_the_shared_resolver_rather_than_a_second_copy() -> None: + """One context->workflow mapping, two callers. + + A second implementation is the drift this repo keeps writing parity tests to catch (ruff/bandit + scan scope; the required-set prose). If the script ever grows its own parser, this fails and forces + that to be a deliberate decision. + """ + src = (_ROOT / "scripts" / "ci" / "check_required_workflow_state.py").read_text( + encoding="utf-8" + ) + assert "from tests._workflow_contexts import" in src, ( + "the reachability check no longer imports the shared resolver — it has grown a second " + "context->workflow mapping, which will drift from tests/_workflow_contexts.py" + ) + + +def test_the_script_imports_without_pytest_installed() -> None: + """The CI job installs the SCANNER lock, not the test toolchain — so pytest is absent there. + + This is the regression that broke PR #76's first run. ``tests/_workflow_contexts`` did + ``import pytest`` at module scope (only to reach ``importorskip("yaml")``), so the script died on + ``ModuleNotFoundError: No module named 'pytest'`` in CI while passing locally, where a dev venv has + pytest. A pure environment difference — exactly the class this repo keeps getting bitten by. + + Asserted by importing in a SUBPROCESS and checking ``pytest`` never entered ``sys.modules``. Testing + it in-process would be meaningless: pytest is, by definition, already imported when this runs. + """ + probe = ( + "import sys; " + "sys.path.insert(0, r'" + str(_ROOT) + "'); " + "import tests._workflow_contexts as m; " + "assert m.required_contexts(), 'resolver returned no contexts'; " + "print('PYTEST_IMPORTED' if 'pytest' in sys.modules else 'CLEAN')" + ) + out = subprocess.run( # noqa: S603 - fixed argv, no shell + [sys.executable, "-c", probe], capture_output=True, text=True, timeout=120 + ) + assert out.returncode == 0, f"importing the resolver failed:\n{out.stderr}" + assert "CLEAN" in out.stdout, ( + "importing tests/_workflow_contexts pulled in pytest. The CI job for " + "scripts/ci/check_required_workflow_state.py installs only ci/locks/ci-scanners.lock, so a " + "pytest import there is a hard failure. Import yaml directly and fall back to importorskip " + f"only when it is absent.\nstdout: {out.stdout}\nstderr: {out.stderr}" + ) + + +def test_the_workflow_is_scheduled_and_least_privilege() -> None: + """Per-PR alone cannot catch it: a workflow disabled AFTER the last PR has no PR to be caught on.""" + yaml = pytest.importorskip("yaml") + doc = yaml.safe_load( + (_ROOT / ".github" / "workflows" / "required-workflow-state.yml").read_text( + encoding="utf-8" + ) + ) + on = doc.get(True, doc.get("on")) + assert isinstance(on, dict) and "schedule" in on, ( + f"required-workflow-state.yml must run on a schedule; `on:` is {on!r}. A workflow disabled " + "after the last PR is invisible to any per-PR check." + ) + job = doc["jobs"]["reachable"] + assert job.get("permissions") == {"contents": "read", "actions": "read"}, ( + f"job permissions are {job.get('permissions')!r} — it needs `actions: read` to read workflow " + "state and nothing more. Notably NOT the admin scope that reading branch protection requires." + )