-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
CI: Add GitHub token permissions for workflows #36325
Conversation
.github/workflows/cspell.yml
Outdated
jobs: | ||
cspell: | ||
permissions: | ||
contents: read # for streetsidesoftware/cspell-action to fetch files for commit |
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.
I wonder, why is this needed since you have specified the same globally?
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.
Hi @XhmikosR this is needed because there are two permissions specified for that job at the job level. When permissions are specified at the job level, only those listed in the job are provided to the token [1]. So if contents: read
is not listed again, the job will only get pull-requests: read
.
If no permissions are listed at the job level, it gets the permissions from the workflow level. If the job only needed contents: read
, we could have omitted the permissions section at the job level altogether.
Please let me know if you have follow up questions. Thanks!
permissions:
contents: read # for streetsidesoftware/cspell-action to fetch files for commit
pull-requests: read # for streetsidesoftware/cspell-action to fetch commits for PR
[1] When the permissions key is used, all unspecified permissions are set to no access, with the exception of the metadata scope, which always gets read access. https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token
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.
@XhmikosR any update on the pull request review? Please let me know if you have any follow up questions.
Thanks for the PR and sorry it took so long! I like the changes, I'm just a little hesitant it might be too strict and we might have broken something here since not all workflows run on contributor PRs. |
Let's try this and see how it goes. |
GitHub asks users to define workflow permissions, see https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/ and https://docs.github.com/en/actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token for securing GitHub workflows against supply-chain attacks.
The Open Source Security Foundation (OpenSSF) Scorecards also treats not setting token permissions as a high-risk issue.
This PR adds minimum token permissions for the GITHUB_TOKEN using https://github.com/step-security/secure-workflows.
This project is part of the top 100 critical projects as per OpenSSF (https://github.com/ossf/wg-securing-critical-projects), so fixing the token permissions to improve security.