fix(ci): pin the turbo cache action and gate e2e on writable reviewers - #315
Merged
Conversation
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
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.
Came out of auditing the repo for real vulnerabilities alongside the four Dependabot alerts in #309. The library source is clean. These two are in CI.
The turbo cache action runs next to NPM_TOKEN on a mutable tag
dtinth/setup-github-actions-caching-for-turbo@v1is the only third-party action here that isn'tactions/*orpnpm/*. It sits on a personal account and hasn't been pushed to since June 2024.@v1is a tag that account can repoint at any time.In
release.ymlit runs at step 6 of the release job. Fourteen steps later the same job writes the npm publish token to~/.npmrcin plaintext and runspnpm run release. It also leaves an HTTP server on localhost:41230 alive for the whole job, as the same UID. Whoever controls that tag controls what is running when the publish credentials hit disk, and this pipeline publishes to npm with no human in the loop.In
docs.ymlthe same action runs in a job holdingpages: writeandid-token: write.Pinned both to
cc723b4600e40a6b8815b65701d8614b91e2669e, which is v1.3.0 and whatv1already resolves to, so nothing about the run changes. The 21:05 docs deploy log shows the runner fetching that exact SHA.Left alone:
actions/*andpnpm/*stay on tags. Those are GitHub-owned and pnpm-owned. Pinning them means 25 moreuses:lines carrying SHAs across 11 distinct actions, and a Renovate PR for every one of them, against a much smaller reduction.Any GitHub account can unlock the 45-minute macOS job
The e2e approval gate reads the PR's reviews and takes the latest APPROVED or CHANGES_REQUESTED, with no filter on who wrote it:
This repo is public, so anyone can submit an approving review. Two throwaway accounts, one to open a fork PR and one to approve it, run 45 minutes of
macos-26on code they wrote.Nothing leaks. I checked whether this is a pwn request and it isn't: GitHub treats
pull_request_reviewon a fork PR the same aspull_request, so no secrets reach the runner,GITHUB_TOKENis read-only, the job declarescontents: readand references no secrets, and the caches are scoped torefs/pull/N/merge. The cost is billed compute.Added an
author_associationfilter for OWNER, MEMBER and COLLABORATOR. An outsider's approval now can't override the owner's CHANGES_REQUESTED, and an outsider's approval on its own leaves the gate closed.Also quoted
"$NPM_TOKEN"on thenpm config setline, which clears one shellcheck SC2086.Verification
npm config setline is gone.v1dereferences to the pinned SHA, and the last real run fetched the same oneNot done here
release.ymlstill doesnpm config set //registry.npmjs.org/:_authToken "$NPM_TOKEN", which leaves the token in~/.npmrcfor the rest of the job. The env-var placeholder form npm supports would keep it out of the file, but that is the publish path and I'm not changing it in the same session as a release. The pin above removes the vector that made it reachable. Worth doing on its own, with a dry run behind it.