diff --git a/.github/workflows/gate-freshness.yml b/.github/workflows/gate-freshness.yml new file mode 100644 index 0000000..471c9b6 --- /dev/null +++ b/.github/workflows/gate-freshness.yml @@ -0,0 +1,127 @@ +name: gate freshness + +# WHY THIS EXISTS +# +# GitHub runs a `pull_request` workflow from the PULL REQUEST'S OWN +# branch, not from the base. So a branch that forked before a gate was +# added keeps running the OLD workflow — and its green tick means +# something weaker than the tick on a fresh branch, while looking +# identical. +# +# That is not hypothetical here. #74 made the database a derived +# artifact and added two gates to ci.yml: a determinism check (two +# builds must agree on the content hash) and a no-drift gate (`git diff +# --exit-code` after the build and after the suite). Branches that +# forked before it kept the pre-#74 workflow, so NEITHER GATE HAD EVER +# RUN against them — and they were reviewed and approved on the +# understanding that both had. They also still tracked the committed +# database that #74 removed. +# +# This job closes that. `pull_request_target` runs the workflow from the +# BASE branch, so a stale head cannot skip it: the check is defined by +# main and applies to every PR regardless of what its own .github looks +# like. +# +# SECURITY: pull_request_target runs in the context of the base repo, so +# this job NEVER checks out or executes pull-request code. It reads git +# metadata only — commit ancestry — and holds read-only permissions. + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +permissions: + contents: read + +jobs: + freshness: + name: PR contains the current gates + runs-on: ubuntu-latest + steps: + # Base branch only. The PR's tree is never checked out. + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.base.sha }} + fetch-depth: 0 + + - name: Fetch the head commit as data + run: | + git fetch --no-tags --depth=200 origin \ + "+${{ github.event.pull_request.head.sha }}:refs/gatecheck/head" || \ + git fetch --no-tags origin \ + "+${{ github.event.pull_request.head.sha }}:refs/gatecheck/head" + + - name: Refuse a branch whose green tick means less than main's + env: + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + BASE_SHA: ${{ github.event.pull_request.base.sha }} + run: | + set -euo pipefail + + # The gate set is DERIVED FROM THE BASE, not hardcoded here, so + # a gate added to ci.yml later is enforced on every open PR + # without anyone remembering to update this file. A "gate" is a + # line of the base workflow that makes the build prove + # something: the determinism assertion and the no-drift checks. + git show "$BASE_SHA:.github/workflows/ci.yml" > /tmp/base-ci.yml + grep -E 'nondeterministic|git diff --exit-code' /tmp/base-ci.yml \ + | sed 's/^[[:space:]]*//' | sort -u > /tmp/required-gates.txt + + echo "Gates defined by the base branch:" + sed 's/^/ /' /tmp/required-gates.txt + + stale=0 + + if git cat-file -e "$HEAD_SHA:.github/workflows/ci.yml" 2>/dev/null; then + git show "$HEAD_SHA:.github/workflows/ci.yml" > /tmp/head-ci.yml + else + echo "STALE this branch has no .github/workflows/ci.yml at all" + : > /tmp/head-ci.yml + stale=1 + fi + + while IFS= read -r gate; do + [ -z "$gate" ] && continue + if grep -qF -- "$gate" /tmp/head-ci.yml; then + echo "ok $gate" + else + echo "STALE this branch's ci.yml is missing: $gate" + stale=1 + fi + done < /tmp/required-gates.txt + + # The committed database left the repo in #74. A branch that + # still tracks it forked before that, and its build cannot have + # been from-scratch. + if git ls-tree -r --name-only "$HEAD_SHA" -- data/scorecard.db | grep -q .; then + echo "STALE data/scorecard.db is still TRACKED on this branch (removed in #74)" + stale=1 + fi + + if [ "$stale" -ne 0 ]; then + cat <<'MSG' + + ──────────────────────────────────────────────────────────── + This branch is running an OLDER workflow than main's. + + GitHub runs a pull_request workflow from the PULL REQUEST'S OWN + branch. So a branch that forked before a gate was added keeps + running the workflow WITHOUT that gate — and its green tick + looks identical to one that ran the full set while meaning + strictly less. + + Fix, and nothing about your own changes needs to move: + + git checkout + git merge origin/main + rm -f data/scorecard.db # gitignored since #74 + PYTHONPATH=. python -m scorecard_db.build_db data/scorecard.db + pytest tests/ -q + git status --short # must be clean + + ──────────────────────────────────────────────────────────── + MSG + exit 1 + fi + + echo "This PR runs every gate the base branch defines." diff --git a/tests/test_ci_gates.py b/tests/test_ci_gates.py new file mode 100644 index 0000000..86b8bea --- /dev/null +++ b/tests/test_ci_gates.py @@ -0,0 +1,105 @@ +"""The CI gates, and the guard that stops a stale branch skipping them. + +GitHub runs a `pull_request` workflow from the PULL REQUEST'S OWN +branch, not from the base. A branch that forked before a gate was added +therefore keeps running the older workflow — and its green tick looks +identical to a full one while meaning strictly less. + +That happened here: #74 made the database a derived artifact and added a +determinism check and a no-drift gate, and branches forked before it +were reviewed and approved on the understanding that both had run +against them when neither had. + +`gate-freshness.yml` closes it by running from the BASE via +`pull_request_target`. These tests keep that guard honest — a workflow +nobody can weaken by accident is the point of it. +""" + +import re +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent +CI = ROOT / ".github" / "workflows" / "ci.yml" +FRESHNESS = ROOT / ".github" / "workflows" / "gate-freshness.yml" + + +def test_the_gates_ci_defines_are_still_there(): + """These two lines are what a green tick is supposed to mean.""" + text = CI.read_text() + assert "git diff --exit-code" in text, "the no-drift gate is gone" + assert "nondeterministic" in text, "the determinism check is gone" + + +def test_the_freshness_guard_exists(): + assert FRESHNESS.exists(), ( + "gate-freshness.yml is what stops a branch that forked before a " + "gate from skipping it" + ) + + +def test_it_runs_from_the_base_branch(): + """`pull_request` would run it from the PR's own branch, which is the + exact failure it exists to prevent — a stale branch would simply not + have the guard.""" + text = FRESHNESS.read_text() + assert "pull_request_target:" in text + assert not re.search(r"^on:\s*\n\s+pull_request:", text, re.M) + + +def test_it_never_executes_pull_request_code(): + """pull_request_target runs in the base repo's context, so checking + out and running PR code would be a privilege-escalation hole. This + job reads git metadata only.""" + text = FRESHNESS.read_text() + assert "ref: ${{ github.event.pull_request.base.sha }}" in text + assert "ref: ${{ github.event.pull_request.head.sha }}" not in text + assert "contents: read" in text + # Scan only the EXECUTABLE part: the failure message is a heredoc + # that tells the author to run pytest, which is help text, not + # something this job does. + body = text.split("Fetch the head commit as data", 1)[1] + executable = re.sub(r"cat <<'MSG'.*?\n\s*MSG\n", "", body, flags=re.S) + for danger in ("pip install", "pytest ", "uv run", "python -m scorecard_db"): + assert danger not in executable, f"the guard must not run {danger!r}" + # ...and the help text really is still there + assert "pytest tests/ -q" in body + + +def test_the_gate_set_is_derived_from_the_base_not_hardcoded(): + """A gate added to ci.yml later must be enforced on every open PR + without anyone remembering to update the guard.""" + text = FRESHNESS.read_text() + assert 'git show "$BASE_SHA:.github/workflows/ci.yml"' in text + assert "required-gates.txt" in text + + +def test_it_also_catches_the_committed_database(): + """A branch that still tracks data/scorecard.db forked before #74, so + its build cannot have been from-scratch.""" + text = FRESHNESS.read_text() + assert "data/scorecard.db" in text + assert "still TRACKED" in text + + +def test_the_failure_message_says_what_to_do(): + """A gate that fails without a fix is a gate people learn to ignore.""" + text = FRESHNESS.read_text() + assert "git merge origin/main" in text + assert "rm -f data/scorecard.db" in text + assert "nothing about your own changes needs to move" in text.lower() + + +@pytest.mark.skipif(not (ROOT / ".git").exists(), reason="needs a git checkout") +def test_the_database_is_not_tracked_here(): + """The property the guard checks, asserted for this tree too.""" + import subprocess + + tracked = subprocess.run( + ["git", "ls-files", "data/scorecard.db"], + cwd=ROOT, + capture_output=True, + text=True, + ).stdout.strip() + assert not tracked, "data/scorecard.db is a derived artifact (#74)"