Open rossjrw opened 2 years ago
PR Preview Action v1.1.1
:---:
:rocket: Deployed preview to https://rossjrw.github.io/pr-preview-action/pr-preview/pr-6/
on branch gh-pages
at 2022-04-14 18:22 UTC
@rossjrw hey, just found this action a perfect-match to our needs if pr previews can be enabled from forks. so, wondering if this pr can be merged soon?
@ryoqun Not soon, no - before I'm willing to release this kind of change I need to be sure that I can appropriately document the horrible security risks of allowing unmoderated forks from deploying code to your repository, and how to work around it.
I took a glance at your profile and it looks like you're using a different repo entirely to contain preview deployments, which seems very sensible. If you need this functionality right now, you could look at extracting the key steps from https://github.com/rossjrw/pr-preview-action/blob/022361539c71c58a7141d4fe8c3e0e4a1c34f9c5/action.yml and putting them in a GitHub Actions workflow directly in your main repo, with the necessary parameters. Alternatively, it looks like https://github.com/ACCESS-NRI/pr-preview-action/issues/1 had some luck using the commits from this as-yet-unmerged PR to parametrise the PR values.
As always my top recommendation is to use a tool that's actually designed to do this reliably and is maintained by people who are paid to maintain it well, e.g. Vercel, Netlify.
@rossjrw I'm wondering if you considered using the pull_request_target
event. The issue I've encountered is with a PR from a fork trying to push to the gh-pages branch on the origin repo. The pull_request_target
enables this, but in order to deal with the security issues you point out, it's necessary to isolate the workflows and even set it so that previews from forks require manual approval.
Is there a reason why forked repos PR's couldn't be hosted on the forks gh-pages
? That would bypass the security risk, no?
Is there a reason why forked repos PR's couldn't be hosted on the forks
gh-pages
? That would bypass the security risk, no?
@zerothi That's an interesting proposal. I'd like to see a proof-of-concept to explore in more detail, but just off the top of my head / with cursory research:
I definitely appreciate and applaud the caution with regard to security here; however aren't these risks precisely what https://docs.github.com/en/actions/managing-workflow-runs/reviewing-deployments is designed to solve? Personally I'd be fine with allowing workflows from forkers if I have to approve them before they can run.
Maybe I'm missing something but it doesn't feel to me like you need to carry the whole responsibility of making this safe. Ultimately if you issue clear caveats and document the safety considerations, then it's up to users of the action to act responsibly based on that. And if their repo breaks, they get to keep the pieces ;-)
I also see that #38 was merged, so the pattern of deploying to a separate repo is an option, with the security isolation that provides. Nice!
@aspiers You're right - but it is my responsibility to avoid implying that things are guaranteed to be safe and secure. I don't know of any instances of malicious actors abusing this preview action and I'm very keen for it to stay that way.
This action is really very basic and most of the upcoming work is about documenting how to use it. You can do almost all of the things you describe right now using different configuration options around the action - e.g. the pull_request_target
event gives a forked workflow permission to access the base repo, you can add deployment reviews if you like, as you pointed out you can even deploy to a different repo. There are a few things you can't do because of hardcoded PR information but that's what this PR #6 is for, to make the action even more generic.
I'm sure these options are valid approaches for an experienced user, but I feel that packaging an Action carries the implication that anyone can drop it into a project and use it regardless of skill level. I don't want to present these nuanced use cases to a user who might not understand the caveats that come with them, so in the meantime, stating that forks aren't supported is easiest on my end. But I certainly can't stop you from finding clever workarounds that work for your use case!
Thanks a lot for the reply an extra info - I think we're probably in violent agreement :-) I wrote "Ultimately if you issue clear caveats and document the safety considerations" so I certainly wasn't advocating for you implying any misplaced guarantees around safety, or facilitating a more risky approach without the appropriate warnings. Perhaps where we may slightly differ in opinion is that I generally prefer for advanced users not to be limited through aiming to protect users who completely disregard their responsibility for their own security. But I can see arguments for both sides :-)
Why doesn't PR Preview Action support preview from forks?
If you came here from the README this is probably the question you're asking.
The answer is that there are big security implications of allowing people you don't know and shouldn't trust (forkers) from being able to add code directly to your repository without any oversight. Even if your previews are isolated to a single branch (e.g.
gh-pages
), letting anyone insert whatever code they like there is not a good idea.Consider what a malicious PR with write access to your repository is able to do:
If you need previews and you need them from forks, don't use a tool that deploys forked code right back into your repository. Use a tool that hosts previews elsewhere, like Vercel/Netlify or some equivalent.
I'd like to add support for forks to this action eventually, but it'll require careful consideration of all the ramifications - along with thoroughly documenting how to navigate them.
Further reading: https://nathandavison.com/blog/github-actions-and-the-threat-of-malicious-pull-requests
This PR will:
workflow_run
, which fixes #3pull_request
event, values that the action needs that previously came from the pull request directly will be parametrised