Conversation
|
@dagecko is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
mahdirajaee
left a comment
There was a problem hiding this comment.
This is a solid supply-chain security hardening. Pinning GitHub Actions to full commit SHAs instead of mutable tags (like v4 or v1) prevents a compromised upstream repository from injecting malicious code via tag rewriting. All four pinned SHAs resolve to valid commits in their respective repositories, and the inline # v4 / # v1 / # main comments are a good practice for human readability.
Coverage looks complete across all six workflow files: code-check.yml (3 instances of pnpm), test.yml, validate-registries.yml, prerelease.yml (pnpm + npm-get-version), prerelease-comment.yml (sticky-pull-request-comment), and release.yml (pnpm + changesets/action). The actions/checkout and actions/setup-node references still use tag-based versions (v4, v5) -- those are first-party GitHub actions with a stronger trust model, so leaving them unpinned is a reasonable tradeoff, though pinning them too would be the most rigorous approach.
Minor note: martinbeentjes/npm-get-version-action is pinned to a commit on main rather than a release tag, which means it could be any arbitrary commit. Worth confirming that specific commit is the one you intend (i.e., the HEAD of main at time of authoring).
|
Thanks @mahdirajaee, appreciate the thorough review. Good catch on And agreed on the first-party GitHub actions, the trust model is different there, which is why we left those unpinned. I've been researching these attack vectors across GitHub for about 4 weeks now and built an open source engine to scan for them: Runner Guard, then going out and putting PRs on things I found. Been a long 4 weeks. Also put up a link to my research on Twitter if you're interested.
|
Re-submission of #10186. Had a problem with my fork and had to delete it, which closed the original PR. Apologies for the noise.
Summary
This PR pins all GitHub Actions to immutable commit SHAs instead of mutable version tags.
Changes by file
A note on internal action pinning
This PR pins all actions including org-owned ones. Best practice is to pin everything — the tj-actions/changed-files attack was an internally maintained action that was compromised, and every repo referencing it by tag silently executed attacker code. That said, it's your codebase. If you'd prefer to leave org-owned actions unpinned, let us know and we'll adjust the PR.
How to verify
Review the diff — each change is mechanical and preserves workflow behavior:
action@v3becomesaction@abc123 # v3— original version preserved as commentI wrote a scanner called Runner Guard and open sourced it here.
If you have any questions, reach out. I'll be monitoring comms.
- Chris Nyhuis (dagecko)