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

Make Danger work on PRs from forks #100

Open
Samasaur1 opened this issue Jun 4, 2022 · 2 comments
Open

Make Danger work on PRs from forks #100

Samasaur1 opened this issue Jun 4, 2022 · 2 comments

Comments

@Samasaur1
Copy link
Owner

I'm also still seeing

Response: {
[38](https://github.com/Samasaur1/DiceKit/runs/6732668468?check_suite_focus=true#step:4:39)
  "message": "Resource not accessible by integration",
[39](https://github.com/Samasaur1/DiceKit/runs/6732668468?check_suite_focus=true#step:4:40)
  "documentation_url": "https://docs.github.com/rest/reference/users#get-the-authenticated-user"
[40](https://github.com/Samasaur1/DiceKit/runs/6732668468?check_suite_focus=true#step:4:41)
}

in the Danger logs, which I believe is due to Danger running in the PR and therefore the token not having write access. For now, it's able to comment because the PR isn't coming from a fork, but I probably want to change this to use the pull_request_target event (and use the Dangerfile from my repo, so that the token can't be exploited that way).. See here: https://github.saobby.my.eu.orgmunity/t/github-actions-are-severely-limited-on-prs/18179/20

Originally posted by @Samasaur1 in #90 (comment)

@Samasaur1
Copy link
Owner Author

This isn't an immediate problem, as it doesn't stop me from working on this, but it would be nice to get it set up before anyone else tries to contribute, so that I don't have to scramble to set it up

@Samasaur1
Copy link
Owner Author

The idea as I currently have it is that we use the pull_request_target event (which gives the built-in GitHub token write access, since it runs on the base, not the merge). This runs my workflow file, which is why it is safe for it to have write access. In the workflow file, I can have it checkout the merge (incorporating their code, while still running my safe workflow), and then run Danger.

The only security implication that I'm aware of when doing this is that Danger runs the local Dangerfile, which would be a problem if I allowed it to run modified Dangerfiles while authenticated. I believe I can circumvent this by telling Danger to use the Dangerfile from the master branch, which means that the only limitation here would be that the Dangerfile cannot be modified in PRs from forks and then run on that PR (which isn't really a limitation, but the desired behavior)

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

1 participant