GoogleCloudPlatform / GoogleCloudPlatform/samples-style-guide
Site: Deploy Previews from forks
- Dominant language
- HTML
- Stars
- 13
- Forks
- 17
- PR merge metrics
- No merged PRs in 30d
Description
## Background
This repository's source code is used with the hugo static site generator to build and deploy a website to GitHub Pages. We have a preview system that writes a complete site build to a subdirectory namespaced to the Pull Request to facilitate reviews. This preview system makes reviews much easier.
## Problem
The preview system for the style guide is currently locked to changes pushed directly to the repository, no changes from forks:
https://github.com/GoogleCloudPlatform/samples-style-guide/blob/0bb1c7e9d98bd5e5de36374c012fc97b5418421a/.github/workflows/github_pages_preview.yml#L72
However, there can be significant contributions from folks working in a fork, such as #101.
The reason this is locked down is that providing write access to a fork is a [pretty bad security risk](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/).
## Proposal
Learning from [On `pull_request_target`, labels, and working with insecurity.](https://glasnt.com/blog/pull_request_target_labels/), @glasnt proposes using labels to toggle permission to run a workflow to test and label. I propose an even more restricted approach to allow the existing PR preview system to preview fork changes.
1. If the changes touches the .github directory, cancel. We only expect repository collaborators to make changes to the preview tooling, and this reduces the risk of reviewer error.
2. Only deploy from fork if a `preview: generate` label is applied by a maintainer
3. When the preview workflow is run, automatically remove the label (optionally, add status labels like `preview: failed` or `preview: ready`)
As a further race condition measure:
4. If we can't check out the branch at the commit associated with the label, can we compare the latest commit on the branch to the commit or parent commit of the checkout?
## Initial Security Assessment
* The sample style guide is meant to publish specialized documentation and is not directly sourced in code.
* Data/credential exfiltration would continue to be a risk similar to running tests for any fork
Contributor guide
Assessment
This issue has not been assessed yet.