Skip to content
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

Merged
merged 15 commits into from Dec 16, 2022

Conversation

varunsh-coder
Copy link
Contributor

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.

@varunsh-coder varunsh-coder changed the title ci: Add GitHub token permissions for workflows #14539 ci: Add GitHub token permissions for workflows May 11, 2022
jobs:
cspell:
permissions:
contents: read # for streetsidesoftware/cspell-action to fetch files for commit
Copy link
Member

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

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.

@XhmikosR XhmikosR changed the title ci: Add GitHub token permissions for workflows CI: Add GitHub token permissions for workflows Nov 12, 2022
@XhmikosR XhmikosR added the CI label Nov 12, 2022
@XhmikosR XhmikosR self-assigned this Nov 12, 2022
@XhmikosR
Copy link
Member

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.

@XhmikosR
Copy link
Member

Let's try this and see how it goes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants