The run() helper used execSync with template-string interpolation, and
the inline `.replace(/"/g, '\\"')` was a porous escape — it only handled
double quotes, leaving backticks, $(...), \, and ; literal. Combined
with `prNumber as number` (a TS cast, not a runtime check), an MCP
client could send:
prNumber: "1; touch /tmp/pwned; echo "
title: 'foo`whoami`bar'
labels: ['bug; rm -rf $HOME']
…and run() would interpolate them straight into the shell.
Fixes:
runArgv(file, args[], cwd?)
New helper using execFileSync with shell:false. argv elements aren't
interpreted by /bin/sh, so metas in user-supplied strings stay
literal. Original run() preserved for the static-string callers
(gh issue list, git rev-list — they need pipes).
toPositiveInt(value)
Coerces + validates prNumber/issueNumber/workflowId as a positive
integer (≤ 2^31). "1; rm -rf /" → null. Number([1,2]) → null.
sanitizeLabels(value)
Each label must be string ∈ /^[A-Za-z0-9][A-Za-z0-9 _\-./]{0,63}$/.
Backticks, $, ;, |, leading separator, length > 64, non-array, or
non-string element → null.
Risky callers converted (10 sites):
pr create : title/body/branches via argv
pr review : prNumber via toPositiveInt + argv
pr merge : same
pr close : same
issue create: title/body via argv, labels via sanitizeLabels
issue update: issueNumber via toPositiveInt, title via argv,
labels re-sanitized
issue close: issueNumber via toPositiveInt + argv
workflow status / trigger / cancel: workflowId/ref via argv
(already validated by validateIdentifier — argv form
kept for consistency / defense in depth)
Tests:
__tests__/github-tools-injection.test.ts — 15 cases pinning the
toPositiveInt + sanitizeLabels contracts. Includes payloads
'1; rm -rf /', '1`whoami`', '1$(whoami)', 'bug`whoami`' so a future
refactor that loosens the validators trips a CI failure.
Verified: tsc --noEmit clean; full audit-fix test surface 46/46 passing
(4 files: validate-input-agent-spawn, validate-input-env, fs-secure,
github-tools-injection).
Reuven