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 minimum GitHub token permissions for workflows #7452

Closed
wants to merge 1 commit into from

Conversation

ashishkurmi
Copy link

@ashishkurmi ashishkurmi commented Sep 9, 2022

Description

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

GitHub Actions workflows have a GITHUB_TOKEN with write access to multiple scopes.
Here is an example of the permissions in one of the workflows:
https://github.com/zaproxy/zaproxy/runs/8252201162?check_suite_focus=true#step:1:19

After this change, the scopes will be reduced to the minimum needed for the following workflows:

  • ci.yml
  • crowdin-upload-files.yml
  • handle-release.yml
  • prepare-release-main-version.yml
  • release-live-docker.yml
  • release-main-docker.yml
  • release-main-version.yml
  • release-weekly-docker.yml
  • release-weekly.yml
  • run-integration-tests.yml
  • sonar.yml
  • test-packaged-scans.yml

The following workflow files already have the least privileged token permission set:

  • lock.yml

Motivation and Context

Signed-off-by: Ashish Kurmi akurmi@stepsecurity.io

@kingthorin
Copy link
Member

kingthorin commented Sep 9, 2022

Thanks for tackling this.

I do see that lock.yml specifies specific permissions, however sonar.yml appears wide open unless I’ve missed something?

@ashishkurmi
Copy link
Author

ashishkurmi commented Sep 9, 2022

Thanks for tackling this.

I do see that lock.yml specifies specific permissions, however sonar.yml appears wide open unless I’ve missed something?

Thanks for reviewing my PR! My bad, you are correct, sonar.yml is wide open (I have updated the PR). As it is passing GITHUB_TOKEN as an environment variable to gradlew as part of the 'Sonarcloud Scan' action, I would need to review the utility to see what kind of GitHub API it calls. I can address that issue in a separate PR.

@kingthorin
Copy link
Member

kingthorin commented Sep 9, 2022

@boahc077 Would it be sufficient/better for us to change the default at the org?

Settings Screenshot:
image

@kingthorin
Copy link
Member

kingthorin commented Sep 9, 2022

Don't worry about a follow-up Sonar PR, we've removed setting the token into env, it seems it was unnecessary.
#7454
https://github.com/zaproxy/zaproxy/actions/runs/3022280554

I've confirmed updated details via sonarcloud.io Last analysis: September 9, 2022 at 8:09 AM

Signed-off-by: Ashish Kurmi <akurmi@stepsecurity.io>
@ashishkurmi
Copy link
Author

@boahc077 Would it be sufficient/better for us to change the default at the org?

Settings Screenshot: image

@kingthorin yes, the repo level setting could be turned on. I have this many projects do both as having permissions defined in the workflow is declarative and anyone can verify it (the repo level settings are only accessible to maintainers).

@ashishkurmi
Copy link
Author

Don't worry about a follow-up Sonar PR, we've removed setting the token into env, it seems it was unnecessary. #7454 https://github.com/zaproxy/zaproxy/actions/runs/3022280554

I've confirmed updated details via sonarcloud.io Last analysis: September 9, 2022 at 8:09 AM

This is super awesome! Because of your changes, I was able to calculate minimum token permission for sonar.yml as well and included it in my PR (I squashed all my commits into one). Please take a look when you get a chance.

@thc202
Copy link
Member

thc202 commented Sep 13, 2022

I don't think we should merge this, we should just change the default. (3rd parties can still check the logs to verify which permissions are defined.)
We have many more repos and workflows we should not need to change every single one of them.

@ashishkurmi
Copy link
Author

Hello @thc202, my recommendation will be to merge this PR as you can turn on the GitHub org level token permission setting irrespective of this PR. This PR will not have any impact on it. Also, one of the workflow files already has the token permission set via this method so this PR will make all workflows consistent.

@kingthorin
Copy link
Member

We've decided it makes most sense for us to set this at the org level as Read.
image

Interested third parties can still check recent workflow logs to see that it is indeed set as such. Where/if needed individual workflows can still be set with more permission via yaml declarations.

@boahc077 thank you for bringing this to our attention, feel free to link this comment as evidence of compliance/completion.

@kingthorin kingthorin closed this Sep 15, 2022
@github-actions
Copy link

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants