ci: scope workflow permissions to what each job uses - #2046
Open
arpitjain099 wants to merge 1 commit into
Open
arpitjain099 wants to merge 1 commit into
arpitjain099 wants to merge 1 commit into
Conversation
ci.yml and update-v8.yml both set permissions: write-all at the top level. Only three steps in the repository need a write-scoped GITHUB_TOKEN, all of them softprops/action-gh-release on tag refs, in build, build-windows-arm64 and build-windows-arm64-simdutf. Set contents: read at the top of both workflows and grant contents: write on those three jobs. update-v8.yml needs nothing beyond read: every write it performs goes through DENOBOT_PAT, and the workflow token is only used by actions/checkout. The publish job authenticates to crates.io with CARGO_REGISTRY_TOKEN and downloads artifacts from its own run, neither of which needs a write scope. actions/cache authenticates to the cache service with the runner's own token rather than GITHUB_TOKEN, so it is unaffected. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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.
Follow-up to #1981, where @bartlomieju pointed out that
release.ymlwas the wrong target and that the top-levelwrite-allon the other workflows is the actual exposure.ci.ymlandupdate-v8.ymlboth setpermissions: write-all. Across the whole repository three steps need a write-scopedGITHUB_TOKEN, all of themsoftprops/action-gh-releasegated onstartsWith(github.ref, 'refs/tags/'). This setscontents: readat the top of both workflows and grantscontents: writeon the three jobs that hold those steps.One correction to what I wrote on #1981: I said the release steps were in
buildandbuild-asan. They are inbuild,build-windows-arm64andbuild-windows-arm64-simdutf.build-asanhas no release step and gets no elevation here.What I checked for everything else that touches a token:
update-v8.ymlperforms every write throughDENOBOT_PAT, including thegit remote set-urland theGITHUB_TOKENit passes toauto_update_v8.ts. The workflow token is used only byactions/checkout, socontents: readis sufficient andwrite-allwas buying nothing, the same point you made aboutrelease.yml.publishjob authenticates to crates.io withCARGO_REGISTRY_TOKEN, and itsactions/download-artifactreads artifacts from its own run, which needs no added scope.actions/cacheauthenticates to the cache service with the runner's own token rather thanGITHUB_TOKEN, so restore and save are unaffected by narrowing the workflow token.release.ymlis untouched, since it has nopermissionsblock and, as you said, its authority comes from the PAT either way.Worth being explicit about what this does and does not buy, since that was the flaw in my last PR. For pull requests from forks GitHub already issues a read-only token no matter what this key says, so this changes nothing for outside contributions. It applies to push and tag runs on the repository itself, where a compromised action currently inherits full write. That is the case the SHA-pinning PR is aimed at, and I will send that separately.
No behavioural change is expected. If a job does turn out to need a scope I have missed, the failure is a clear 403 from the API rather than anything silent.