-
-
Notifications
You must be signed in to change notification settings - Fork 398
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
Authentication with repository API tokens is unsafe for PR builds #182
Comments
Yes, It's actually one of the big troublesome problems for reviewdog to recommend users to use it for OSS projects. One minor correction: many CI services treated PR from forked repo as untrusted code and do not pass secret env var, but PR from same repo can use secret vars. You might be interested in GitHub Actions because they provide GITHUB_TOKEN which has read/write access for PR from the same repo and has only read access from forked repo. github-pr-check reporter currently fallback to use GitHub Actions log console as output target if p-r is from forked repo. |
Thanks for the info and correction 👍 I'll likely look into this at some point but I'm short of time currently. If you think there's nothing to act on in this issue feel free to close it. |
This release solved the problem for GitHub Actions. https://github.com/reviewdog/reviewdog/releases/tag/v0.9.15 |
Reviewdog is specifically designed to run in CI's PR/MR builds, in order to generate code reviews.
In these conditions it's not safe to use private environment variables. PR's should be regarded as "untrusted code" and not run in CI's with access to private information before they're accepted (which is what the CI build does).
Even if the variables aren't being added as clear text, the author of the PR can add a keylogger and steal your variables.
Travis has enabled a protection against this, but this also means reviewdog can't access the key.
It's possible to avoid this by using the "checks"-reporter instead, but I prefer code reviews over checks.
This seems like a great tool that I want to use, but this issue is a deal breaker for me. I think the GitHub App has to do this instead, like with your "checks"-reporter or other solutions like stickler-ci or HoundCI.
The text was updated successfully, but these errors were encountered: