Files
orca/src/shared/child-process/windows-command-line.ts
T
OrcaWinandOrca Worker c252d855ac fix(windows): resolve npm/pnpm .cmd shims past cmd.exe (#17869)
* fix(windows): resolve npm/pnpm .cmd shims past cmd.exe

A `.cmd` target forces every spawn through `cmd.exe /c` with each argument
caret-escaped, and Microsoft Defender for Endpoint scores a long `cmd.exe /c`
line carrying caret-escaped natural language as obfuscation. `codex.cmd` is
named in the spawn cluster of the MDE incident this addresses.

npm's `cmd-shim` and pnpm's `@zkochan/cmd-shim` generate files whose whole body
is "find node, run this script". Read one, and the spawn can go straight to
`node.exe <script> <args>` — no cmd.exe, no caret escaping. Anything the parser
does not recognise exactly, or whose target cannot be confirmed on disk, keeps
the existing cmd.exe path.

Incidentally fixes a real bug: cmd ends its command at a raw CR/LF whatever the
quote state, so a multi-line agent prompt through a `.cmd` shim had to be
rejected. Resolved shims have no such limit.

* fix(windows): refuse drive-relative shim paths and run the win32 tests in CI

Two blocking findings from review.

A drive-relative path defeated the absolute-path guard:
`win32.isAbsolute('D:evil.js')` is false, but `win32.resolve` reads the drive
letter and lands on `D:\evil.js`, outside the shim directory. cmd would have
built `C:\shim\D:evil.js` and failed; we would have executed the wrong file.
Adding `:` to the unsafe-character set closes it, and the alternate-data-stream
spelling `a.js:zone` with it. It costs no coverage: 84 of the 91 real shims on
this box still resolve, the same seven fall back.

Neither `windows-cmd-shim-resolution.test.ts` nor its `.win32` sibling was in
the Windows package job's file list, so the whole filesystem/resolution half and
the real-spawn equivalence suite ran nowhere. Both are now in
`WINDOWS_PACKAGE_TESTS` and in the pr.yml step.

Also from review: clear `windowsVerbatimArguments` explicitly on the resolved
branch rather than inheriting it, since there is no caller-built command line
there; document the kill switch and the PTY/hook-wrapper scope limits in
docs/reference; and cover drive-relative, BOM, line-ending, casing and `%*`
tampering in the platform-independent half of the tests.

* docs(windows): justify the shim-path colon guard from the filesystem rule

The guard was argued empirically ("none of the 91 shims on this box has one"),
which invites a future reader to relax it for a shim we have not seen. Windows
reserves `:` within a path segment, so a relative path cannot carry one at all:
the only spellings that can are drive-qualified, an alternate data stream, or a
`\?\` device path, and the last is already refused as absolute. That makes a
false refusal impossible rather than unobserved.

* refactor(child-process): move resolveSpawn into its own module

The merge with main pushed run-process.ts one line past the 300-line cap:
both sides grew it. The spawn-argv decision is already a pure, separately
tested unit, so it moves out rather than the cap moving up. run-process.ts
re-exports it, so no caller changes.

* perf(child-process): cache the shim interpreter lookup

The parse cache spared the shim read but not the PATH walk, so a second
resolution of the same .cmd did 0 reads and one statSync per PATH entry --
30 on a 30-entry PATH, synchronous on resolveSpawn, where one dead network
mount blocks the calling thread on every spawn.

Keyed by shim directory AND PATH, since the shim's own rule is
%~dp0\node.exe first then PATH, and a PATH edit between spawns must miss.
Corrects the stat comment, which accounted only for the shim itself.

* fix(child-process): revalidate a cached shim interpreter before using it

The node cache was held for process life and never rechecked, so a cached
node.exe that was later uninstalled -- or dropped from PATH by a version
manager -- was still handed to resolveSpawn, failing the spawn with ENOENT.
An uncached process in the same state returns null and falls back to
cmd.exe successfully, so the cache was strictly worse than no cache.

One statSync on a non-null hit, not one per PATH entry, so the walk this
cache exists to skip is still skipped. The stale-null direction stays
uncorrected on purpose: it only keeps the working cmd.exe fallback. Both
directions are now stated in the comment, along with the known miss for
callers that vary PATH per spawn.

* fix(child-process): honour PATHEXT when resolving the shim interpreter

The doc claimed a node.com/.bat/.cmd on PATH returned null and fell back to
cmd.exe. The scan actually skipped those entries and kept looking for a
node.exe, so PATH=C:\A;C:\B with C:\A\node.com and C:\B\node.exe resolved to
B's node.exe while the shim runs A's node.com -- a different binary, chosen
silently, on the one axis this module must not get wrong.

The scan now follows cmd's rule: first PATH directory holding any PATHEXT
spelling wins, PATHEXT order decides within it, and only an .exe winner is
returned. Anything else gives up and keeps the cmd.exe path, which restores
the strict-subset-of-cmd property everywhere except the documented cwd case.

PATHEXT is read from the child's env and joined into the cache key, since it
now changes the answer. Costs one stat per PATHEXT entry per node-less
directory, paid once per process behind the cache.

---------

Co-authored-by: Orca Worker <orca-worker@localhost>
2026-09-05 21:33:16 -07:00

125 lines
5.5 KiB
TypeScript

/**
* Windows command-line construction.
*
* Two parsers read the line Orca hands to CreateProcess, and they disagree:
*
* - The target program's own startup code (`CommandLineToArgvW`, which the CRT and
* Node both use) understands `\"` as a literal quote and `\\` as a literal
* backslash.
* - `cmd.exe` — unavoidable for a `.cmd`/`.bat` target — does NOT. It counts `"`
* naively to track quote state, and inside that state it still expands `%VAR%`.
*
* So a `\"` written for the first parser silently flips cmd's quote parity, and
* every later `&` `|` `<` `>` on the line stops being data and becomes an
* operator. Measured on Windows 11 with a real `.cmd` shim: `["a b", 'c"d',
* "e%F%g", "h&i", "j^k"]` came back as `["a b", 'c"d', "e^%F^%g", "h"]` — the
* `&` truncated the argument AND ran the remainder as a command.
*
* The encoding below satisfies both parsers at once and round-trips all of the
* adversarial cases; see windows-command-line.test.ts for the corpus.
*/
/**
* Quote one argument for `CommandLineToArgvW`, writing an embedded `"` as `""`
* rather than `\"`.
*
* Why `""` and not `\"`: both spellings decode to a literal quote, but only `""`
* leaves cmd's naive quote count even. Since the same encoding has to survive a
* cmd hop for `.cmd` targets, we pay the (identical-length) `""` spelling
* everywhere rather than keep two dialects.
*
* Backslashes are still doubled before a quote and at the end of a quoted run,
* because that part `CommandLineToArgvW` does interpret.
*/
function quoteWindows(value: string, escapePercent: boolean): string {
let quoted = '"'
let backslashes = 0
for (const char of value) {
if (char === '\\') {
backslashes += 1
continue
}
if (char === '"') {
// The backslash run is literal, so double it; the quote itself becomes `""`.
quoted += `${'\\'.repeat(backslashes * 2)}""`
backslashes = 0
continue
}
if (escapePercent && char === '%') {
// `%VAR%` expands even inside a quoted token, so the pair has to be
// broken: close the quote, escape the percent, reopen. The backslash run
// must be DOUBLED first -- a quote straight after a single backslash is
// an escaped quote to CommandLineToArgvW, which silently corrupts every
// `C:\Users\%USERNAME%\...` path.
quoted += `${'\\'.repeat(backslashes * 2)}"^%"`
backslashes = 0
continue
}
quoted += `${'\\'.repeat(backslashes)}${char}`
backslashes = 0
}
// Trailing backslashes precede the closing quote, so they need doubling too —
// otherwise `C:\dir\` ends the argument with an escaped quote and swallows it.
return `${quoted}${'\\'.repeat(backslashes * 2)}"`
}
/** Quote one argument for `CommandLineToArgvW`. */
export function quoteWindowsArgument(value: string): string {
return quoteWindows(value, false)
}
/**
* Quote one argument that has to survive a cmd hop as well.
*
* Separate from `quoteWindowsArgument` rather than a boolean parameter: the
* two differ only in whether `%` is neutralised, and a flag there is easy to
* pass by accident — `values.map(quoteWindowsArgument)` hands `map`'s index in
* as the flag, which is exactly how this was first written.
*/
export function quoteWindowsCmdArgument(value: string): string {
return quoteWindows(value, true)
}
/**
* Build the argv Node should spawn to run `program` with `args` through
* `cmd.exe`, for targets cmd must interpret (`.cmd`, `.bat`).
*
* Callers must pass the result with `windowsVerbatimArguments: true` so Node
* hands the line through untouched, and must NOT set `shell: true` — that
* concatenates arguments without escaping (Node warns DEP0190) and silently
* disables `windowsHide`.
*
* `/d` skips AutoRun commands from the registry, which would otherwise run
* arbitrary user configuration before our command. `/v:off` pins delayed
* expansion off so a `!` in an argument stays literal even where the Command
* Processor registry default turns it on. `/s` makes cmd strip exactly the
* outer quote pair and treat the rest verbatim.
*/
export function buildWindowsCmdShimCommandLine(program: string, args: readonly string[]): string {
// Why reject rather than encode: cmd's line parser ends the command at a raw
// CR or LF whatever the quote state, so there is no escape for it -- quoting
// does not survive a line break. Encoding one anyway truncates the argument
// and can leave the remainder to be interpreted as a further command. Agent
// prompts are the motivating input here and can contain newlines, so this
// has to fail loudly rather than silently mangle. Recognised npm/pnpm shims
// no longer reach this line at all — windows-cmd-shim-resolution.ts spawns
// their target directly, where a newline is just another character.
for (const value of [program, ...args]) {
if (/[\r\n]/.test(value)) {
throw new Error('cmd.exe cannot receive an argument containing a line break')
}
}
// The program path needs the same treatment as the arguments: it is just as
// likely to contain `%USERNAME%`, and cmd expands it just the same.
const inner = [program, ...args].map(quoteWindowsCmdArgument).join(' ')
return `/d /v:off /s /c "${inner}"`
}
const CMD_INTERPRETED_EXTENSIONS = ['.cmd', '.bat']
/** Whether `program` is a target Windows can only start through `cmd.exe`. */
export function isCmdInterpretedProgram(program: string): boolean {
const lower = program.toLowerCase()
return CMD_INTERPRETED_EXTENSIONS.some((extension) => lower.endsWith(extension))
}