pin github actions to hashes - #12
Conversation
A mutable ref (@v4, @v10, and especially the @release/v1 *branch*) resolves at run time to whatever upstream points at today. Anyone able to move that tag -- an upstream maintainer, or an attacker who compromises the account -- executes arbitrary code in our CI with our tokens. Commit SHAs are immutable, so a compromised upstream cannot retroactively change what our workflows run. This matters twice over: once here, and once in the template, where every generated project inherits these refs verbatim. The highest-value fix is pypa/gh-action-pypi-publish, which was tracking a branch and holds the PyPI trusted-publishing identity of every generated project. All 17 third-party refs across the four workflow files are pinned, each with a trailing "# vX.Y.Z" comment so the pin stays reviewable. Actions are bumped to their latest major at the same time. SHAs were resolved from the GitHub API; the two annotated-tag actions were dereferenced to their commit objects. actions/checkout@3d3c42e # v7.0.1 actions/setup-python@5fda3b9 # v7.0.0 astral-sh/setup-uv@20cfd1b # v10.0.1 pypa/gh-action-pypi-publish@dc37677 # v1.14.2 python-semantic-release/python-semantic-release@39dd205 # v10.6.1 release.yaml's local reusable-workflow ref (./.github/workflows/cicd.yaml) is deliberately left unpinned: it lives in the generated repo itself, so it is not a supply-chain surface and a SHA there would point at a commit that does not exist yet. Tests assert the pinning against the *rendered* project, so the guarantee is proven to survive copier rendering into every downstream repo.
|
Build complete. ~Written by Claude, run via the agentic engineering loop |
|
Builder committed 1 of the 4 tasks its own plan ( ~Written by Claude, run via the agentic engineering loop |
mariushelf
left a comment
There was a problem hiding this comment.
Review — #4, pin GitHub Actions to hashes
Verdict: approve. Intent and behaviour line up. All 17 third-party call sites are pinned, every SHA independently re-verified against the GitHub API, the deliberate exemption is intact, and the PR's own CI is green on the bumped majors — which is the one criterion that could only be closed after the PR existed.
What I verified independently
All five SHAs re-resolved from the API just now, not taken from the PR body. Each is a real commit object, each is what its claimed tag actually points at, and each claimed version is still that repo's latest release — no drift since triage, matching the PR body's claim:
| action | pinned SHA | claimed | tag→commit | is latest |
|---|---|---|---|---|
actions/checkout |
3d3c42e…ba90b1 |
v7.0.1 |
✅ match | ✅ |
actions/setup-python |
5fda3b9…e4b97 |
v7.0.0 |
✅ match | ✅ |
astral-sh/setup-uv |
20cfd1b…c9a30d |
v10.0.1 |
✅ match | ✅ |
pypa/gh-action-pypi-publish |
dc37677…73ba33 |
v1.14.2 |
✅ match (annotated, correctly dereferenced) | ✅ |
python-semantic-release/… |
39dd205…d2550e |
v10.6.1 |
✅ match (annotated, correctly dereferenced) | ✅ |
The spec's trap #1 was the real risk here, and it was handled: git/ref/tags/vX returns a tag object for both of the bottom two, and pinning that SHA would fail at run time with an unresolvable-action error. Both are the dereferenced commits.
Everything else, checked rather than taken on trust:
git grep -nE 'uses:' -- .github template | grep -v '@[0-9a-f]\{40\}'→ exactly one line,template/.github/workflows/release.yaml:28: uses: ./.github/workflows/cicd.yaml. Was 18 onorigin/main.- Call-site counts per file match the spec exactly:
ci.yaml3,cicd.yaml9,publish.yaml1,release.yaml4 third-party + 1 local. pypa/gh-action-pypi-publishno longer tracks therelease/v1branch — the highest-value fix in the ticket, and the one holding trusted-publishing identity for every generated project.- The local reusable-workflow ref is byte-identical to
origin/main; it does not appear in the diff at all. uv run pytest tests→ 25 passed locally.- CI run
31827345853(pull_request, headd828f00) → success, along with the push run. Socheckoutv4→v7,setup-pythonv5→v7 andsetup-uvv5→v10 are proven on a real runner, not asserted. The spec's fallback (pin at the current major's tip) is not needed. - Commit subject is
ci: pin GitHub Actions to commit SHAs;git log --format='%(trailers)'is empty — no AI attribution trailer. - No stray action refs anywhere else in the repo (docs, README, template body): the grep for the five action names outside
spec/returns nothing unpinned. - Only
uses:lines moved. No job, trigger, matrix or step changes in any of the four files.
On the loop's "1 of 4 tasks" note
Not a truncated build. tasks/plan.md lists 4 tasks, of which #3 is run the regression suite and #4 is commit — neither produces its own commit. Tasks 1 (RED test) and 2 (pin the call sites) both landed in d828f00. The diff covers the plan in full.
The tests are real, not decorative
The three new tests in tests/test_template.py go beyond what the spec asked for, and the design choice is the right one: test_generated_workflows_pin_actions_to_sha asserts against the rendered project rather than the template source, so the guarantee is proven to survive copier rendering into every downstream repo — which is where the actual supply-chain exposure lives.
I mutation-tested both assertions rather than trusting a green run:
- reverting one ref to
@v4→ fails withassert not ['ci.yaml:17: actions/checkout@v4'] - stripping one
# v10.0.1comment → fails withassert not ['ci.yaml:23: astral-sh/setup-uv@20cfd1b…']
Both catch what they claim to catch.
Findings
Two non-blocking notes, both on the test file, both inline. Neither affects the pins themselves.
- minor — the tripwire globs
*.yamlonly; a future.ymlworkflow would silently escape it. - nit —
test_generated_release_keeps_local_workflow_ref_unpinnedhardcodescicd.yaml, the exact name #8 renames. Whoever rebases #8 should expect a red test here rather than a real regression.
For the human reviewer, not for this PR
- Cache growth is real and unmitigated.
setup-uvv9 flippedenable-cachetoauto(on for hosted runners onpush/pull_request) andprune-cachetofalse. Actions cache usage will grow here and in every generated project. The spec accepted this explicitly; recording it because it is the one operational cost the bump carries and it lands on downstream users who never opted into it. - Runner floor.
checkoutv5+ andsetup-pythonv6+ are node24 and need runner ≥v2.327.1.ubuntu-latestis fine; self-hosted runners in generated projects may not be, and those projects inherit the bump without asking for it. - Pins never self-update. Without Dependabot/Renovate these freeze and will miss upstream security fixes. Out of scope per the spec — worth confirming that follow-up ticket actually exists for both this repo and the template, since this PR is what makes it load-bearing.
Acceptance criteria
| # | criterion | |
|---|---|---|
| 1 | 17 third-party refs, all 40-hex SHA | ✅ met |
| 2 | trailing # vX.Y.Z on each |
✅ met |
| 3 | pypi-publish off the release/v1 branch |
✅ met |
| 4 | local reusable-workflow ref untouched | ✅ met |
| 5 | SHAs from the API, each a commit object | ✅ met — re-verified independently |
| 6 | PR body states the five pairs, flags drift | ✅ met — no drift, correctly stated |
| 7 | grep returns exactly one line | ✅ met |
| 8 | uv run pytest tests green |
✅ met — 25 passed |
| 9 | PR's own CI green on the bumped majors | ✅ met — run 31827345853 on d828f00 |
| 10 | ci: prefix, no AI attribution trailer |
✅ met |
One thing I can confirm only by outcome, not by process: criterion 5 also says the SHAs were resolved rather than recalled. Provenance isn't observable from a diff. What I can say is that every one of the five is independently verified to be the correct commit object for its claimed tag, including both annotated-tag dereferences — so the failure mode that criterion exists to prevent (a hallucinated or tag-object SHA) is demonstrably absent.
~Written by Claude, run via the agentic engineering loop
|
|
||
| def _uses_refs(workflow_dir): | ||
| """Yield ``(path, lineno, ref, comment)`` for every ``uses:`` in a directory.""" | ||
| for path in sorted(Path(workflow_dir).glob("*.yaml")): |
There was a problem hiding this comment.
minor — the tripwire globs *.yaml only, so a workflow added as .yml escapes it silently.
.yml is at least as common a spelling for GitHub workflow files, and the same gap would cover .github/actions/*/action.yml composite actions if any are ever added here or in the template. Today there are no .yml files in either tree, so this is not a live hole — but the whole point of these three tests is to keep the pinning from eroding, and a future release-notes.yml would sit outside the check with no signal.
for path in sorted(
p for ext in ("*.yaml", "*.yml") for p in Path(workflow_dir).glob(ext)
):Not gating: every pin that exists today is covered, and I mutation-tested both assertions — reverting one ref to @v4 and stripping one # vX.Y.Z comment each fail loudly.
~Written by Claude, run via the agentic engineering loop
| for path, _, ref, _ in _uses_refs(project.path / ".github" / "workflows") | ||
| if ref.startswith("./") | ||
| ] | ||
| assert local == [("release.yaml", "./.github/workflows/cicd.yaml")] |
There was a problem hiding this comment.
nit — this exact-match assertion hardcodes cicd.yaml, which is precisely the filename #8 renames to ci.yaml.
The spec calls out the #4/#7/#8 rebase interaction and notes the template CI workflow's nine call sites are unaffected by the rename — true for the workflows, but this test is a new place the rename has to be threaded through. Asserting the shape rather than the literal name would decouple it:
assert len(local) == 1
name, ref = local[0]
assert name == "release.yaml"
assert ref.startswith("./.github/workflows/")Arguable either way — the strict form does force a human to look when the name moves, which is not a bad property. Flagging it so whoever rebases #8 reads the red test as an expected rename fallout rather than a real regression.
~Written by Claude, run via the agentic engineering loop
|
Review iteration 1: approve — All 17 third-party ~Written by Claude, run via the agentic engineering loop |
|
Review verdict: approved (tier 2) ~Written by Claude, run via the agentic engineering loop |
All acceptance criteria verified. Work is committed on
feat/4-loop-pin-github-actions-to-hashesatd828f00; not pushed, no PR opened.PR-ready summary
ci: pin GitHub Actions to commit SHAsReplaces every third-party
uses:ref across all four workflow files with an immutable 40-hex commit SHA plus a# vX.Y.Zcomment, bumping each action to its latest major at the same time.Pins used
All five SHAs were resolved from the GitHub API during this run. No upstream drift since triage — every value matches the spec's table exactly.
actions/checkout3d3c42e5aac5ba805825da76410c181273ba90b1v7.0.1actions/setup-python5fda3b95a4ea91299a34e894583c3862153e4b97v7.0.0astral-sh/setup-uv20cfd1bf945f4377ade1205e4dbc17946fc9a30dv10.0.1pypa/gh-action-pypi-publishdc37677b2e1c63e2034f94d8a5b11f265b73ba33v1.14.2python-semantic-release/…39dd2052f2ce8282a5d932c31d58a2ca06d2550ev10.6.1The last two publish annotated tags, so the tag-object SHA was dereferenced to the underlying commit and each confirmed a genuine commit object via
gh api …/git/commits/<sha>returning the same SHA — the trap the spec called out.What changed
.github/workflows/ci.yaml(3),template/.github/workflows/cicd.yaml(9),publish.yaml(1),release.yaml(4).release.yaml:28's local reusable-workflow ref./.github/workflows/cicd.yamlleft untouched, as specified.uses:lines moved — no job, trigger, matrix, or step changes.New behavior: a regression tripwire
Three tests in
tests/test_template.pykeep the pinning from silently eroding:test_own_workflows_pin_actions_to_sha— this repo's own CI.test_generated_workflows_pin_actions_to_sha— asserted against the rendered project, not the template source, so the guarantee is proven to survive copier rendering into every downstream repo.test_generated_release_keeps_local_workflow_ref_unpinned— pins the deliberate exemption in place so a future sweep doesn't "helpfully" SHA-pin it.Each also requires the trailing
# vX.Y.Zcomment, so an opaque hash can't land unlabeled.Verification
git grep -nE 'uses:' -- .github template | grep -v '@[0-9a-f]\{40\}'→ exactly one line, the local ref (was 18 onorigin/main).uv run pytest tests→ 25 passed (22 pre-existing + 3 new).ci:prefix and carries no AI attribution trailer.Impact and risk
setup-uvv9 flippedenable-cachetoauto— caching on for hosted runners onpush/pull_request— and defaultsprune-cachetofalse. Actions cache usage will grow here and in every generated project. This is the real operational cost of the major bump.checkoutv5+ andsetup-pythonv6+ are node24 and need runner ≥v2.327.1. GitHub-hostedubuntu-latestis fine; self-hosted runners in generated projects may not be.pip-install,pyproject-file/uv-file) are unused here; nosetup-uvstep passes anywith:inputs.checkoutv7's fork-PR restriction needspull_request_target/workflow_run, neither of which appears in these files.git revert.Two notes for the reviewer
cicd.yamlhad not been renamed on this branch (#8 hasn't landed), so the template CI workflow was pinned under its current name; whichever of #4/#7/#8 lands first, the others rebase cleanly since none of the nine call sites move.Fixes #4
~Written by Claude, run via the agentic engineering loop