Skip to content
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.

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

⚠️ Not working with forks #13

Open
samuelmeuli opened this issue Jan 19, 2020 · 19 comments
Open

⚠️ Not working with forks #13

samuelmeuli opened this issue Jan 19, 2020 · 19 comments
Labels
blocked

Comments

@samuelmeuli
Copy link
Collaborator

@samuelmeuli samuelmeuli commented Jan 19, 2020

Creating annotations and auto-fixes works as expected when the code is on a branch in the same repository.

Unfortunately, it currently doesn't seem work with pull requests from forks: The action has no permission to push auto-fix changes or create annotations.

@samuelmeuli
Copy link
Collaborator Author

@samuelmeuli samuelmeuli commented Feb 22, 2020

It seems like this is impossible with GitHub's current token scopes.

The action needs permissions for two operations:

  • Permission to create annotations with GitHub's Check Runs API to display the user's linting errors
  • Permission to push changes with Git (if auto_fix: true)

If the action is triggered on changes to the main repository, it will work because the GITHUB_TOKEN has permissions to modify that repository. On forks, however, this becomes tricky to impossible:

push event pull_request event
commit to main repo [Expected behavior] Annotations and auto-fixing work [Expected behavior] Annotations and auto-fixing work
commit to fork [Expected behavior] User needs to manually activate GitHub Actions on the fork. Even then, annotations will not display on PRs from that fork. [Unexpected behavior] No permission to push auto-fix changes to the fork. No permission to create annotations.

Note that the pull_request event always runs on the main repository, even for pushes to the fork. The token scopes are defined accordingly.

If anybody is aware of a workaround, I'd be very happy to hear it :)

Related: actions/checkout#124 (comment)

samuelmeuli added a commit that referenced this issue Feb 22, 2020
Only works partly due to GitHub's token scopes. See #13
@samuelmeuli samuelmeuli removed their assignment Feb 22, 2020
@samuelmeuli samuelmeuli changed the title Auto-fixing not working with PRs from forks Not working with forks Feb 22, 2020
@samuelmeuli samuelmeuli pinned this issue Feb 25, 2020
@samuelmeuli
Copy link
Collaborator Author

@samuelmeuli samuelmeuli commented Feb 27, 2020

All I can do for now is document these limitations in the README and provide more helpful error messages.

If you'd also like to see these permissions changed, please submit feedback to GitHub.

@MarvinT
Copy link

@MarvinT MarvinT commented Jul 3, 2020

Would it be possible to catch the 403 error when the PR is coming from a fork so it doesn't mark the action as having failed?

@ocean90 ocean90 changed the title Not working with forks ‼️ Not working with forks Jul 30, 2020
@ocean90 ocean90 changed the title ‼️ Not working with forks ⚠️ Not working with forks Jul 30, 2020
@ocean90
Copy link
Member

@ocean90 ocean90 commented Aug 6, 2020

GitHub introduced a new pull_request_target event which might help here, see https://github.blog/2020-08-03-github-actions-improvements-for-fork-and-pull-request-workflows/#improvements-for-public-repository-forks

@1212gmartinez
Copy link

@1212gmartinez 1212gmartinez commented Aug 26, 2020

@samuelmeuli Thanks for making this easy to use library!

Have you had any chance to consider supporting github's pull_request_target.
It would be great to have the fork support available.

@ocean90
Copy link
Member

@ocean90 ocean90 commented Aug 27, 2020

@1212gmartinez There's a WIP feature branch for this which you can try: https://github.com/wearerequired/lint-action/tree/feature/pull_request_target

You should be able to use it with wearerequired/lint-action@feature/pull_request_target.

@1212gmartinez
Copy link

@1212gmartinez 1212gmartinez commented Aug 27, 2020

Awesome, thanks!

arnavb added a commit to InVisionAR/web-frontend that referenced this issue Sep 15, 2020
zrthxn added a commit to zrthxn/vaccination-slot-notifier that referenced this issue May 19, 2021
abbasidaniyal pushed a commit to Beta-Persei/vaccination-slot-notifier that referenced this issue May 21, 2021
* New placeholders

* Allow one email or phone per district or pincode

* transaction test

* Fix assertions

* Fix formatting issues

* Run black only on push, not PR. See issue wearerequired/lint-action#13

* Messages UI

*Alert Messages UI

* Fix test

* Test case and ago group
@github-actions
Copy link
Contributor

@github-actions github-actions bot commented May 27, 2021

A stale label has been added to this issue because it has been open 15 days with no activity. To keep this issue open, add a comment within 5 days.

@github-actions github-actions bot added the stale label May 27, 2021
@ocean90 ocean90 removed the stale label May 27, 2021
javila35 added a commit to javila35/cvp that referenced this issue Jun 9, 2021
@Thatkookooguy
Copy link

@Thatkookooguy Thatkookooguy commented Jun 10, 2021

Hey! Was this solved? I see that the pull_request_target branch has been merge and pull_request_target is being referenced in code.

But still when I try to use pull_request_target with this aciton and a fork opens a PR, I get the 404 error.

see this dependabot PR for example

will it work if I change github_token: ${{ secrets.BOT_TOKEN }}to github_token: ${{ secrets.GITHUB_TOKEN }}? is that related to the different things you can do on a fork compared to repo PRs?

@ocean90
Copy link
Member

@ocean90 ocean90 commented Jun 10, 2021

@Thatkookooguy Pull requests by Dependabot don't have access to secrets anymore, see dependabot/dependabot-core#3253 (comment).
You should be able to use the the default GITHUB_TOKEN secret in combination with pull_request_target. Consider also limiting the default permissions. For a working example see test-action.yml.

@jnm2
Copy link

@jnm2 jnm2 commented Jun 10, 2021

Please consider using workflow commands (stdout) so that tokens are not needed and pull_request_target is not needed, as mentioned above (#13 (comment)).

@Thatkookooguy
Copy link

@Thatkookooguy Thatkookooguy commented Jun 11, 2021

@ocean90 thanks for the tip! will definitely check it out.

@jnm2
Copy link

@jnm2 jnm2 commented Jun 11, 2021

@ocean90 I'm confused. Why did you downvote?

@moodysalem
Copy link

@moodysalem moodysalem commented Nov 29, 2021

Please consider using workflow commands (stdout) so that tokens are not needed and pull_request_target is not needed, as mentioned above (#13 (comment)).

This particular change seems like it should not be blocked, but won't help with auto_fix: true

@samuelmeuli Will you take a PR? Seems like this piece of code needs changing:

async function createCheck(linterName, sha, context, lintResult, neutralCheckOnWarning, summary) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked
Projects
None yet
Development

No branches or pull requests

11 participants