-
Notifications
You must be signed in to change notification settings - Fork 843
actions: Set permissions in linting and gardening workflows #46067
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
actions: Set permissions in linting and gardening workflows #46067
Conversation
I'm not sure quite how much I trust `GitHubSecurityLab/actions-permissions/monitor`. It identified most of these, but it didn't cover all code paths. What it did find: * `dorny/paths-filter` needs `pull-requests: read`. * repo-gardening seems to need `issues: write` and `pull-requests: write`. What it didn't: * `actions/checkout` supposedly needs `contents: read`, but it wasn't flagged. Maybe it only needs it for private repos? * Our `./.github/actions/turnstile` should need `actions: read`, but it wasn't flagged. Maybe it only needs it for private repos? * Linting / Phan stubs / Warn about stubs will need `pull-requests: write`, which wasn't flagged because it was never triggered. * As for repo-gardening's project stuff, AFAICT all the project stuff is org-level rather than repo-level and so doesn't have permissions. 🤷
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
tbradsha
left a comment
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.
Seems safe enough.
| # dorny/paths-filter | ||
| pull-requests: read |
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.
It seems steps can't have permissions, and they just inherit from the job (or the workflow). Is that correct?
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.
https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax only documents workflow-level and job-level, no step-level.
| group: gardening-${{ github.event_name }}-${{ github.event.action }}-${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.ref || github.run_id }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: |
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.
Is it worth updating projects/github-actions/repo-gardening/README.md with this info?
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.
Probably not this specifically. But either documenting that the action as a whole needs these permissions, or documenting which automations within the action need which permissions, would probably be useful if someone wants to do it.
Closes MONOREP-256
Proposed changes:
I'm not sure quite how much I trust
GitHubSecurityLab/actions-permissions/monitor. It identified most of these, but it didn't cover all code paths.Overall it's probably useful as one tool in the toolbox, but can't be relied on to catch everything.
What it did find:
dorny/paths-filterneedspull-requests: read.issues: writeandpull-requests: write.What it didn't:
actions/checkoutsupposedly needscontents: read, but it wasn't flagged. Maybe it only needs it for private repos?./.github/actions/turnstileshould needactions: read, but it wasn't flagged. Maybe it only needs it for private repos?pull-requests: write, which wasn't flagged because it was never triggered.Other information:
Jetpack product discussion
Follow-up to #46004
Does this pull request change what data or activity we track or use?
No
Testing instructions:
No idea how to test the gardening project parts. Maybe @jeherve knows?Oh, that part uses a different token (triage_projects_token), so the permissions in the workflow don't matter for it.