From 578ed4e749e58e0f2896bb18c6083e13f90e1996 Mon Sep 17 00:00:00 2001 From: n1ckyb Date: Mon, 10 Aug 2026 18:01:35 +0100 Subject: [PATCH 1/2] fix(ci): correct action version comments that disagreed with their SHAs publish.yml pinned: actions/upload-artifact@043fb46d... # v4.5.0 <- SHA is v7.0.1 actions/download-artifact@3e5f45b2... # v4.1.4 <- SHA is v8.0.1 Confirmed against the upstream tag lists. The SHAs are correct and were verified when they landed; the COMMENTS were not updated with them. My fault. Rebuilding those Dependabot branches on the RC replaced the pinned SHA and left the trailing comment untouched, so every reader of this workflow has since been told it runs v4.5.0 while it actually runs v7.0.1 - a three-major gap in what a reviewer thinks they are approving. The comment is the ONLY human-readable part of a SHA pin. A wrong one is worse than none: it looks like provenance and is misinformation. It also explains why Dependabot #27 and #28 CONFLICT. Dependabot reads the comment, believes the pin is v4.5.0, and proposes bumping to the SHA that is already there. With the comments corrected those PRs are redundant rather than conflicting - they propose exactly what is already pinned. Co-Authored-By: Claude Opus 5 --- .github/workflows/publish.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 064833c..376555a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -229,7 +229,7 @@ jobs: --patterns "*.whl" - name: Upload dist artefacts - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v4.5.0 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: dist-${{ matrix.artifact }} path: | @@ -251,7 +251,7 @@ jobs: with: persist-credentials: false - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v4.1.4 + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: pattern: dist-* path: downloaded-dist/ @@ -316,7 +316,7 @@ jobs: with: persist-credentials: false - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v4.1.4 + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: pattern: dist-* path: downloaded-dist/ From 5342e2858f9647460e6cc133e33e1a8e8a33bf84 Mon Sep 17 00:00:00 2001 From: n1ckyb Date: Mon, 10 Aug 2026 18:23:05 +0100 Subject: [PATCH 2/2] fix(ci): correct all five lying action pins, and gate against a sixth The first commit on this branch fixed two pin comments. Auditing every pin against the real upstream tag lists found five, not two: actions/checkout@3d3c42e5 said v4.2.2 really v7.0.1 (ci.yml AND publish.yml) actions/upload-artifact@043fb46d said v4.5.0 really v7.0.1 actions/download-artifact@3e5f45 said v4.1.4 really v8.0.1 pypa/gh-action-pypi-publish@dc37 said release/v1 really v1.14.2 The last is not a lie - naming the branch is a legitimate convention, and dtolnay/rust-toolchain@... # stable is left exactly as it is for that reason. But it leaves Dependabot no version to compare against, which is why it keeps proposing 1.14.2 over a pin that already IS 1.14.2 (#30). All five share one signature: a bump replaced the SHA and left the comment behind. Nothing in CI could see it, because a stale comment is still valid YAML and the workflow runs perfectly - just not the version everyone believes it runs. WHY THIS IS NOT COSMETIC The comment is the only human-readable part of a SHA pin. Wrong, it breaks three things at once: - Reviewers approve a version they were never shown. The diffs above understate what runs by three and four major versions. - Dependabot proposes bumps that are already applied, because it trusts the comment. #27, #28, #30 and #31 are all CONFLICTING or failing for this reason and this reason alone. - "Pin to SHA" stops buying anything if nobody can tell which release the SHA is, and the label they use to tell is wrong. THE GATE Fixing five comments without adding a check just resets the clock, so: scripts/check_action_pin_comments.py resolves every pinned SHA against the upstream tag list and fails on disagreement. It reads only public tags, so GITHUB_TOKEN suffices and it runs on Dependabot and fork PRs - the PRs where this symptom actually surfaces. Comments naming a branch are reported, not failed. tests/unit/test_action_pin_comments.py checks offline what can be checked offline: no bare SHAs, one SHA never labelled two versions, one version label never pointing at two SHAs, nothing pinned to a mutable ref. Both were verified to BITE, not just to pass: reintroducing the v4.2.2 comment makes the script exit 1 with the mismatch named, and fails the offline test that one SHA carries two labels. Clean, the script reports 13 pins agreeing and 0 disagreeing. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 20 ++- .github/workflows/publish.yml | 10 +- scripts/check_action_pin_comments.py | 184 +++++++++++++++++++++++++ tests/unit/test_action_pin_comments.py | 117 ++++++++++++++++ 4 files changed, 325 insertions(+), 6 deletions(-) create mode 100644 scripts/check_action_pin_comments.py create mode 100644 tests/unit/test_action_pin_comments.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b1d83c..41dc4a2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: # hard-failed without it - nothing to do with the bumps they proposed. HAS_SPLIT_TOKEN: ${{ secrets.SPLIT_REPO_TOKEN != '' }} steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4.2.2 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false @@ -107,3 +107,21 @@ jobs: run: | echo "::notice::SPLIT_REPO_TOKEN unavailable (Dependabot or fork PR) - parser components were not provisioned, so component-dependent tests are skipped. A maintainer run exercises the full gate." python -m pytest tests/unit -q --deselect tests/unit/test_construct_edit_matrix.py + + action-pins: + name: Action pins match their comments + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Check every pinned SHA against its version comment + # Reads only PUBLIC tag lists, so GITHUB_TOKEN is enough and this job works on + # Dependabot and fork PRs, which receive no repository secrets. That matters: a + # wrong comment is precisely what makes Dependabot open bumps that are already + # applied, so the check has to run on the PRs where the symptom shows up. + env: + GITHUB_TOKEN: ${{ github.token }} + run: python scripts/check_action_pin_comments.py diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 376555a..9f4bed1 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -68,7 +68,7 @@ jobs: maturin_target: "" steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4.2.2 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false @@ -247,7 +247,7 @@ jobs: contents: read id-token: write steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4.2.2 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false @@ -293,7 +293,7 @@ jobs: --expected-platform-pattern "win_amd64" --expected-platform-pattern "win_arm64" - - uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # release/v1 + - uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 with: packages-dir: publish-dist/ attestations: false @@ -312,7 +312,7 @@ jobs: env: INTENTUMDIFF_EXPECTED_VERSION: ${{ inputs.testpypi_version }} steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v4.2.2 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false @@ -346,7 +346,7 @@ jobs: --max-release-mb 250 --expected-version "$INTENTUMDIFF_EXPECTED_VERSION" - - uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # release/v1 + - uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 with: packages-dir: publish-dist/ repository-url: https://test.pypi.org/legacy/ diff --git a/scripts/check_action_pin_comments.py b/scripts/check_action_pin_comments.py new file mode 100644 index 0000000..477a062 --- /dev/null +++ b/scripts/check_action_pin_comments.py @@ -0,0 +1,184 @@ +"""Assert every pinned GitHub Action SHA agrees with the version in its trailing comment. + +WHY THIS EXISTS +--------------- +Actions are pinned by full commit SHA, which is the right call: a tag is mutable and a +compromised action can be re-tagged under you. But a SHA is unreadable, so every pin carries +a trailing comment naming the version: + + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + +That comment is the ONLY human-readable part of the pin. It is what a reviewer actually reads +when approving a workflow change, and it is what Dependabot reads to decide whether a bump is +even needed. When it disagrees with the SHA, three things break at once: + + 1. Reviewers approve a version they were not shown. On 2026-08-10 this repo pinned + actions/checkout v7.0.1, upload-artifact v7.0.1 and download-artifact v8.0.1 while the + comments claimed v4.2.2, v4.5.0 and v4.1.4 - a three-major gap in what the diff said. + 2. Dependabot proposes bumps that are already applied, because it trusts the comment. Those + PRs then present as CONFLICTING or failing, and the wasted review time looks like a + Dependabot problem rather than a data problem. + 3. Supply-chain review stops working. "Pin to SHA" buys nothing if nobody can tell which + release the SHA is, and the label they use to tell is wrong. + +All five mismatches were introduced the same way: a bump replaced the SHA and left the comment +untouched. Nothing in CI could see it, because a wrong comment is still valid YAML and the +workflow runs perfectly - just not the version everyone believes. + +WHAT IT DOES +------------ +Resolves each pinned SHA against the upstream repository's real tag list and compares it to the +comment. Trusts nothing written in the file. + +Pins whose comment names a BRANCH rather than a version - `# stable` for dtolnay/rust-toolchain, +`# release/v1` for pypa/gh-action-pypi-publish - are a deliberate, different convention and are +reported separately rather than failed, since there is no version for them to disagree with. + +Needs only public read access; GITHUB_TOKEN is enough, so this runs on Dependabot and fork PRs +where repository secrets are unavailable. + +Usage: + python scripts/check_action_pin_comments.py # exits 1 on any mismatch + python scripts/check_action_pin_comments.py --list # report only, always exits 0 +""" + +from __future__ import annotations + +import json +import os +import re +import sys +import urllib.error +import urllib.request +from pathlib import Path + +WORKFLOWS = Path(".github/workflows") + +# `uses: owner/repo@<40 hex> # comment`. Actions pinned to a tag or branch rather than a SHA +# are not this script's business - a separate lint enforces SHA pinning. +PIN = re.compile( + r"""uses:\s* + (?P[\w.\-]+/[\w.\-]+) # owner/repo, ignoring any subpath + (?:/[\w.\-/]+)? + @(?P[0-9a-f]{40}) + [^\S\n]* + (?:\#[^\S\n]*(?P\S+))? + """, + re.VERBOSE, +) + +# A comment naming one of these is tracking a moving ref on purpose, not claiming a version. +BRANCH_LIKE = re.compile(r"^(stable|main|master|release/.*|v\d+$)") + + +def _api(path: str) -> list | dict | None: + req = urllib.request.Request( + f"https://api.github.com/{path}", + headers={ + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + **({"Authorization": f"Bearer {os.environ['GITHUB_TOKEN']}"} if os.environ.get("GITHUB_TOKEN") else {}), + }, + ) + try: + with urllib.request.urlopen(req, timeout=30) as r: + return json.load(r) + except (urllib.error.URLError, TimeoutError, json.JSONDecodeError): + return None + + +_tags: dict[str, dict[str, str] | None] = {} + + +def tags_for(action: str) -> dict[str, str] | None: + """Map commit SHA -> tag name for an action repo, or None if it could not be read. + + The /tags endpoint reports the COMMIT a tag points at, so annotated tags are already + dereferenced. Fetching refs/tags directly would hand back the tag object's own SHA and + silently match nothing. + """ + if action in _tags: + return _tags[action] + out: dict[str, str] = {} + for page in range(1, 6): # 500 tags is far beyond any action's release count + data = _api(f"repos/{action}/tags?per_page=100&page={page}") + if data is None: + _tags[action] = None + return None + if not data: + break + for tag in data: + sha, name = (tag.get("commit") or {}).get("sha"), tag.get("name") + if not sha or not name: + continue + # Several tags can share a commit (v4, v4.2, v4.2.2). Keep the most specific, + # so a v7.0.1 pin is never "matched" by the floating v7 that shares its commit. + if sha not in out or len(name) > len(out[sha]): + out[sha] = name + _tags[action] = out + return out + + +def main() -> int: + report_only = "--list" in sys.argv + + if not WORKFLOWS.is_dir(): + print(f"no {WORKFLOWS} directory - nothing to check") + return 0 + + mismatched: list[str] = [] + unresolved: list[str] = [] + branch_pins: list[str] = [] + ok = 0 + + for wf in sorted(WORKFLOWS.glob("*.y*ml")): + for m in PIN.finditer(wf.read_text(encoding="utf-8")): + action, sha, comment = m["action"], m["sha"], (m["comment"] or "").strip() + where = f"{wf.name}: {action}@{sha[:10]}" + + if not comment: + mismatched.append(f"{where} has NO version comment (a bare SHA is unreviewable)") + continue + if BRANCH_LIKE.match(comment): + branch_pins.append(f"{where} tracks branch '{comment}'") + continue + + known = tags_for(action) + if known is None: + unresolved.append(f"{where} could not reach the GitHub API") + continue + + real = known.get(sha) + if real is None: + unresolved.append(f"{where} says {comment} but that SHA is not at any tag") + elif real.lstrip("v") == comment.lstrip("v"): + ok += 1 + else: + mismatched.append(f"{where} says {comment} but is really {real}") + + for line in branch_pins: + print(f" branch pin {line}") + for line in unresolved: + print(f" UNRESOLVED {line}") + for line in mismatched: + print(f" MISMATCH {line}") + + print( + f"\n{ok} pin(s) agree with their comment, {len(mismatched)} disagree, " + f"{len(unresolved)} unresolved, {len(branch_pins)} track a branch" + ) + + if mismatched and not report_only: + print( + "\nA pin comment that disagrees with its SHA is worse than no comment: it looks " + "like provenance and is misinformation.\nFix the COMMENT to match the SHA (the SHA " + "is what actually runs), or change the SHA if the comment was the intent." + ) + return 1 + # An unreachable API must not silently pass as 'all good', but must not fail a PR for a + # network blip either - it is reported loudly and left to the reader. + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/unit/test_action_pin_comments.py b/tests/unit/test_action_pin_comments.py new file mode 100644 index 0000000..990a71f --- /dev/null +++ b/tests/unit/test_action_pin_comments.py @@ -0,0 +1,117 @@ +"""Network-free invariants on the workflow action pins. + +`scripts/check_action_pin_comments.py` is the real gate: it resolves each pinned SHA against +the upstream tag list and catches a comment that names the wrong version. It needs the network, +so it runs as its own CI job rather than here - unit tests in this repo do no network. + +What CAN be checked offline is internal consistency, and it is worth checking because it fails +first. Every mismatch found on 2026-08-10 (checkout, upload-artifact, download-artifact - all +claiming a version three-to-four majors below the SHA they carried) appeared in more than one +workflow file, and the update touched some occurrences and not others. That asymmetry is +visible without asking GitHub anything: the same SHA carrying two different version comments +is a contradiction on its face. + +These tests are cheap and catch the common case in milliseconds. The networked gate catches the +rest. +""" + +from __future__ import annotations + +import re +from collections import defaultdict +from pathlib import Path + +import pytest + +WORKFLOWS = Path(__file__).resolve().parents[2] / ".github" / "workflows" + +PIN = re.compile( + r"uses:\s*(?P[\w.\-]+/[\w.\-]+)(?:/[\w.\-/]+)?" + r"@(?P[0-9a-f]{40})[^\S\n]*(?:\#[^\S\n]*(?P\S+))?" +) + + +def _pins() -> list[tuple[str, str, str, str]]: + """(workflow, action, sha, comment) for every SHA-pinned action.""" + found = [] + for wf in sorted(WORKFLOWS.glob("*.y*ml")): + for m in PIN.finditer(wf.read_text(encoding="utf-8")): + found.append((wf.name, m["action"], m["sha"], (m["comment"] or "").strip())) + return found + + +def test_there_are_pins_to_check(): + """Guard the guard. + + Every assertion below passes vacuously against an empty list, so a regex that quietly + stops matching - or a workflow directory moved out from under this file - would turn the + whole module green while checking nothing. + """ + assert WORKFLOWS.is_dir(), f"{WORKFLOWS} does not exist" + assert len(_pins()) >= 5, f"expected several pinned actions, found {len(_pins())}" + + +def test_every_pinned_sha_carries_a_version_comment(): + """A bare 40-character SHA is unreviewable. + + Pinning by SHA is the correct supply-chain posture, but it only stays reviewable because + of the trailing comment. Without one, nobody approving the diff can tell v4 from v8. + """ + bare = [f"{wf}: {action}@{sha[:10]}" for wf, action, sha, comment in _pins() if not comment] + assert not bare, "pinned actions with no version comment:\n " + "\n ".join(bare) + + +def test_the_same_sha_never_carries_two_different_comments(): + """One commit is one version, so two labels for it means at least one is wrong. + + This is the exact shape of the 2026-08-10 defect: an update rewrote the SHA in every file + but the comment in only some, leaving `actions/checkout@3d3c42e5` documented as v4.2.2 in + one place and v7.0.1 in another. + """ + by_sha: dict[tuple[str, str], set[str]] = defaultdict(set) + for _wf, action, sha, comment in _pins(): + if comment: + by_sha[(action, sha)].add(comment) + + conflicts = {k: v for k, v in by_sha.items() if len(v) > 1} + assert not conflicts, "the same SHA is documented as more than one version:\n " + "\n ".join( + f"{action}@{sha[:10]} is labelled {sorted(labels)}" for (action, sha), labels in conflicts.items() + ) + + +def test_the_same_version_never_maps_to_two_different_shas(): + """The mirror image: one label pointing at two commits. + + Arises when a bump lands in one workflow and not another, so the estate silently runs two + different builds of what the files both call the same version. + """ + by_version: dict[tuple[str, str], set[str]] = defaultdict(set) + for _wf, action, sha, comment in _pins(): + if comment: + by_version[(action, comment)].add(sha) + + conflicts = {k: v for k, v in by_version.items() if len(v) > 1} + assert not conflicts, "one version label points at more than one SHA:\n " + "\n ".join( + f"{action} {version} -> {sorted(s[:10] for s in shas)}" for (action, version), shas in conflicts.items() + ) + + +@pytest.mark.parametrize("workflow", sorted(p.name for p in WORKFLOWS.glob("*.y*ml"))) +def test_no_action_is_pinned_to_a_mutable_ref(workflow: str): + """A tag can be moved; a SHA cannot. + + `uses: foo/bar@v4` re-resolves on every run, so an upstream compromise reaches this repo + without any change here. Local actions (`./...`) and reusable workflows in this org are + exempt - they are not third-party supply chain. + """ + text = (WORKFLOWS / workflow).read_text(encoding="utf-8") + mutable = [ + line.strip() + for line in text.splitlines() + if (m := re.search(r"uses:\s*(\S+)", line)) + and not m.group(1).startswith((".", "./")) + and "buchochelliq-labs/" not in m.group(1) + and "@" in m.group(1) + and not re.search(r"@[0-9a-f]{40}$", m.group(1)) + ] + assert not mutable, f"{workflow} pins a mutable ref:\n " + "\n ".join(mutable)