Fix check-labels workflow commenting on forked PRs #101467
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/101467
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit d53e2ab: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
.github/workflows/check-labels.yml
Outdated
| pull_request_target: | ||
| types: [opened, synchronize, reopened, labeled, unlabeled] |
There was a problem hiding this comment.
Let's limit pull_request_target trigger only to PRs that target main branch. And perhaps, to be extra paranoid: only PRs that do not modify any of the .yml files
| pull_request_target: | |
| types: [opened, synchronize, reopened, labeled, unlabeled] | |
| pull_request: | |
| types: [opened, synchronize, reopened, labeled, unlabeled] | |
| branches-ignore: [main] | |
| pull_request_target: | |
| types: [opened, synchronize, reopened, labeled, unlabeled] | |
| branches: [main] |
There was a problem hiding this comment.
For something bad to happen with non-main branch, the attacker needs to have write permissions to the repo.
And with the write permissions, anything can be changes.
But I'm fine with this change if it feels safer.
There was a problem hiding this comment.
Discussed offline, will do the suggested way.
|
@pytorchbot merge |
|
The merge job was canceled. If you believe this is a mistake,then you can re trigger it through pytorch-bot. |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
Using `pull_request_target` allows securely passing the secrets to make comments on a forked PRs. See more about `pull_request_target` in https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/ The change was verified in malfet/deleteme#53 - with `on pull_request` there were no "This PR needs a label" comment, with with `on pull_request_target` the comment can be posted. Pull Request resolved: #101467 Approved by: https://github.com/malfet
Using
pull_request_targetallows securely passing the secrets to make comments on a forked PRs.See more about
pull_request_targetin https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/The change was verified in malfet/deleteme#53 - with
on pull_requestthere were no "This PR needs a label" comment, with withon pull_request_targetthe comment can be posted.