-
-
Notifications
You must be signed in to change notification settings - Fork 368
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
Github Action fails when PR comes from forked repo #918
Comments
Update: something happened and the next push was successful:
Link: https://github.com/sobolevn/itmo-2019/pull/19/checks?check_run_id=222337878 |
This happened again: https://github.com/sobolevn/itmo-2019/pull/24/checks?check_run_id=222778615 |
I've heard reports of issues when the other person doesn't have access to GitHub Actions yet, could be that? |
Yes, that's correct. These users don't have an access yet. |
a solution here might be to report the status using the github actions tooling when running in a github action (there is an env var to detect that) instead of submitting a comment and a commit status. This way, the github API would be used only in read-only mode. |
Good point, Danger supports both the checks api and the comment API - but it looks like both are read-only in that list. Are there other useful feedback mechanisms for an action? |
I think that just dumping everything into |
you can write to stdout (and use the exit code to indicate failure) |
From the docs about secrets in GitHub:
More details, you can reference here: https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets#using-encrypted-secrets-in-a-workflow |
IMO we should be recommending people use their own GitHub auth tokens, rather than the provided GH one |
@orta is it safe? I have seen several article that do not recommend using personal tokens. Because they can be leaked. |
It's always been the recommended way to do it: https://danger.systems/js/guides/getting_started.html#creating-a-bot-account-for-danger-to-use It's safe for OSS, as long as you don't give the bot access to anything |
Well, there should be a mode where danger-js reports things using the Github Actions reporting system (sending some logs to the output of the script, with some special formats in it to achieve some actions like adding annotations, if needed) instead of adding a comment. Then, it would be usable in actions directly. Github Actions already have a reporting UI integrated in Gtihub. |
I had a play around, and it's relatively straightforward to understand whether Danger is running on a PR originated from a fork. I came up with this simple const headRepoName = danger.github.pr.head.repo.full_name
const baseRepoName = danger.github.pr.base.repo.full_name
if headRepoName != baseRepoName {
// This is shown inline in the output
console.log("\033[1;31mRunning from a forked repo. Danger won't be able to post comments on the main repo unless GitHub Actions are enabled on the fork, too.\033[0m")
// This is shown inline in the output and also integrates with the GitHub
// Action reporting UI and produces a warning
console.log("##[warning]Running from a forked repo. Danger won't be able to post comments on the main repo unless GitHub Actions are enabled on the fork, too.\033[0m")
} I guess logic could be added somewhere that, if it detects that the CI source is GitHub Actions and the PR is from a fork posts this comments. Or, even better, when the API call to post a comment gets a 403 with message "Resource not accessible by integration", checks the above conditions and post the warning. In this case, one could use |
Also worth trying to use the Danger checks implementation on a forked PR as @stof mentioned |
As suggested here danger/danger-js#918 (comment)
I tried it with this commit, same result, see the build here.
I also tried to use a custom A PR from the base repo works as expected. A PR from a forked repo fails saying there's no token. My guess would be that despite running on the base repo, it accesses the environment of the forked repo, where no token exists. |
I also found this as an issue on my org. I have also tried to use It seems that there are #1125 and #1126. It's days ago, but hopefully it got implemented soon. |
Danger doesn't appear to work with PRs from forks: danger/danger-js#918 Will need to research this some more. This reverts commit 53d0cc6.
Anyone have any success working around this issue? I'd love to use Danger for the Zed repo, but not being able to run on forks is a huge blocker for us. |
Big fan of Zed, but the issue here is "a CI security thing" (in this case how GitHub CI works) which we can't really address inside danger - for DefinitelyTyped, a very non-trivial repo we do use the advice from this comment: #918 (comment) |
Gotcha. I suppose if it has been fine to have a token out in the open like that for DefinitelyTyped it's probably fine for us (even if it does make my skin crawl 😅). I guess another option would be to run a small proxying service that forwards to the GitHub API and attaches the access token at that point? |
I ended up pursuing this route. Here's the solution I came up with: https://github.com/maxdeviant/danger-proxy If anyone else finds the proxy useful, do let me know 😄 |
This PR updates Danger to proxy its requests to GitHub through a proxy service. ## Motivation Currently Danger is not able to run on PRs opened from forks of Zed. This is due to GitHub Actions' security policies. Forks are not able to see any of the repository secrets, and the built-in `secrets.GITHUB_TOKEN` has its permissions [restricted](https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token) to only reads when running on forks. I asked around on the Danger repo, and some big projects (DefinitelyTyped) are working around this by using a publicly-listed (although slightly obfuscated) token: danger/danger-js#918 (comment). While this approach is _probably_ okay given the limited scope and permissions of the GitHub token, I would still prefer a solution that avoids disclosing the token at all. ## Explanation I ended up writing a small proxy service, [Danger Proxy](https://github.com/maxdeviant/danger-proxy), that can be used to provide Danger with the ability to make authenticated GitHub requests, but without disclosing the token. From the README: > Danger Proxy will: > > - Proxy all requests to `/github/*` to the GitHub API. The provided GitHub API token will be used for authentication. > - Restrict requests to the list of repositories specified in the `ALLOWED_REPOS` environment variable. > - Restrict requests to the subset of the GitHub API that Danger requires. I have an instance of this service deployed to [danger-proxy.fly.dev](https://danger-proxy.fly.dev/). Release Notes: - N/A
Cool answer 👍🏻 |
Problem
Danger crashes when I accept a PR from the fork: https://github.com/sobolevn/itmo-2019/pull/18/checks?check_run_id=222332195
But, works well when I create PRs inside the repo: https://github.com/sobolevn/itmo-2019/pulls?q=is%3Apr+is%3Aclosed (just an example).
My configuration:
Link: https://github.com/sobolevn/itmo-2019/blob/master/.github/workflows/review.yml
Output
Image (in case output it is easier to read):
Possible reason
I guess that this is possibly related with how
GITHUB_TOKEN
works for forked repos: https://help.github.com/en/articles/virtual-environments-for-github-actions#github_token-secretAnd I have no ideas, how to fix it. [Official docs] do not say much about this problem: https://danger.systems/js/guides/getting_started.html#setting-up-danger-to-run-on-your-ci Moreover, I cannot find any other real-world usage of
danger-js
as an action. So, I cannot verify that it also happens to other users as well.Any ideas where to look?
The text was updated successfully, but these errors were encountered: