search

SAME-Project / same-project

https://sameproject.ml/
Apache License 2.0
19 stars 8 forks source link

Enable forked repos to contribute PRs without permission errors #61

Closed lukemarsden closed 2 years ago

lukemarsden commented 2 years ago

example: https://github.com/SAME-Project/same-project/runs/5228905644?check_suite_focus=true

annajung commented 2 years ago

I'll pick this up and try to find a solution or a workaround

annajung commented 2 years ago

According to the GitHub blog post, they introduced pull_request_target that behaves the almost same way as the pull_request event. Unlike use of pull_request that limits the GITHUB_TOKEN to read access for forked repo, pull_request_target will grant write permission for forked repos that will allow the bot to post a comment on the pull request.

However, given that write access is given to anyone who forks the repo, it is not recommended to run the workflow on untrusted code and recommends the use of labels to indicate PR has been vetted.

@lukemarsden @aronchick @js-ts What do you think about adding needs-ok-to-test labels (automatically using github actions) to indicate that PR needs to be vetted and once vetted, those who have access to add labels can add ok-to-test label to trigger the workflow.

lukemarsden commented 2 years ago

Yes please!

On Fri, 18 Mar 2022, 14:30 Anna, @.***> wrote:

According to the GitHub blog post https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/, they introduced pull_request_target https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target that behaves the almost same way as the pull_request event. Unlike use of pull_request that limits the GITHUB_TOKEN to read access for forked repo, pull_request_target will grant write permission for forked repos that will allow the bot to post a comment on the pull request.

However, given that write access is given to anyone who forks the repo, it is not recommended to run the workflow on untrusted code and recommends the use of labels to indicate PR has been vetted.

@lukemarsden https://github.com/lukemarsden @aronchick https://github.com/aronchick @js-ts https://github.com/js-ts What do you think about adding needs-ok-to-test labels (automatically using github actions) to indicate that PR needs to be vetted and once vetted, those who have access to add labels can add ok-to-test label to trigger the workflow.

— Reply to this email directly, view it on GitHub https://github.com/SAME-Project/same-project/issues/61#issuecomment-1072466521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACATUQRBNZUJZJTSAAJ7ZDVASHQRANCNFSM5PKVOZ2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

annajung commented 2 years ago

Sorry for the delay, was out all last week. Created a PR to test out the actions https://github.com/SAME-Project/same-project/pull/87, looks like it might have been fixed. Could someone help add the label ok-to-test to test out the actions?

annajung commented 2 years ago

We can close this issue now (I don't have permission to close)

Thanks!