ci: set minimum amount of permission needed for each workflow #54
Conversation
zimeg
added
security
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
=======================================
Coverage 62.91% 62.91%
=======================================
Files 210 210
Lines 22156 22156
=======================================
+ Hits 13939 13940 +1
- Misses 7131 7132 +1
+ Partials 1086 1084 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
zimeg
left a comment
There was a problem hiding this comment.
📝 A few notes while this is in draft, but I hope to test one more thing before considering it reviewtime!
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
📣 This is enough to cause the LICENSE checks to fail when needed - 6591d1f causes:
https://github.com/slackapi/slack-cli/actions/runs/14578766377/job/40890636282?pr=54
| permissions: | ||
| contents: none |
There was a problem hiding this comment.
📣 Permissions are handled within CircleCI for this workflow:
| permissions: | ||
| contents: none # Permissions are set with an application token |
There was a problem hiding this comment.
📣 The application token is included when needed, and it remains possible to start the job. We might have to wait for an actual update to confirm this though:
https://github.com/slackapi/slack-cli/actions/runs/14578846863/job/40890875279
There was a problem hiding this comment.
📝 More notes on these permissions are included in the MAINTAINERS_GUIDE.md!
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
📣 "write" contents are required to delete a release:
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
📣 Checking out the repo is all that's required with the default token!
An application token helps with other permissions. Testing after the changes of #52-
| permissions: | ||
| checks: write | ||
| contents: read |
There was a problem hiding this comment.
📣 AFAICT "contents" is not required for public repos but seems to be a good practice for making this permission clear.
📚 The "checks" permission is required for the health score: https://github.com/slackapi/slack-health-score
|
@mwbrooks Thanks so much for reviewing once more 🙏 I'm not wanting to admit that some of these permissions have become familiar, but I am glad we can keep track of the actual permissions needed for each job! 🔐 ✨ |
Summary
This PR sets the minimum amount of permission needed for each workflow without breaking the existing steps setup.
Reviewers
Testing is happening within this PR and notes left below! 👾
Requirements