MINOR: Add missing permission to milestone assignment bot #673
Conversation
lidavidm
requested review from
laurentgo,
wgtmac,
kou and
jbonofre
as code owners
This comment has been minimized.
This comment has been minimized.
| env: | ||
| GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| permissions: | ||
| contents: read | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
@jbonofre Can we use this configuration? Is this still satisfied our policy? https://infra.apache.org/github-actions-policy.html
This is what I asked on Zulip: #java-chat > GitHub Action versions alias @ 💬
There was a problem hiding this comment.
Yes, it should not be a problem to write issues/pull-requests.
Do you really need to have GH_TOKEN env variable ? Why not directly using GITHUB_TOKEN ?
There was a problem hiding this comment.
I can rename the env var (it appears gh CLI accepts both), but the question is whether putting it in the environment in the first place is acceptable? From the Apache Infra page, it sounds like this is actually not allowed anymore?
There was a problem hiding this comment.
That's my point (sorry if I wasn't clear): why storing as env variable ?
I discussed with Gavin (from the ASF Infra) to clarify the "triggers" statement on the GitHub Action policy page.
There was a problem hiding this comment.
Because the script is invoking the GitHub CLI to do things, and the GitHub CLI needs a token from an environment variable
There was a problem hiding this comment.
@jbonofre was there any clarification here?
There was a problem hiding this comment.
@lidavidm let me double check with the Infra again (sorry I forgot).
There was a problem hiding this comment.
Since pull_request_target runs in the target repository's context with write access to secrets, directly executing a .sh file from a PR poses a security risk. To ensure the script hasn't been tampered with, we must verify its SHA256 checksum before execution to prevent unintended scripts from running and potential malicious attacks.
There was a problem hiding this comment.
This should be from the main branch, though, not from the PR. (But I guess would it be clearer/safer to have a separate repo of custom actions for the project that we can use and pin?)
There was a problem hiding this comment.
oops...we're not executing the checkout operation... please disregard me
| permissions: | ||
| contents: read | ||
| issues: write | ||
| pull-requests: write | ||
| run: | | ||
| ./.github/workflows/dev_pr_milestone.sh "${GITHUB_REPOSITORY}" ${{ github.event.number }} |
There was a problem hiding this comment.
What's Changed
This step needs permissions to write to issues so we can set the milestone.