Enable deploy previews via cloudflare - #40
Open
magicmark wants to merge 14 commits into
Open
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>
magicmark
added a commit
to magicmark/gaps
that referenced
this pull request
May 26, 2026
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- 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
reviewed
May 27, 2026
benjie
left a comment
Member
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!
| 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" |
Member
There was a problem hiding this comment.
Maybe include the commit has in the description?
Member
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?
4 tasks
- Add explicit ref: main to checkout step (prevents config poisoning) - Include commit hash in wrangler deploy message for traceability - Extract hide-old-comments logic to .github/scripts/hide-old-deploy-comments.mjs - Add .github/scripts to sparse-checkout so the extracted script is available - Add security comments documenting zip-slip safety and static-only wrangler model
- Use github.paginate to find deploy-preview comments beyond the first 30 results - Filter by github-actions[bot] author to avoid hiding human comments that quote the deploy-preview marker Co-Authored-By: Claude <noreply@anthropic.com>
The assets-only config already resolves to wrangler's internal no-op template, so bundling is unnecessary. Skipping it removes any ambiguity about whether esbuild could be influenced by _site contents. Co-Authored-By: Claude <noreply@anthropic.com>
Addresses the review comments from benjie on #40: 1. **`ref: main`** on checkout step — makes it explicit we're pulling trusted config from main, not from the PR branch 2. **Commit hash in deploy message** — adds `(${{ github.event.workflow_run.head_sha }})` for traceability 3. **Extract inline JS** — moves the hide-old-comments logic to `.github/scripts/hide-old-deploy-comments.mjs` (also adds `.github/scripts` to sparse-checkout so it's available at runtime) 4. **Security: outside bounds zip extraction** — documents that `actions/download-artifact` handles extraction safely 5. **Security: static-only wrangler** — documents that wrangler is configured for static asset serving only All changes verified end-to-end on [magicmark/gaps-website-test-1](https://github.com/magicmark/gaps-website-test-1) with both fork and non-fork PRs deploying successfully to Cloudflare Workers.
- actions/checkout v6 → v7 - actions/setup-node v6 → v7 - actions/github-script v7 → v9 Co-Authored-By: Claude <noreply@anthropic.com>
magicmark
added a commit
to magicmark/gaps-website-test-1
that referenced
this pull request
Jul 14, 2026
Update deploy preview workflows to match graphql/gaps#40
robo-magicmark
pushed a commit
to robo-magicmark/gaps-website-test-1
that referenced
this pull request
Jul 14, 2026
- Upgrade actions/checkout to v7, actions/setup-node to v7, actions/github-script to v9 - Add --no-bundle flag to wrangler deploy command - Use paginate() and user.login filter in hide-old-deploy-comments script - Remove pages.yml (replaced by Cloudflare deploy) Co-Authored-By: Claude <noreply@anthropic.com>
The wrangler-action splits the `command` input by newlines and runs each as a separate wrangler invocation, so the multiline >- scalar doesn't work as expected here. Put the command on a single line. Co-Authored-By: Claude <noreply@anthropic.com>
martinbonnin
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@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: