Skip to content

Commit 083b107

Browse files
committed
Harden fork-safe preview workflows
1 parent a31f6e9 commit 083b107

3 files changed

Lines changed: 55 additions & 19 deletions

File tree

.github/workflows/preview-build.yml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,19 @@ jobs:
6767
build:
6868
runs-on: ubuntu-latest
6969
steps:
70-
- uses: actions/checkout@v4
70+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
7171
with:
7272
# Untrusted code: never persist credentials.
7373
persist-credentials: false
7474
fetch-depth: ${{ inputs.fetch-depth }}
7575

7676
- if: ${{ inputs.node-version != '' }}
77-
uses: actions/setup-node@v4
77+
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
7878
with:
7979
node-version: ${{ inputs.node-version }}
8080

8181
- if: ${{ inputs.php-version != '' }}
82-
uses: shivammathur/setup-php@v2
82+
uses: shivammathur/setup-php@728c6c6b8cf02c2e48117716a91ee48313958a19 # v2
8383
with:
8484
php-version: ${{ inputs.php-version }}
8585

@@ -124,6 +124,7 @@ jobs:
124124
fi
125125
cp "$src" "$bundle/zips/$name.zip"
126126
echo "Staged: $name.zip ($(wc -c <"$bundle/zips/$name.zip") bytes)"
127+
unzip -l "$bundle/zips/$name.zip"
127128
done <<< "$ARTIFACTS_INPUT"
128129
129130
if [ -z "$(ls -A "$bundle/zips")" ]; then
@@ -144,7 +145,7 @@ jobs:
144145
fi
145146
146147
- name: Upload artifact bundle
147-
uses: actions/upload-artifact@v4
148+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
148149
with:
149150
# Single bundle keeps upload-artifact's static step count happy and
150151
# the publish workflow only needs to find one artifact, not N.

.github/workflows/preview-publish.yml

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,12 @@ permissions:
6161
jobs:
6262
publish:
6363
runs-on: ubuntu-latest
64-
if: github.event_name == 'workflow_run'
6564
steps:
6665
- name: Guard against misuse
67-
# Defense in depth: the job-level `if` already gates on workflow_run,
68-
# but we also want a loud failure (not a silent skip) if a caller
69-
# wires this up via pull_request_target or schedule.
66+
# This workflow is privileged. It may only run after an unprivileged
67+
# pull_request build completed. Do not convert this to a job-level
68+
# `if`: misconfigured callers should see a loud failure, not a
69+
# silently skipped publish job.
7070
shell: bash
7171
env:
7272
EVENT_NAME: ${{ github.event_name }}
@@ -96,6 +96,7 @@ jobs:
9696
BLUEPRINT: ${{ inputs.blueprint }}
9797
KIND: ${{ inputs.kind }}
9898
FROM_ARTIFACT: ${{ inputs.blueprint-from-artifact }}
99+
ARTIFACTS_TO_KEEP: ${{ inputs.artifacts-to-keep }}
99100
run: |
100101
set -euo pipefail
101102
set_count=0
@@ -110,11 +111,17 @@ jobs:
110111
echo "::error::kind must be 'plugin' or 'theme', got: $KIND" >&2
111112
exit 1
112113
fi
114+
if [ "$ARTIFACTS_TO_KEEP" != "keep-all" ]; then
115+
if ! [[ "$ARTIFACTS_TO_KEEP" =~ ^[0-9]+$ ]] || [ "$ARTIFACTS_TO_KEEP" -lt 1 ]; then
116+
echo "::error::artifacts-to-keep must be a positive integer or 'keep-all', got: $ARTIFACTS_TO_KEEP" >&2
117+
exit 1
118+
fi
119+
fi
113120
114121
- name: Identify artifact bundle
115122
id: meta
116123
if: env.SKIP != '1'
117-
uses: actions/github-script@v7
124+
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7
118125
env:
119126
SOURCE_RUN_ID: ${{ github.event.workflow_run.id }}
120127
with:
@@ -303,7 +310,7 @@ jobs:
303310
304311
- name: Post Playground preview button
305312
if: env.SKIP != '1'
306-
uses: WordPress/action-wp-playground-pr-preview@v2
313+
uses: WordPress/action-wp-playground-pr-preview@c8607529dac8d2bf9a1e8493865fc97cd1c3c87b # v2
307314
with:
308315
mode: ${{ inputs.mode }}
309316
blueprint: ${{ steps.blueprint.outputs.blueprint }}

