diff --git a/.github/workflows/publish-python.yaml b/.github/workflows/publish-python.yaml index 86ce7e6..d619a33 100644 --- a/.github/workflows/publish-python.yaml +++ b/.github/workflows/publish-python.yaml @@ -17,9 +17,9 @@ jobs: environment: pypi steps: - - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.11" diff --git a/.github/workflows/test-python-build.yaml b/.github/workflows/test-python-build.yaml index 1200bdc..43ca300 100644 --- a/.github/workflows/test-python-build.yaml +++ b/.github/workflows/test-python-build.yaml @@ -9,8 +9,8 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.11" - name: Install build tools @@ -28,8 +28,8 @@ jobs: matrix: python-version: ["3.9", "3.10", "3.11", "3.12"] steps: - - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: ${{ matrix.python-version }} - name: Install @@ -42,11 +42,22 @@ jobs: # nothing. Run it on one version (lint results don't vary across the matrix). runs-on: ubuntu-latest steps: - - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" - name: Install run: pip install -e ".[dev]" - name: Lint (ruff) run: ruff check . + + # Same rationale as the ruff step above: pin results don't vary across the + # matrix, so this lives here rather than in the 4-way `test` job — one + # `git ls-remote` per action, not four. This is the half of pin hygiene that + # needs the network, so it can't live in the (hermetic) pytest suite: does + # each pin's `# vX.Y.Z` actually name the tag its SHA carries? A wrong label + # is worse than none — it makes a major-version jump read as a routine + # same-line bump, which is how a checkout v6 -> v7.0.1 bump nearly landed in + # nf-spawn labelled `# v6`. Read-only, no auth. + - name: Verify pin comments match their SHAs + run: ./scripts/verify-pins.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b93df..3c46a71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ Release tags use the `python-vX.Y.Z` prefix. ## [Unreleased] +### Fixed +- **A pin's version comment can no longer silently misstate what CI runs.** + `tests/test_ci_hygiene.py` required only that *some* `# vN` comment be present, + never that it was true. A wrong label is worse than a missing one: it makes a + major-version jump read as a routine same-line bump. Not hypothetical — + Dependabot bumped nf-spawn's `checkout` pin to a **v7.0.1** SHA while leaving the + comment reading `# v6`, and the identical regex passed it. Two complementary + halves now, because neither alone suffices: the test requires an exact `vX.Y.Z` + (offline, hermetic — catches vague labels), and a new `scripts/verify-pins.sh` + resolves each SHA against the tag its comment claims and fails if they disagree + (needs the network, so it runs as its own CI step — catches exact-but-false + labels the offline half cannot see). This repo's nine pins were already exact + and true, so nothing needed relabelling; the gate is what changed. The new step + runs in the single-version `lint` job rather than the 4-way `test` matrix, for + the same reason `ruff` does — pin results don't vary across Python versions. + ### Security - **The PyPI publish job ran a mutable *branch* ref; every action is now pinned to a commit SHA, with Dependabot to bump the pins** ([#10]). `publish-python.yaml` diff --git a/scripts/verify-pins.sh b/scripts/verify-pins.sh new file mode 100755 index 0000000..99256dc --- /dev/null +++ b/scripts/verify-pins.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# Verify every action pin's `# vX.Y.Z` comment actually names the tag its SHA +# carries. +# +# This is the half of pin hygiene that needs the network, so it is NOT in the +# pytest suite (which must stay hermetic). tests/test_ci_hygiene.py enforces the +# offline half: every pin is a 40-hex SHA with an exact vX.Y.Z comment. +# +# Why it exists: a pin's comment can be silently false. Dependabot bumped +# actions/checkout to 3d3c42e — which is v7.0.1 — while leaving `# v6` on all +# five refs, and a stale `# v4` on gradle/actions/setup-gradle (really v4.4.3) +# had sat on main for months. Both read as fine. A wrong label is worse than no +# label: it makes a major-version jump look like a routine same-line bump. +# +# Read-only: git ls-remote only. No auth, no writes, no side effects. +# +# Usage: scripts/verify-pins.sh +set -euo pipefail + +cd "$(dirname "$0")/.." + +fail=0 +found=0 + +while IFS= read -r line; do + # file:lineno: ... uses: owner/action@ # vX.Y.Z + loc=${line%%:*} + rest=${line#*:} + lineno=${rest%%:*} + + ref=$(printf '%s\n' "$line" | sed -nE 's/.*uses:[[:space:]]*([^[:space:]]+@[0-9a-f]{40}).*/\1/p') + ver=$(printf '%s\n' "$line" | sed -nE 's/.*@[0-9a-f]{40}[[:space:]]*#[[:space:]]*(v[0-9][^[:space:]]*).*/\1/p') + [ -n "$ref" ] && [ -n "$ver" ] || continue + + action=${ref%@*} + sha=${ref#*@} + # gradle/actions/setup-gradle is a subdirectory action: the repo is the first + # two path segments, so trim anything deeper before querying. + repo=$(printf '%s\n' "$action" | cut -d/ -f1,2) + + found=$((found + 1)) + + # Resolve the claimed tag to BOTH acceptable SHAs and accept either. + # + # For an annotated tag, `refs/tags/vX` is the tag OBJECT and `refs/tags/vX^{}` + # the commit it points at, and the two differ. Both are legitimate pins — + # gradle/actions/setup-gradle@48b5f21 is v4.4.3's tag object, actions/checkout + # pins the commit — and GitHub resolves either. Comparing against just one form + # reports a correct pin as mislabeled (which this script did on its first run, + # absurdly printing "comment says v4.4.3 ... SHA is actually v4.4.3"). + claimed=$(git ls-remote "https://github.com/$repo" \ + "refs/tags/$ver" "refs/tags/$ver^{}" 2>/dev/null | awk '{print $1}') + + if [ -z "$claimed" ]; then + printf ' MISSING TAG %s:%s %s @ %.12s — tag %s does not exist in %s\n' \ + "$loc" "$lineno" "$action" "$sha" "$ver" "$repo" + fail=1 + elif ! printf '%s\n' "$claimed" | grep -qFx "$sha"; then + # Name what the SHA really is, so the fix is obvious rather than a puzzle. + actual=$(git ls-remote --tags "https://github.com/$repo" 2>/dev/null \ + | awk -v s="$sha" '$1==s {gsub("refs/tags/",""); gsub(/\^\{\}$/,"",$2); print $2}' \ + | sort -u | paste -sd, -) + printf ' MISLABELED %s:%s %s @ %.12s — comment says %s (that tag is %.12s); SHA is actually %s\n' \ + "$loc" "$lineno" "$action" "$sha" "$ver" "$(printf '%s' "$claimed" | head -1)" \ + "${actual:-untagged}" + fail=1 + else + printf ' ok %s:%s %s %s\n' "$loc" "$lineno" "$action" "$ver" + fi +done < <(grep -rn 'uses:.*@[0-9a-f]\{40\}' .github/workflows/) + +# Anti-vacuous: a grep that stops matching would otherwise report success forever. +if [ "$found" -eq 0 ]; then + echo "verify-pins: found no SHA-pinned actions under .github/workflows — the check" >&2 + echo "matched nothing, which is a parser problem, not a clean bill of health." >&2 + exit 1 +fi + +if [ "$fail" -ne 0 ]; then + echo + echo "verify-pins: FAIL — a pin's version comment does not match its SHA." >&2 + echo "Fix the comment to name the tag the SHA really carries (or move the SHA)." >&2 + exit 1 +fi + +echo +echo "verify-pins: OK — all $found pins' comments match their SHAs." diff --git a/tests/test_ci_hygiene.py b/tests/test_ci_hygiene.py index 8237575..7d78547 100644 --- a/tests/test_ci_hygiene.py +++ b/tests/test_ci_hygiene.py @@ -29,7 +29,14 @@ # owner/action@<40-hex> followed by a `# vX.Y.Z` comment. The comment is required: # a bare SHA is unreadable, and the version is what makes a bump reviewable — # without it nobody can tell whether a pin is current or two years stale. -PINNED = re.compile(r"^[^@\s]+@[0-9a-f]{40}\s+#\s*v?\d") +# The comment must be an EXACT vX.Y.Z, not a bare `# v7`. A bare major cannot be +# checked against the SHA and can silently misstate what CI runs: Dependabot bumped +# nf-spawn's checkout pin to a v7.0.1 SHA while leaving the comment reading `# v6`, +# and the older `v?\d` form of this pattern passed it. A wrong label is worse than a +# missing one — it makes a major-version jump read as a routine same-line bump. +# scripts/verify-pins.sh checks comment-against-tag for real; that needs the network, +# so it stays out of the test suite. +PINNED = re.compile(r"^[^@\s]+@[0-9a-f]{40}\s+#\s*v\d+\.\d+\.\d+\s*$") def _uses_refs() -> list[tuple[str, int, str]]: