Resolve permissions error when preview is initiated from fork

[rossjrw]

https://github.com/scpwiki/interwiki/runs/5834681840?check_suite_focus=true

This workflow run failed because it did not have permission to push to the upstream repository.

I believe this happened because the workflow was executed from the fork. The pull_request event is executed in the context of the merge commit, whereas the pull_request_target event is executed in the base repository. For internal PRs, these are identical; for forks, they are not. In particular this means that the action will not have permission to push to the repository when using the pull_request event type, as its GITHUB_TOKEN lacks that permission.

Switching to the pull_request_target event might resolve this, but there are security considerations to take into account: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Changes shouldn't be needed to this action, but I will need to amend documentation to cover this use case. The README is getting pretty bloated - I should take this chance to move some of the code samples into the repository proper.

[rossjrw]

As recommended by the linked article, the flow I'm going to go with is a pull_request event that builds the deployment and saves it to an artifact, followed by a workflow_run that downloads the artifact and deploys it.

workflow_run is unique in that it executes with permissions inherited from the base repository, not the fork, so it will be able to deploy code. It's important to ensure that any code execution (i.e. the build script) is run in a non-privileged environment, in this case the pull_request event.

Downside to workflow_run is that it runs totally isolated from the calling context, and won't appear as e.g. a failing check in the PR. This is in contrast with e.g. workflow_call which is tightly integrated with the PR system and will appear as its own check; however, workflow_call does not execute with elevated permissions.

Also, the action currently strongly assumes that it's going to be run from a pull_request event, going so far as to assume github.event refers to a pull request. I need to make it also work for workflow_run, and in so doing I might as well remove its dependency on any specific kind of event.

That'll mean that I need to be able to pass it all the values that it would normally take directly from the pull request, via parameter instead. I can probably just set the defaults to the pull request values.

[electrofelix]

Would an option here be to have an example that uses pull_request_target for closed & labeled where it requires that a specific label has been applied indicating a review has taken place. The action can then remove the label at the end of the run to ensure that any updates require a fresh review. The closed event can always skip performing a build which prevents the risk of executing code under control of the fork.

I'm planning to set up a local action this way to support preview until workflow_run support is available at least, but I might still keep with the labeled approach to give things a once over before letting it publish either way.

[rossjrw]

Good idea, if perhaps a little complex to set up and then have your team remember. Depending on the scope of your project, at that point it may be a better idea to outsource that work to somewhere you know it's been done correctly e.g. Netlify, Vercel.

[Fir121]

Not knowledgeable about this workflow, but I've been reading the docs and a few issues and I was wondering to get past this security problem is there no way to prompt the repository owners to allow deployment of a preview for the created PR. Ie: Require the owner to click a button or put a comment and only preview the PR after that?

[rossjrw]

@Fir121 Yes, the button you describe is possible - have a look at Deployment Reviews. I learned about this just the other day from @aspiers in #6 (comment)

There's any number of things you could put between a forked PR and that PR's preview running your repo with elevated permissions, and you could do any of those things right now, but I'm still wary at this time. That being said, if you happen to take the time to put together something functional that uses this or something like this, I'd be happy to have a look and see if I feel it's worth recommended in this documentation.