README.md

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,14 +385,35 @@ Two kinds of public URL work:
385385

386386
The action handles (1) directly. The two reusable workflows handle (2) end-to-end: they run your build, upload the result to a `ci-artifacts` prerelease, and call the action with a Blueprint pointing at the resulting URL.
387387

388-
### Two-workflow split (build path only)
389-
390-
GitHub doesn't let one workflow simultaneously (a) run untrusted code from a fork PR and (b) write to releases or PR comments. The build path therefore splits:
391-
392-
- **Build workflow** runs on `pull_request`, `permissions: contents: read`. Untrusted PR code executes here. The reusable workflow declares `contents: read` as a *ceiling*, so callers can't accidentally elevate it.
393-
- **Publish workflow** runs on `workflow_run`, `permissions: contents: write` + `pull-requests: write`. Never checks out PR code. GitHub reads its YAML from the *default branch* (security guarantee), so PRs can't tamper with what runs here.
394-
395-
The publish workflow has a runtime guard that **fails loudly** if invoked from any trigger other than `workflow_run`. Misconfigured callers (e.g. someone reaches for `pull_request_target`) get a red ❌ instead of a silent compromise.
388+
### Fork safety model (build path only)
389+
390+
GitHub doesn't let one workflow simultaneously (a) run untrusted code from a
391+
fork PR and (b) write to releases or PR comments. The build path therefore
392+
splits the work at the artifact boundary:
393+
394+
- **Build workflow** runs on `pull_request`, `permissions: contents: read`.
395+
It checks out the PR head, runs your `build-command`, validates that the
396+
expected zip(s) exist, logs `unzip -l` for inspection, and uploads a single
397+
bundle artifact. It has no secrets and does not persist checkout credentials.
398+
- **Publish workflow** runs on `workflow_run`, `permissions: contents: write`
399+
+ `pull-requests: write`. It never checks out PR code. GitHub reads this
400+
workflow from the default branch, so a fork PR cannot change the privileged
401+
publish logic in the same PR.
402+
- **Artifact bundle** is the only handoff from untrusted to trusted code. The
403+
publish workflow treats it as opaque bytes: it uploads the zip(s), substitutes
404+
their URLs into a Blueprint, and lets Playground run them later inside its
405+
browser sandbox.
406+
407+
The publish workflow has a runtime guard that **fails loudly** if invoked from
408+
any trigger other than `workflow_run`, or if the source run was not a successful
409+
`pull_request` run. Misconfigured callers (for example someone reaches for
410+
`pull_request_target`) get a red failure instead of a silent skip.
411+
412+
Because the publish workflow is privileged, its third-party action references
413+
are pinned to commit SHAs. This avoids granting write permissions to a moved
414+
major-version tag. The internal button action is also called through an
415+
immutable v2 commit; v3 adds the reusable workflow layer around the same button
416+
action behavior.
396417

397418
### Trigger model and security
398419

@@ -407,6 +428,11 @@ The publish workflow has a runtime guard that **fails loudly** if invoked from a
407428

408429
Translation: the action does not require any trust in PR code. The zip is treated as opaque bytes everywhere except inside the Playground iframe, where the WebAssembly sandbox is the actual mitigation.
409430

431+
For public repositories, release assets are public. A fork PR can therefore
432+
cause its built zip to be hosted on the repository's `ci-artifacts` prerelease
433+
until cleanup removes it. That is the tradeoff that makes one-click browser
434+
previews possible for fork contributors.
435+
410436
`{{ARTIFACT_URL:<name>}}` substitution uses `JSON.stringify(url).slice(1, -1)`, so any character that could break JSON parsing is escaped. The `"{{...}}"` template convention is non-breaking and produces valid JSON for any URL.
411437

