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 #47652

Merged
merged 3 commits into from
Jul 11, 2022

Conversation

varunsh-coder
Copy link
Contributor

This PR adds minimum token permissions for the GITHUB_TOKEN using https://github.com/step-security/secure-workflows.

GitHub recommends defining minimum GITHUB_TOKEN permissions for securing GitHub Actions 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.

Signed-off-by: Varun Sharma [email protected]

  • closes #xxxx (Replace xxxx with the Github issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Signed-off-by: Varun Sharma <[email protected]>
@mroeschke
Copy link
Member

If we set Read repository contents permission in our repository settings, would we then only need to specify the write permissions when needed?

@mroeschke mroeschke added the CI Continuous Integration label Jul 10, 2022
@mroeschke mroeschke added this to the 1.5 milestone Jul 10, 2022
jobs:
stale:
permissions:
issues: write # for actions/stale to close stale issues
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not closing the prs, the not only adds a label stale

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 @phofl, is your ask to remove or change the comment? Just wanted to confirm before making the change.

I can remove the comment or change it to to label stale PRs.

Also, looks like this workflow does not update stale issues. If that is the case, I can remove the issues: write permission.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This action only adds a stale label and writes one comment to a PR, so whatever permission are needed to perform those actions can be set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated stale-pr.yml. Removed issues: write since issues are not being updated based on the config. Also removed the comment for pull-requests: write.

@varunsh-coder
Copy link
Contributor Author

If we set Read repository contents permission in our repository settings, would we then only need to specify the write permissions when needed?

Hi @mroeschke yes, that is true. At the same time, it is a best practice to set the permissions in the workflow files explicitly.

  • The permissions are encapsulated with the workflow file, so easier to review and version (same reason applies for other as-code files)
  • If someone forks the repo, the repository settings are not copied over, so the workflow in the forked repo is not secure. Whereas if the workflow file has explicit permissions, the settings are copied over.

So, setting Read repository contents permission in the repo setting is a good idea as future workflows in the repo will be secure-by-default. In addition, I would also recommend setting permissions explicitly in the workflow files.

@mroeschke
Copy link
Member

Thanks for the explanation. Agreed to make these explicit in the the yaml files then

Signed-off-by: Varun Sharma <[email protected]>
@mroeschke mroeschke merged commit 25d1c17 into pandas-dev:main Jul 11, 2022
@mroeschke
Copy link
Member

Thanks @varunsh-coder

yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
* ci: add GitHub token permissions for workflows

Signed-off-by: Varun Sharma <[email protected]>

* trim trailing whitespace

Signed-off-by: Varun Sharma <[email protected]>

* Update stale-pr.yml

Signed-off-by: Varun Sharma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants