Fix command-injection RCE in the CI integration

[YuliiaKovalova]

Summary

execCommand in src/ciIntegration.ts passed shell: true on Windows. Node does not escape the args array when a shell is used — it concatenates the arguments into a command line handed to cmd.exe (this is exactly what DEP0190 warns about). An argument of main&echo X causes echo X to run as a separate command.

Every value that reached those arguments was attacker-influenced:

Value	Source	Sink
owner / repo	parseGitHubRemote(git remote get-url origin)	gh run list --repo …, gh run download --name …
org / project	parseAzdoRemote	az … --org https://dev.azure.com/… --project …
branch	git rev-parse --abbrev-ref HEAD	gh run list --branch …
artifactName	returned by the CI server	gh run download --name …

Git permits &, |, ;, $, (, ) in branch names, and parseGitHubRemote matched github.com as a substring, so all of these parsed successfully:

• https://evil-github.com/attacker/payload
• https://notgithub.com/attacker/payload
• https://github.com/owner/repo&calc

Net effect: cloning a repo with a crafted remote URL or branch name and running "Download Binlog from CI" was remote code execution on Windows.

The fix — three independent layers

1. No shell — src/commandResolver.ts (new)

shell: true is gone. resolveExecutable searches PATH once per command name, honouring PATHEXT, restricted to .exe/.com/.cmd/.bat, never searching the working directory (binary planting), and caches successful resolutions only. Real executables are launched directly with shell: false.

.cmd/.bat shims (only az in practice) go through cmd.exe /d /s /c.

⚠️ Note on the shim path. spawn('cmd.exe', ['/d','/s','/c', shim, ...args]) is not sufficient. libuv's quote_cmd_arg only quotes an argument that contains a space, tab or quote, so main&calc would arrive on the command line unquoted and cmd.exe would still split it. Node's CVE-2024-27980 mitigation only triggers when the .bat/.cmd is the file argument, not when it appears inside args.

So the command line is built and quoted here and passed with windowsVerbatimArguments: true — mirroring what Node does for shell: true, but with our own quoting, and still shell: false. Characters that cannot be neutralised inside cmd.exe double quotes (", %, !, CR, LF, NUL) are rejected, not escaped.

2. Allow-list validation at the boundary — src/ciSafety.ts

isValid* / assertValid* for:

• owner / repo / org / project — ^[A-Za-z0-9._-]+$, no leading - (option injection), not ./..
• branch / ref — shell metacharacters, whitespace and control characters rejected
• artifact name — no path separators or metacharacters
• run / build id — digits only

Applied in every function that builds gh/az arguments, and to manually entered org/project and owner/repo. Rejection surfaces a user-facing error naming the offending value.

3. Real URL parsing — src/gitRemoteParse.ts (new)

The scp form git@host:owner/repo is handled explicitly; everything else goes through new URL().

• GitHub requires hostname === 'github.com' || hostname.endsWith('.github.com')
• Azure DevOps requires dev.azure.com, ssh.dev.azure.com or *.visualstudio.com
• Path segments are percent-decoded before validation, so %26 / %2F payloads are caught
• github.com.attacker.io is deliberately classified as unrelated, not as Enterprise

GitHub Enterprise remains unsupported (unchanged behaviour), but the failure is now explicit — GitHub Enterprise remotes are not supported. — instead of a generic "no CI provider found".

Two adjacent sites with the same defect

Both are reachable from the same attack chain, since a CI artifact may contain a file named evil&calc.binlog:

• src/buildCheck.ts detectSdkVersion used shell: true.
• src/extension.ts binlog.openInStructuredLogViewer passed an attacker-influenced file path into cmd /c start unquoted.

Tests

• src/test/gitRemoteParse.test.ts (new) — spoofed hosts (evil-github.com, notgithub.com, github.com.attacker.io, userinfo tricks), metacharacter payloads (repo&calc, repo;whoami, repo|x, $(calc), backticks, percent-encoded), legitimate remotes (My.Repo, .git suffix, trailing slash, scp form, ssh://, deep Actions URLs), Enterprise classification, AzDO host anchoring.
• src/test/ciSafety.test.ts — the validators; quoteForCmdExe; buildLaunchSpec (direct exec vs. cmd shim vs. unresolvable, and that the working directory is never searched); plus a regression test that walks src/ and fails if any file ever hands child_process a truthy shell option.

npm test → 316 passing.

Behaviour changes worth reviewing

• Azure DevOps project names containing spaces are now rejected (they are legal in AzDO and appear as %20).
• CI artifact names containing parentheses (e.g. Build (Release)) are now rejected.

Both follow the strict allow-list; loosen REPO_IDENTIFIER_RE / ARTIFACT_NAME_RE in src/ciSafety.ts if they turn out to matter in practice.