412438
The `ci-artifacts` release is created as a **`--prerelease`**, not a draft. Prerelease assets are publicly downloadable on first run; draft assets require auth and Playground can't read them. The action handles this for you on first use; existing draft releases need a one-time conversion (see [Migrating](#migrating-from-older-usage)).
@@ -471,7 +497,7 @@ Runs in the privileged `workflow_run` context, exposes the artifact bundle's zip
471497
| `blueprint` | one of three‡ | — | Blueprint JSON template. Use `{{ARTIFACT_URL:<name>}}` placeholders inside double quotes. |
472498
| `kind` | one of three‡ | — | `plugin` or `theme`. Shortcut: requires exactly one zip in the bundle, generates an `installPlugin`/`installTheme` step with `activate: true`. |
473499
| `blueprint-from-artifact` | one of three‡ | `false` | When `true`, read `blueprint.json` from the artifact bundle (requires `blueprint-from-build:` on the build side). |
474-
| `artifacts-to-keep` | no | `2` | Distinct PR commits worth of zips to keep on the release. Older zips for the same PR get pruned. Set to `keep-all` to disable cleanup. |
500+
| `artifacts-to-keep` | no | `2` | Positive integer number of distinct PR commits worth of zips to keep on the release. Older zips for the same PR get pruned. Set to `keep-all` to disable cleanup. |
475501
| `release-tag` | no | `ci-artifacts` | Tag used to host artifacts publicly. Auto-created as a prerelease on first use. |
476502
| `mode` | no | `append-to-description` | `append-to-description` or `comment`. |
477503

@@ -515,13 +541,15 @@ All variables except `PLAYGROUND_BUTTON` are HTML-escaped before substitution.
515541
- **Two workflow files when there's a build step.** GitHub's permission model around fork PRs makes this unavoidable. The reusable workflows minimise but don't eliminate the boilerplate.
516542
- **Permissions ceiling is rigid.** Reusable workflows declare a max permission set; callers can match but not extend. Almost certainly a feature, but worth naming if you need the publish workflow to also push tags.
517543
- **Build and publish workflows must be pinned to compatible versions.** The artifact-naming format is the implicit interface between them. Use the same `@v3` (or branch ref) in both.
544+
- **Fork PR build output becomes public.** The publish workflow never trusts the zip, but it does upload it to a public release URL so Playground can fetch it. Keep `artifacts-to-keep` low unless you deliberately want longer retention.
518545
- **`{{ARTIFACT_URL:<name>}}` substitution is the only template feature.** No conditionals, no loops, no other placeholders. For per-PR variable shapes, write the blueprint at build time and use `blueprint-from-artifact: true`.
519546
- **One zip per `artifacts` entry.** Multi-file install bundles still need a hand-rolled flow.
520547
- **Plugin zips must extract to a slug-named folder.** When you `zip -r my-plugin.zip .` from inside the plugin dir, the zip contents are at the root, and Playground will install them with no slug folder. Wrap with a directory: `mkdir stage/my-plugin && rsync -a ./ stage/my-plugin/ && (cd stage && zip -r ../my-plugin.zip my-plugin)`.
521548
- **Vite/webpack assets need stable filenames.** Playground enqueues from a fixed path, so disable hashing on the entry file (`rollupOptions.output.entryFileNames: 'admin.js'`) or read the build manifest in PHP.
522549
- **Hidden directories are skipped by `actions/upload-artifact@v4`.** If you stage your bundle in a `.foo/` directory it'll silently produce an empty artifact. Use a non-hidden name.
523550
- **`fetch-depth: 0` is required for diffs.** The default checkout is shallow (depth 1). Diffs against the PR base ref need full history, otherwise `git diff` fails with "no merge base."
524551
- **The `ci-artifacts` release is shared across all PRs.** Each PR's zips are unique (`pr-<N>-<SHA>-<name>.zip`); cleanup keeps the N most recent commit-sets per PR.
552+
- **`artifacts-to-keep` must be a positive integer or `keep-all`.** `0`, negative numbers, and arbitrary strings fail before any release assets are uploaded.
525553
- **`workflow_run`-triggered workflows always read their YAML from the default branch.** Workflow changes on a PR branch don't take effect until merged. Test publish-side changes on a scratch repo first.
526554
- **Private repos won't work.** Playground runs in the user's browser and needs unauthenticated download URLs. `git:directory` and release assets in private repos both require auth Playground doesn't have. Make the repo public or self-host the zip.
527555

0 commit comments

Comments
 (0)