-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scope GITHUB_TOKEN
permissions
#4084
Conversation
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
A new generated diff is ready to view.
A new doc preview is ready to view. |
6bc6126
to
826d538
Compare
A new generated diff is ready to view.
A new doc preview is ready to view. |
actions: read | ||
contents: read | ||
id-token: write | ||
pull-requests: read |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to educate myself, what's the reason for pull-requests
to be set to read
when it was previously none
? Does that mean the workflow needs to read metadata for a pull request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is because the dry-run release calls release.yml
which in turn invokes ci.yml
which needs the pull-request
permission. The GH permissions of the parent job are inherited by the child job and even if that child job skips the step that actually uses that permission it still needs it from the parent.
Interestingly I believe it was previously set to write
rather than none
since our previous default token permissions were set to:
Read and write permissions
Workflows have read and write permissions in the repository for all scopes.
As a side note, I think I will probably have to increase the pull-request
permissions to write
since the prod release will need that to cut the backport PR and prod/dry-run release both share the underlying release.yml
action definition. Going to attempt a prod release once this is merged to finalize those permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must've been painful to go through all of these. Thanks for the update!
Description
This PR does a few things related to scoping our tokens:
- uses: GitHubSecurityLab/actions-permissions/monitor@v1
to most of our actions so we can get ongoing summaries of the permissions each action is using. Some actions, like Windows tests and the TLS tests, are excluded because they are not supported or the proxy it uses breaks the test.permissions
scoping to various jobs that need it.Testing
Note: I did not test the prod release workflow for obvious reasons. It might need permissions added next time it is invoked. I will cut a release as a follow up to this PR to see if anything needs updating
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.