From 0a7c0be25ebc40fdbe15bf56b63ddda7b1987ab1 Mon Sep 17 00:00:00 2001 From: giulio d'erme Date: Sat, 10 Oct 2026 18:58:58 +0200 Subject: [PATCH] ci: pin every action to a commit SHA, starting with the one holding the PyPI token The publish step handed PYPI_API_TOKEN to pypa/gh-action-pypi-publish@release/v1. That is a branch, so whoever can push to it decides what code receives a long lived upload credential for this project, after any review I did of it. A tag is the same problem with an extra step. A full commit SHA is the only ref that names fixed content. Every remote action in ci.yml and release.yml is now pinned to the commit its latest release tag resolves to (annotated tags dereferenced), with the version in a trailing comment for Dependabot and for whoever reads the diff: actions/checkout v7.0.1 actions/setup-python v7.0.0 actions/upload-artifact v4.6.2 (latest v4; majors left to Dependabot) actions/download-artifact v4.3.0 (same, and the pair must move together) pypa/gh-action-pypi-publish v1.14.2 dtolnay/rust-toolchain v1, with `toolchain: stable` now explicit, since `@stable` chose it from the ref name No behaviour change is intended: every pin stays on the major it already used. pypi-publish pulls ghcr.io/pypa/gh-action-pypi-publish:, and that image exists for the pinned SHA (manifest HEAD returned 200 on 2026-10-10). tests/test_ci_contract.py gains a guard that every remote `uses:` is a 40 hex SHA followed by a `# vX` comment. Red proof: against d2317de it fails on actions/checkout@v7 in ci.yml; with the `# v4.3.0` comment deleted from download-artifact it fails on the comment assertion. Both green after. Auth is unchanged: the token stays, as 64cce6b decided. Returning to trusted publishing once the PyPI publisher record is fixed would remove the token altogether. The stale "opts into id-token" comment that 64cce6b left at the top of release.yml is corrected. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 13 ++++++---- .github/workflows/release.yml | 14 ++++++----- tests/test_ci_contract.py | 46 +++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2526316..1a4ce73 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,9 +16,9 @@ jobs: python-version: ["3.10", "3.11", "3.12", "3.13"] steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@v7 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: ${{ matrix.python-version }} @@ -35,8 +35,11 @@ jobs: # That is the same defect class test_dev_extra_completeness.py exists to # prevent for optional Python dependencies; the assertion that this step # stays here lives in tests/test_ci_contract.py. - uses: dtolnay/rust-toolchain@stable + # Pinned to the `v1` tag's commit. The `@stable` form picks the toolchain + # from the ref name, so with a SHA ref the toolchain is passed explicitly. + uses: dtolnay/rust-toolchain@e2a55d2ffb04f378e9626c28d38b36d230d1e12f # v1 with: + toolchain: stable components: clippy - name: Install the deterministic layer @@ -63,9 +66,9 @@ jobs: # or data file missing from the wheel stays invisible until a user installs it. runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - uses: actions/setup-python@v7 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 00881ff..8716d04 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,7 +42,7 @@ on: required: true type: string -# Least privilege by default; the publish job opts into `id-token` alone. +# Least privilege by default; each job opts into exactly what it needs. permissions: {} jobs: @@ -57,7 +57,7 @@ jobs: outputs: version: ${{ steps.verify.outputs.version }} steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: ref: ${{ github.event.release.tag_name || inputs.tag }} # Nothing here pushes, and a checkout that keeps a credential around @@ -65,7 +65,7 @@ jobs: # backend running code from the tree. persist-credentials: false - - uses: actions/setup-python@v7 + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" @@ -104,7 +104,7 @@ jobs: - name: twine check run: python -m twine check dist/* - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 with: name: dist path: dist/ @@ -124,12 +124,14 @@ jobs: # and requesting a credential this job cannot use is the kind of leftover # that reads as intentional to the next person. steps: - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 with: name: dist path: dist/ - name: Publish - uses: pypa/gh-action-pypi-publish@release/v1 + # This step receives PYPI_API_TOKEN, so it runs a commit SHA, never a + # tag or branch its owner could move. tests/test_ci_contract.py holds that. + uses: pypa/gh-action-pypi-publish@dc37677b2e1c63e2034f94d8a5b11f265b73ba33 # v1.14.2 with: password: ${{ secrets.PYPI_API_TOKEN }} diff --git a/tests/test_ci_contract.py b/tests/test_ci_contract.py index 21e7f1f..944c7e4 100644 --- a/tests/test_ci_contract.py +++ b/tests/test_ci_contract.py @@ -105,6 +105,52 @@ def test_ci_runs_the_configured_type_checker(): assert "run: python -m pyright" in _ci_text() +WORKFLOWS = REPO / ".github" / "workflows" + +# `uses: owner/repo[/path]@` with whatever follows on the line. Local actions +# (`./...`) and `docker://` images are not fetched from a git ref, so they are left +# to their own rules rather than matched here. +_USES = re.compile(r"^\s*(?:-\s+)?uses:\s*([^\s#]+)(.*)$", re.MULTILINE) +_SHA_PIN = re.compile(r"^[A-Za-z0-9_.\-]+/[A-Za-z0-9_.\-/]+@[0-9a-f]{40}$") +_VERSION_COMMENT = re.compile(r"^\s*#\s*v\d") + + +def _remote_uses() -> list[tuple[str, str, str]]: + found = [] + for path in sorted(WORKFLOWS.glob("*.yml")) + sorted(WORKFLOWS.glob("*.yaml")): + for ref, rest in _USES.findall(path.read_text(encoding="utf-8")): + if ref.startswith(("./", "docker://")): + continue + found.append((path.name, ref, rest)) + return found + + +def test_some_workflow_uses_a_remote_action(): + """Otherwise the pin check below passes over nothing.""" + assert _remote_uses(), "no `uses:` found in .github/workflows; this guard is inert" + + +def test_every_remote_action_is_pinned_to_a_commit_sha(): + """A tag or branch is a pointer its owner can move, and the publish job hands + PYPI_API_TOKEN to whatever `pypa/gh-action-pypi-publish@release/v1` resolves to + at run time. A 40-character SHA is the only ref that names fixed content. + + The trailing `# vX.Y.Z` comment is required too: it is what Dependabot reads to + propose the next pin, and what a reviewer reads to know which release a SHA is. + + Red proof: against origin/master at d2317de (every action on a tag or branch + ref) the first assertion fails on `actions/checkout@v7` in ci.yml. Deleting the + `# v4.3.0` comment from download-artifact in release.yml fails the second. + """ + for workflow, ref, rest in _remote_uses(): + assert _SHA_PIN.match(ref), ( + f"{workflow}: `{ref}` is not pinned to a full commit SHA; a tag or " + f"branch can be moved to different code after review") + assert _VERSION_COMMENT.match(rest), ( + f"{workflow}: `{ref}` has no trailing `# vX.Y.Z` comment naming the " + f"release the SHA was taken from") + + @pytest.mark.parametrize("language", ["python", "rust"]) def test_the_packaging_job_checks_every_backends_catalog(language): """A rule file missing from the wheel is invisible to the test job, which runs