FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

MINOR: Add missing permission to milestone assignment bot by lidavidm · Pull Request #673 · apache/arrow-java · GitHub

/ arrow-java Public

MINOR: Add missing permission to milestone assignment bot - #673

Merged
lidavidm merged 1 commit into
apache:mainfrom
lidavidm:minor-perm
Jun 2, 2025
ConversationCommits (1)ChecksFiles changed
Merged

MINOR: Add missing permission to milestone assignment bot#673
lidavidm merged 1 commit into
apache:mainfrom
lidavidm:minor-perm

Conversation

lidavidm commented Mar 13, 2025

Copy link
Copy Markdown
Member

What's Changed

This step needs permissions to write to issues so we can set the milestone.

lidavidm marked this pull request as ready for review March 13, 2025 00:41

This comment has been minimized.

Sign in to view
lidavidm added the chore PRs that make misc changes. label Mar 13, 2025
github-actions Bot added this to the 18.3.0 milestone Mar 13, 2025
kou approved these changes Mar 13, 2025
View reviewed changes

kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1

Comment thread .github/workflows/dev_pr.yml
Comment on lines 81 to +86
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
permissions:
contents: read
issues: write
pull-requests: write

kou Mar 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jbonofre Mar 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 ?

lidavidm Mar 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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?

jbonofre Mar 13, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

lidavidm Mar 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jbonofre Mar 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

CalvinKirs Apr 3, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

lidavidm Apr 3, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

CalvinKirs Apr 3, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jbonofre May 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I double checked and it looks good to me.

martin-traverse mentioned this pull request Mar 26, 2025
Merged
CalvinKirs reviewed Apr 3, 2025
View reviewed changes
Comment thread .github/workflows/dev_pr.yml
issues: write
pull-requests: write
run: |
./.github/workflows/dev_pr_milestone.sh "${GITHUB_REPOSITORY}" ${{ github.event.number }}

CalvinKirs Apr 3, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

jbonofre approved these changes May 8, 2025
View reviewed changes
Comment thread .github/workflows/dev_pr.yml
Comment on lines 81 to +86
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
permissions:
contents: read
issues: write
pull-requests: write

jbonofre May 8, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I double checked and it looks good to me.

jbonofre commented May 8, 2025

Copy link
Copy Markdown
Member

@CalvinKirs are you good with this PR ?

wgtmac modified the milestones: 18.3.0, 18.4.0 May 13, 2025

CalvinKirs commented May 30, 2025

Copy link
Copy Markdown
Member

@CalvinKirs are you good with this PR ?↳

LGTM, Sorry for the delay!

lidavidm commented Jun 2, 2025

Copy link
Copy Markdown
Member Author

Thanks Calvin & JB for double-checking things! Rebased and will merge

lidavidm merged commit fad4e14 into apache:main Jun 2, 2025
lidavidm deleted the minor-perm branch June 2, 2025 02:42
kou mentioned this pull request Oct 8, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

kou kou approved these changes
jbonofre jbonofre approved these changes
laurentgo Awaiting requested review from laurentgo laurentgo is a code owner
wgtmac Awaiting requested review from wgtmac wgtmac is a code owner
+1 more reviewer
CalvinKirs CalvinKirs approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

chore PRs that make misc changes.

Projects

None yet

Milestone

19.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL