-
Notifications
You must be signed in to change notification settings - Fork 205
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
Apply some security hardening, as recommended by OSSF scorecard #595
Conversation
NWilson
commented
Dec 4, 2024
- Use hashes to pin versions of third-party Actions used in CI
- Use GitHub's dependabot to do a monthly update of the Actions
- Restrict all Workflows to use a read-only token (except for the steps which need to upload a SARIF file)
Will this make our life easier or more difficult? Is this needed for CI security, or what is the actual gain here? |
It's needed for CI security. As it stands, we depend on some third party github actions, and if someone pushed malicious code to those repos, they would be able to perform pretty much arbitrary actions on our PCRE2 repo, the next time CI runs. It should be much more locked down. |
These changes won't make our life more difficult (nor any easier). |
This is disturbing. Can a CI write the repository in any way? Where can I disable that? |
Yes, sadly. The GitHub Actions have a very permissive token by default - a CI job can happily check out the code, make some changes, and push them to the master branch (and even rewrite history on master). The token that's used by the git checkout job can happily push code too. And worse, it can even perform other actions on the GitHub organisation using the HTTP API (or GitHub CLI). Adding |
Thank you for the explanation. I found this in Settings/Actions/General Workflow permissions Choose the default permissions granted to the GITHUB_TOKEN when running workflows in this repository. You can specify more granular permissions in the workflow using YAML. Learn more about managing permissions. Read and write permissions Read repository contents and packages permissions @PhilipHazel may I change this to read-only? Honestly, I am shocked that writing is the default. |
Yes, yes, do go ahead. I too am a bit shocked. |
Config updated. Back to the patch, I am neutral, we can land it. Security is not my strength. |
I've broken the Scorecard scanning - because it's designed to run on master but not on PRs! So it wasn't really possible to discover problems before merging. Hmm. I'll sort it out, don't worry. |