Skip to content

Security: pin GitHub Actions in release and CI workflows to commit SHAs - #10

Open
pimfeltkamp wants to merge 1 commit into
mainfrom
ai-security/code-d39f0006
Open

pimfeltkamp wants to merge 1 commit into
mainfrom
ai-security/code-d39f0006

Conversation

@pimfeltkamp

Copy link
Copy Markdown
Contributor

What was vulnerable

The tag-triggered publish job in .github/workflows/release.yml runs with contents: write and exposes CARGO_REGISTRY_TOKEN to cargo publish, yet it used third-party actions on mutable refs: dtolnay/rust-toolchain@stable (a moving branch), softprops/action-gh-release@v3 (a moving tag), plus actions/checkout@v6 and actions/cache@v5. If any of those upstream refs were compromised, the action could install a trojaned cargo that reads the crates.io token or tampers with the published crate.

Review also showed that ci.yml writes the same cargo cache family (${{ runner.os }}-cargo-) that the release job restores via restore-keys, so an unpinned action in CI could poison registry sources or build outputs that the release job then consumes. CI is therefore pinned as well.

What the fix does

Pins every uses: in both workflows to a full 40-character commit SHA with a version comment:

Action Pinned to Equals
actions/checkout d23441a4… v6.1.0 (same commit v6 resolves to today)
dtolnay/rust-toolchain b725eda5… current head of stable (2026-10-16)
actions/cache caa29612… v5.1.0 (same commit v5 resolves to today)
softprops/action-gh-release efb35369… v3.0.3 (same commit v3 resolves to today)

All SHAs were resolved live from GitHub in this run and independently re-verified by a second reviewer pass.

Why it is non-breaking

Each pin is byte-identical to the commit the old mutable ref currently points at, so the jobs run exactly the same action code as before. dtolnay/rust-toolchain at that commit still runs rustup toolchain install stable on every run, so releases continue to build with the newest stable Rust; only the action's own script is frozen. No public API, crate contents or accepted input change.

.github/dependabot.yml already has a github-actions entry, so the three version-tagged pins will be bumped automatically. Note: the dtolnay/rust-toolchain pin is a branch head without a tag, so Dependabot will likely not bump it on its own; it will need an occasional manual refresh. This is low impact because the Rust version itself is not frozen.

Tests run

The repository has no tests covering workflow files. Both files were syntax-checked:

npx js-yaml .github/workflows/release.yml   -> parsed OK, 4 uses: refs show the SHA pins
npx js-yaml .github/workflows/ci.yml        -> parsed OK, 3 uses: refs show the SHA pins

No Rust code changed, so the cargo test suite was not run.

🤖 Automated security fix by claude-code-security. It is reviewed by the automated PR reviewer and auto-merged only if the review passes, CI is green, and the change is small and non-breaking.

🤖 Generated with Claude Code

The tag-triggered publish job holds CARGO_REGISTRY_TOKEN and contents: write
but ran dtolnay/rust-toolchain@stable (moving branch), softprops/action-gh-release@v3
(moving tag), actions/checkout@v6 and actions/cache@v5. A compromised action
could install a trojaned cargo that reads the token or tampers with the
published crate. ci.yml writes the same cargo cache the release job restores,
so its actions are pinned too.

Each pin resolves to the exact commit the old mutable ref pointed at today,
so no behaviour changes. Dependabot's github-actions entry keeps them updated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread .github/workflows/ci.yml Resolved
Comment thread .github/workflows/release.yml Resolved

@pimfeltkamp pimfeltkamp left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 findings (high): dtolnay/rust-toolchain pinned to a SHA with no toolchain input will fail in ci.yml and release.yml (the release would never publish). Details are in the inline comments.

@pimfeltkamp

Copy link
Copy Markdown
Contributor Author
Review feedback round: no code changes.

Both findings (ci.yml line 18 and release.yml line 20) say dtolnay/rust-toolchain fails when pinned to a SHA because it has no toolchain input. That doesn't apply to this pin:

  • ci.yml line 18: no change needed. The pinned SHA b725eda525362e7cf34af9a7243cac1e3f01a7ec is the current head of the stable branch (the compare API returns identical). The action.yml at that commit declares toolchain with required: false and default: stable, so the input resolves to stable and the parse step's empty-input error never fires. The action doesn't read the ref name; it reads its input, and the default comes from the commit's own action.yml.
  • release.yml line 20: no change needed, same reasoning. The toolchain step installs stable, and cargo publish runs as before.

Caveat, noted in both threads: the master branch's action.yml has no default. If this pin is ever refreshed to a master commit, toolchain: stable will need to be added then.

I replied in both threads and left them open for the reviewer.

@pimfeltkamp pimfeltkamp left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Automated review passed — no issues found. (Would approve, but the reviewer account authored this PR and GitHub forbids self-approval.)

✅ Safe to merge

@pimfeltkamp pimfeltkamp added the ai-security-needs-human Security agent: this PR needs a human decision label Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-security-needs-human Security agent: this PR needs a human decision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant