Pin every third-party action to a commit SHA - #16
Merged
Merged
Conversation
Found while auditing all 26 unslothai repositories against the 2026-09-21 cargo-miri cache disclosure. This is not an instance of that class: the two jobs here that write caches hold no secrets at all, so the chain never starts. It is the adjacent supply-chain problem the same audit looks for. 52 uses across 20 distinct actions referenced a mutable tag. A tag is a pointer the upstream owner can move at any time, so every one of them was an agreement to run whatever that repository contains on the day CI happens to run, decided by someone outside this project. Two matter more than the rest: ilammy/msvc-dev-cmd@v1 and anzz1/action-create-release@v1 are major-version tags on small single-maintainer repositories, and the second runs in the release job, which holds secrets.GITHUB_TOKEN. That token is ephemeral and expires with the job, which bounds the damage but does not remove it. Each ref is now a 40-character commit SHA with the tag it resolved to kept in a trailing comment, so the version stays readable and renovate-style tooling can still see it. No version changes: every SHA is what the tag pointed at when this was written, so the behaviour is identical to the last run. One incidental finding, recorded because it looks alarming and is not. build.yml used two spellings, prompt/actions-commit-hash in seven places and pr-mpt/actions-commit-hash in one, which reads like a typosquat. They are the same repository: pr-mpt was renamed to prompt, GitHub redirects the old path, and both resolve to 01d19a83c242e1851c9aa6cf9625092ecd095d09. Pinning collapses them onto one identity, so the question cannot come up again. These files are inherited from upstream leejet/stable-diffusion.cpp, so the diff is deliberately nothing but the refs. Not addressed here, so it is not lost: the ROCm cache at build.yml:476 stores C:\TheRock\build, which is a Python virtual environment the later build activates and executes, under the key rocm-wheels-<ROCM_VERSION>-<runner.os>. That key records no content identity, and there is no lockfile in the repository to hash because the wheels come from an index at install time. The exposure is cache poisoning rather than secret disclosure, and it is bounded by cache writes being scoped to the branch that made them, so a pull request cannot reach the entry master uses. Fixing it properly means splitting the combined action into a restore and a gated save in inherited upstream code, which is a larger change than this one should carry.
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while auditing all 26
unslothairepositories against the 2026-09-21 Rust disclosure, wherecargo miriwrote the whole process environment undertarget/, CI cached it, andpull_requestruns could restore caches written on the default branch.This repository is not an instance of that class. The two jobs that write caches (
windows-latest-rocm,ubuntu-latest-rocm) contain nosecrets.*reference at all, so the chain never starts. What the same audit does turn up here is the adjacent supply-chain problem.The change
52 uses across 20 distinct actions referenced a mutable tag. A tag is a pointer the upstream owner can move whenever they like, so each one was an agreement to run whatever that repository contains on the day CI happens to run. Two stand out:
ilammy/msvc-dev-cmd@v1andanzz1/action-create-release@v1are major-version tags on small single-maintainer repositories, and the second runs in thereleasejob (build.yml:735), which holdssecrets.GITHUB_TOKENat:777. That token expires with the job, which bounds the damage without removing it.Every ref is now a 40-character commit SHA with the tag it resolved to kept in a trailing comment, so the version stays readable and tooling can still see it. No version changes: each SHA is what the tag pointed at when this was written, so behaviour is identical to the last run.
One incidental finding, which looks worse than it is
build.ymlused two spellings:prompt/actions-commit-hashin seven places andpr-mpt/actions-commit-hashin one. That reads like a typosquat. It is not —pr-mptwas renamed toprompt, GitHub redirects the old path, and both resolve to the same commit01d19a83c242e1851c9aa6cf9625092ecd095d09. Pinning collapses them onto one identity, so the question cannot arise again.These files are inherited from upstream
leejet/stable-diffusion.cpp, so the diff is deliberately nothing but the refs.Not addressed here
The ROCm cache at
build.yml:476storesC:\TheRock\build, which is a Python virtual environment the build later activates and executes (:497-498), under the keyrocm-wheels-${{ env.ROCM_VERSION }}-${{ runner.os }}. That key records no content identity, and there is no lockfile in the repository to hash, because the wheels come from an index at install time.The exposure is cache poisoning rather than secret disclosure, and it is bounded: cache writes are scoped to the branch that made them, so a pull request cannot reach the entry
masteruses, and poisoning would need push access to a branch matching the target scope. Fixing it properly means splitting the combinedactions/cacheinto a restore plus a gated save in inherited upstream code, which is a larger and more opinionated change than this one should carry.Verification
Every workflow still parses as YAML, and a re-scan for third-party
uses:refs not matching@<40 hex>now returns nothing.