deckhearth/.github/workflows/visual-diff.yml
Randall Stillwell b6f8688df8 fix(ci): route github.event.pull_request.body through env: to avoid shell injection
Pre-existing latent bug surfaced by PR #17's CI run. The Decide step's
inline `${{ github.event.pull_request.body }}` interpolation pastes
arbitrary PR-body text directly into a bash script. When the body
contains shell metacharacters (parens, pipes, backticks, redirections,
etc.) the resulting script either errors out at YAML-load time OR —
worse — executes attacker-controlled shell.

This bit PR #17 with a real syntax error ("unexpected token `('")
because the PR body contains parenthesized phrases like
"(was: 10-minute timeout)". Every Decide-step run in this repo has
been one badly-formatted PR body away from breaking the gate.

Fix: forward `github.event.pull_request.body` and
`github.event.pull_request.head.repo.fork` through the step's `env:`
block as `PR_BODY` and `PR_IS_FORK`, then quote them in shell
(`"$PR_BODY"`, `"$PR_IS_FORK"`). The env-var path leaves the values
as plain strings rather than syntactically embedded code, which is the
standard GitHub Actions hardening pattern (see GitHub's "Security
hardening for GitHub Actions" → "Using a third-party action").

Same change in both workflows; ~9 LOC each.

This fix is technically beyond Brief 1's scope (which targeted only
Vercel-bypass plumbing) but is added in this convoy because the bug
actively blocks Brief 1's success criterion from being validated on
PR #17. Documented in the convoy file's "Anything flagged but not
acted on" follow-up pass.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-24 15:31:16 -05:00

108 lines
3.7 KiB
YAML

name: Visual diff
# Same as preview-smoke — waits for Vercel's Preview deployment, then captures
# Playwright screenshots against it. UI-paths-only trigger to keep cost down.
# Paths are tcg-vault-specific (pages router, JS).
on:
pull_request:
branches: [main]
paths:
- 'pages/**'
- 'components/**'
- 'styles/**'
- 'tailwind.config.js'
- 'postcss.config.js'
concurrency:
group: visual-diff-${{ github.event.pull_request.number }}
cancel-in-progress: true
# `deployments: read` + `statuses: read` are required by wait-for-vercel-preview
# (see preview-smoke.yml for context). `pull-requests: write` is required by the
# final github-script step that posts the "Visual Diff" comment back to the PR;
# without it the API returns 403 even though the screenshots upload fine.
permissions:
contents: read
deployments: read
pull-requests: write
statuses: read
jobs:
gate:
name: Should run?
runs-on: ubuntu-latest
outputs:
should_run: ${{ steps.check.outputs.should_run }}
steps:
- id: check
env:
# See preview-smoke.yml for the rationale: ${{ github.event.* }}
# inlined into shell is a syntax-error + injection vector.
PR_BODY: ${{ github.event.pull_request.body }}
PR_IS_FORK: ${{ github.event.pull_request.head.repo.fork }}
run: |
if [[ "$PR_IS_FORK" == "true" ]]; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped on fork PR (bypass secret unavailable to forks)"
elif echo "$PR_BODY" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
echo "should_run=false" >> $GITHUB_OUTPUT
echo "::notice::Visual diff skipped via pipeline directive"
else
echo "should_run=true" >> $GITHUB_OUTPUT
fi
visual:
name: Screenshot diff
needs: gate
if: needs.gate.outputs.should_run == 'true'
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/checkout@v4
- name: Wait for Vercel Preview deployment
id: vercel
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
with:
token: ${{ secrets.GITHUB_TOKEN }}
max_timeout: 120
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}&x-vercel-set-bypass-cookie=true
- uses: actions/setup-node@v4
with:
node-version: '20'
cache: npm
- run: npm ci
- run: npx playwright install --with-deps chromium
- name: Capture screenshots (PR)
run: npx playwright test --project=visual --update-snapshots=none
env:
BASE_URL: ${{ steps.vercel.outputs.url }}
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
continue-on-error: true
- name: Upload screenshots + diffs
if: always()
uses: actions/upload-artifact@v4
with:
name: visual-diff
path: |
tests/visual/__screenshots__/
test-results/
retention-days: 7
- name: Comment on PR with diff link
if: always()
uses: actions/github-script@v7
with:
script: |
const run = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: `## Visual Diff\n\nScreenshots and diffs uploaded as artifacts: [view run](${run})\n\nIf intentional changes: update snapshots locally with \`npx playwright test --project=visual --update-snapshots\` and commit.`
});