ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
88 stars 44 forks source link

Action doesn't work in forks #13

Closed vadi2 closed 3 years ago

vadi2 commented 3 years ago

Got a somewhat unintuitive error with the action:

github.GithubException.GithubException: 403 {"message": "Resource not accessible by integration", "documentation_url": "https://docs.github.com/rest"}

What does it mean? See https://github.com/Mudlet/Mudlet/pull/4885/checks?check_run_id=1998372846#step:11:1297

ZedThree commented 3 years ago

Is it because the PR is from a fork? It looks like it's failing when creating the review. One option is to disable the check on forks, using something like: https://github.community/t/dont-run-actions-on-pull-request-from-fork/17560/4

There may be a way to give frequent collaborators access to create reviews -- I'm not sure how to do that myself

vadi2 commented 3 years ago

Hm, but that's not how Github works: anyone should be able to make a PR from their fork without needing any kind of access. Requiring access is non-scalable and wouldn't make these kinds of actions really work - does Github really not permit this?

ZedThree commented 3 years ago

It's not the PR itself, its the token used to create the review which only has read permissions when used from a forked repo: https://docs.github.com/en/actions/reference/authentication-in-a-workflow#permissions-for-the-github_token So it can do everything but write to the repo -- review comments count as a "write", which is why it's failing when trying to create the review.

I think the issue is basically in order to post the review comments, the user would need (some level of) write permissions, but if the PR is from a fork, they could've modified the action to do other things.

It looks like there's a similar limitation on creating Checks Annotations, so even if I switched it to them, there would still be the same problem.

I could make it an option to do everything but post the review comments, that way it'd still be able to run on forks and you'd have the option of failing if there were clang-tidy warnings, you'd just have to go into the check to see them.

vadi2 commented 3 years ago

What if a custom token is generated and passed to the action which has the review write capability, would that work?

vadi2 commented 3 years ago

Solved it in the end with the pull_request_target hook :+1: