Skip to content

MINOR: Add missing permission to milestone assignment bot #673

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 2, 2025
Merged

MINOR: Add missing permission to milestone assignment bot #673

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/dev_pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,9 @@ jobs:
if: '! github.event.pull_request.draft'
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
permissions:
contents: read
issues: write
pull-requests: write
Comment on lines 81 to +86
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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 @ 💬

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the script is invoking the GitHub CLI to do things, and the GitHub CLI needs a token from an environment variable

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lidavidm let me double check with the Infra again (sorry I forgot).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops...we're not executing the checkout operation... please disregard me

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I double checked and it looks good to me.

run: |
./.github/workflows/dev_pr_milestone.sh "${GITHUB_REPOSITORY}" ${{ github.event.number }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/

Loading