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

Action fails on forked repos #2

Closed
Vlaaaaaaad opened this issue Sep 29, 2019 · 10 comments
Closed

Action fails on forked repos #2

Vlaaaaaaad opened this issue Sep 29, 2019 · 10 comments

Comments

@Vlaaaaaaad
Copy link
Collaborator

@Vlaaaaaaad Vlaaaaaaad commented Sep 29, 2019

For some reason the action does not output anything if running for a forked repository.

Tflint does output the errors correctly, but reviewdog does not post a comment or any output.

Example of run without anything happening: https://github.com/Castravete/repo-for-fork-testing/pull/7/checks

@haya14busa any idea how I can debug this?

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

Note that it reports result to GitHub Actions log consle for Pull Requests from fork repository because due to GitHub Actions restriction, GITHUB_TOKEN for PullRequest from forked repository doesn't have write access to Check API.

https://github.com/reviewdog/reviewdog#option-1-run-reviewdog-from-github-actions-w-secretsgithub_token-experimental

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

And it's same for -reporter=github-pr-review too. GITHUB_TOKEN doesn't have write access.

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

Oh, but it looks like there is a different problem. reviwedog should output something as your comments.

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

I get it. The errors are both outside diff, so reviewdog didn't find new errors and exit with 0.

A new feature in reviewdog reviewdog/reviewdog#332 can report results regardless of that the results are in or outside diff, but it's expected behavior for github-pr-review rerpoter.

@Vlaaaaaaad
Copy link
Collaborator Author

@Vlaaaaaaad Vlaaaaaaad commented Sep 29, 2019

Yup, it's not showing any output which is very weird. Still, even if I get output I fear I'll hit the token permissions issue. I am testing if the personal token workaround is valid or not.

I think it's a diff issue, but I am unsure of what to even do there.

@Vlaaaaaaad
Copy link
Collaborator Author

@Vlaaaaaaad Vlaaaaaaad commented Sep 29, 2019

I get it. The errors are both outside diff, so reviewdog didn't find new errors and exit with 0.

A new feature in reviewdog reviewdog/reviewdog#332 can report results regardless of that the results are in or outside diff, but it's expected behavior for github-pr-review rerpoter.

AHA! So it was a diff issue. I'll do some more tests

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

Still, even if I get output I fear I'll hit the token permissions issue. I am testing if the personal token workaround is valid or not.

You can investigate GitHub Actions spec but there are no workaround unfortunetely.
It's unsafe to allow p-r from fork repository to access secret variable.

@Vlaaaaaaad
Copy link
Collaborator Author

@Vlaaaaaaad Vlaaaaaaad commented Sep 29, 2019

For pull_request_review event a token for a bot user in the base repo can be used to work around the limitation. It's ugly, but it works. See terraform-aws-modules/terraform-aws-eks#541 and the two linked example PRs( https://github.com/Castravete/repo-for-fork-testing/pull/3 and https://github.com/Castravete/repo-for-fork-testing/pull/6).

Buuuut that was for commits. Unsure if a Personal Token can be used for checks -- and from my initial testing it looks like it won't work 😞 Still, I imagine that for comments/ reviews it should be fine so I'll test a bit more.
Any chance the github-check from reviewdog/reviewdog#332 can have a github-review "brother" that instead of using checks can post reviews like github-pr-check has github-pr-review? I imagine that will work with the token hack but I am unsure if it's worth the extra complexity.

@haya14busa
Copy link
Member

@haya14busa haya14busa commented Sep 29, 2019

For pull_request_review event a token for a bot user in the base repo can be used to work around the limitation.

Very Interesting. It should be still unsafe to access a secret variable because malicious users can steal the secret... I didn't try it by myself yet, but it might be better to report it to GitHub.

Any chance the github-check from reviewdog/reviewdog#332 can have a github-review "brother" that instead of using checks can post reviews like github-pr-check has github-pr-review?

Can you elaborate it more? I don't understand your question.
Note that Review API doesnt' support to post comments anywhere but it can just report comments in diff, so we cannot use github-pr-review (Review API) to post comments outside diff.

@Vlaaaaaaad
Copy link
Collaborator Author

@Vlaaaaaaad Vlaaaaaaad commented Sep 29, 2019

For pull_request_review event a token for a bot user in the base repo can be used to work around the limitation.

Very Interesting. It should be still unsafe to access a secret variable because malicious users can steal the secret... I didn't try it by myself yet, but it might be better to report it to GitHub.

For the pull_request_review event the Actions run in the base repo not in the fork so it is safe. I think? Buuuut I can modify the action from my fork and I could do evil stuff. Huh. I'll check and report.

Any chance the github-check from reviewdog/reviewdog#332 can have a github-review "brother" that instead of using checks can post reviews like github-pr-check has github-pr-review?

Can you elaborate it more? I don't understand your question.
Note that Review API doesnt' support to post comments anywhere but it can just report comments in diff, so we cannot use github-pr-review (Review API) to post comments outside diff.

Ah. My logic was this: user tokens cannot be used for Checks but they can be used to post reviews and/or comments. What if there was a github-check that instead of posting an annotation( which cannot be done with a personal token) would comment instead?
But sine the Review API can just post comments in diff and github-check is not using a diff I think the idea is invalid.

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

No branches or pull requests

2 participants