From 029f061eba0a1504e5505fbf82a2679fee2dbe7c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Wed, 6 May 2026 17:48:56 -0700 Subject: [PATCH] fix(ci): prevent shell injection via CI event data in gitCommitInfoPlugin The gitCommitInfoPlugin passes prBaseHash from the GitHub Actions event payload into shell commands via spawnAsync with shell: true. The prBaseHash originates from json.pull_request.base.sha in the event file, which is attacker-influenced in malicious PRs. Replace the shell-based command execution with argument arrays and remove shell: true. Also split the chained git log && git rev-parse into two separate calls. --- .../src/plugins/gitCommitInfoPlugin.ts | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/playwright/src/plugins/gitCommitInfoPlugin.ts b/packages/playwright/src/plugins/gitCommitInfoPlugin.ts index 6f71deecd5374..2b4096c2feff5 100644 --- a/packages/playwright/src/plugins/gitCommitInfoPlugin.ts +++ b/packages/playwright/src/plugins/gitCommitInfoPlugin.ts @@ -124,12 +124,12 @@ async function gitCommitInfo(gitDir: string): Promise '%cn', // committer name '%ce', // committer email '%ct', // committer date, UNIX timestamp - '', // branch ]; - const output = await runGit(`git log -1 --pretty=format:"${tokens.join(separator)}" && git rev-parse --abbrev-ref HEAD`, gitDir); - if (!output) + const logOutput = await runGit(['log', '-1', `--pretty=format:${tokens.join(separator)}`], gitDir); + if (!logOutput) return undefined; - const [hash, shortHash, subject, body, authorName, authorEmail, authorTime, committerName, committerEmail, committerTime, branch] = output.split(separator); + const branchOutput = await runGit(['rev-parse', '--abbrev-ref', 'HEAD'], gitDir); + const [hash, shortHash, subject, body, authorName, authorEmail, authorTime, committerName, committerEmail, committerTime] = logOutput.split(separator); return { shortHash, @@ -146,7 +146,7 @@ async function gitCommitInfo(gitDir: string): Promise email: committerEmail, time: +committerTime * 1000, }, - branch: branch.trim(), + branch: branchOutput?.trim() ?? '', }; } @@ -154,8 +154,8 @@ async function gitDiff(gitDir: string, ci?: CIInfo): Promise const diffLimit = 100_000; if (ci?.prBaseHash) { // https://git-scm.com/docs/git-fetch - await runGit(`git fetch origin ${ci.prBaseHash} --depth=1 --no-auto-maintenance --no-auto-gc --no-tags --no-recurse-submodules`, gitDir); - const diff = await runGit(`git diff ${ci.prBaseHash} HEAD`, gitDir); + await runGit(['fetch', 'origin', ci.prBaseHash, '--depth=1', '--no-auto-maintenance', '--no-auto-gc', '--no-tags', '--no-recurse-submodules'], gitDir); + const diff = await runGit(['diff', ci.prBaseHash, 'HEAD'], gitDir); if (diff) return diff.substring(0, diffLimit); } @@ -165,7 +165,7 @@ async function gitDiff(gitDir: string, ci?: CIInfo): Promise return; // Check dirty state first. - const uncommitted = await runGit('git diff', gitDir); + const uncommitted = await runGit(['diff'], gitDir); if (uncommitted === undefined) { // Failed to run git diff. return; @@ -174,20 +174,20 @@ async function gitDiff(gitDir: string, ci?: CIInfo): Promise return uncommitted.substring(0, diffLimit); // Assume non-shallow checkout on local. - const diff = await runGit('git diff HEAD~1', gitDir); + const diff = await runGit(['diff', 'HEAD~1'], gitDir); return diff?.substring(0, diffLimit); } -async function runGit(command: string, cwd: string): Promise { - debug(`running "${command}"`); +async function runGit(args: string[], cwd: string): Promise { + debug(`running "git ${args.join(' ')}"`); const start = monotonicTime(); const result = await spawnAsync( - command, - [], - { stdio: 'pipe', cwd, timeout: GIT_OPERATIONS_TIMEOUT_MS, shell: true } + 'git', + args, + { stdio: 'pipe', cwd, timeout: GIT_OPERATIONS_TIMEOUT_MS } ); if (monotonicTime() - start > GIT_OPERATIONS_TIMEOUT_MS) { - print(`timeout of ${GIT_OPERATIONS_TIMEOUT_MS}ms exceeded while running "${command}"`); + print(`timeout of ${GIT_OPERATIONS_TIMEOUT_MS}ms exceeded while running "git ${args.join(' ')}"`); return; } if (result.code)