Enable deploy previews via cloudflare#40
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
First half of a secure fork-safe deploy preview setup. This workflow runs on all PRs (including forks), builds the site with no access to secrets, and uploads the artifact. A separate workflow_run-triggered workflow will handle the privileged Cloudflare deploy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Second half of the secure fork-safe deploy preview setup. Downloads the build artifact from the unprivileged build workflow and deploys to Cloudflare Pages. Has access to secrets but never executes fork code. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ssions - Fix critical bug: pull_requests[] is empty for fork PRs in workflow_run events. Save PR number as artifact metadata instead. - Add actions: read permission so download-artifact can fetch cross-workflow - Update action versions to latest: checkout@v6, upload-artifact@v7, download-artifact@v8, wrangler-action@v4 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
magicmark
changed the title
- Pin all actions to commit SHAs (supply-chain protection) - Replace artifact-based PR number with gh pr list --head lookup (prevents PR number spoofing from untrusted artifacts) - Remove "Save PR metadata" step from build (no longer needed) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
benjie
left a comment
benjie
left a comment
There was a problem hiding this comment.
I don’t know much about the security model of workflow completed, I’ll need to read up. Seems a promising direction!
| steps: | ||
| - name: Checkout wrangler config | ||
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 | ||
| with: |
There was a problem hiding this comment.
Can we add a branch name to make it explicit it’s not pulling from the PR branch? Lowers cognitive overhead
| with: | ||
| apiToken: ${{ secrets.CLOUDFLARE_API_TOKEN }} | ||
| accountId: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }} | ||
| command: versions upload --preview-alias "pr-${{ steps.pr.outputs.number }}" --message "PR #${{ steps.pr.outputs.number }} preview" |
There was a problem hiding this comment.
Maybe include the commit has in the description?
There was a problem hiding this comment.
Can you confirm that this is a simple upload where under no circumstances is any code ran, config files read, etc from within _site?
| - name: Hide old deploy preview comments | ||
| uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7 | ||
| with: | ||
| script: | |
There was a problem hiding this comment.
I really dislike code in yaml, is it possible to put this code in a file somewhere?
| name: preview-build | ||
| run-id: ${{ github.event.workflow_run.id }} | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| path: _site |
There was a problem hiding this comment.
AFAIK, this downloads a tarball or similar and extracts it to that folder. There have been vulnerabilities in the past where a maliciously structured tarball has resulted in the extractor writing files outside of the target directory, are we certain we’re not at risk of this?
2 participants
@benjie for your consideration...
Turns out Cloudflare Pages / Workers cannot be run auto-run on fork-PRs.
(Vercel gives us this out the box. This PR is basically "we have vercel deploy previews at home".)
Flow + Security
This is set up as two actions:
on: pull_request) - to build the app + upload artifacton: workflow_run) - fires whenever the pull_request job completes, and deploys the site to a subdomainPrior art: https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#using-data-from-the-triggering-workflow
(1)
on: pull_requestactions run untrusted code. This file can be modified by PRs, do anything - yikes. Luckily, this only gets a RO Github API token (which the public already implicitly has). This cannot be modified.(2)
on: workflow_runactions execute the workflow definition frommainbranch (not the PR branch). It does have access to the Cloudflare Token, but we're only running trusted code here. Sadly it doesn't look like cloudflare lets you more scope tokens to specific projects, it's org-wide.Another layer of protection we already have set up in https://github.com/graphql/graphql.github.io is that workflows should not run automatically on fork PRs. A maintainer has to click a button to see a deploy preview.
This would be nice, but conflicts with "editors can merge their own edits to GAPs" philosophy. I think we have to disable this if we want to encourage easy and frictionless edits to GAPs. More specifically, there is an option to only make someone click a button for first-time contributors, which could be a nice middleground and prevents drive-bys.
Turning off Cloudflare's Github integration
Note: I would recommend removing the git integration in the cloudflare dashboard to avoid double-commenting deploy preview urls on non-fork prs.
Should we ship this?
🤷 idk all that yaml ain't pretty, but it does provide some real value.
If you like this approach, I can package this up as reusable action and we can reuse it across our various websites (
graphql/website-deploy-previewor whatever)On the other hand, if you want to stick with vercel, or abandon non-fork prs altogether that's fine too.
Validation / Example PRs:
Tested this all against a copy of this repo here: https://github.com/magicmark/gaps-website-test-1 so you can see what it looks like: