1 Commits

Include renames

Author	SHA1	Message	Date
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