-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Potential fix for code scanning alert no. 12: Workflow does not contain permissions #3822
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
base: master
Are you sure you want to change the base?
Conversation
…in permissions Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #3822 +/- ##
=======================================
Coverage 99.45% 99.45%
=======================================
Files 3 3
Lines 183 183
=======================================
Hits 182 182
Misses 1 1 ☔ View full report in Codecov by Sentry. |
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.
ok
LOL. Very low quality PR. |
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.
What about changing the default in workflow permissions?
https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
It should be changed for release.yml and other actions which does need write permission.
I guess this is a better approach (setting it to read by default)
@mdtro @oioki Heya, what do you folks think about what @aminvakil said above? This is a very low hanging fruit, but raises 8 security issues here: https://github.com/getsentry/self-hosted/security/code-scanning ![]() |
@aldy505 - Setting the permissions explicitly would still be considered best practice and something we recommend even if the token is default set to read-only. Too many footguns with implicit behavior. :) |
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.
@aldy505 - Setting the permissions explicitly would still be considered best practice and something we recommend even if the token is default set to read-only. Too many footguns with implicit behavior. :)
Agree!
Let's do this and change default permission later (or not).
Note
This was me playing around with GH Copilot Security. Feel free to merge if you think this helpful. Otherwise, please just close it.
Potential fix for https://github.com/getsentry/self-hosted/security/code-scanning/12
To fix this issue, add a
permissions:
block at the top level of the workflow file, directly under thename:
line and before theon:
block. This block should specify the minimal permissions required for the jobs in this workflow. Since the jobs only perform test-related tasks and do not push code or manage issues, settingcontents: read
is the safest and most appropriate minimal permission set. No changes to existing functionality are necessary, as the jobs should not require write permissions.Suggested fixes powered by Copilot Autofix. Review carefully before merging.