From bf87dd415691f92b7eadf060a6ab8a0fe0518e26 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 18 Aug 2026 14:12:44 -0400 Subject: [PATCH] enhance(#3617): one canonical Windows binary resolver in the platform seam (epic #3411 Phase 1) (#3621) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(#3411): one canonical Windows binary resolver in the platform seam CONTEXT.md declares src/shell-command-projection.cts the single OS-facing seam, but Windows binary resolution had grown four divergent implementations outside it. #3445 folded two of them together — inside gsd-core/bin/gsd-tools.cjs, not the seam — so the declaration stayed untrue and execTool still had no handling at all. Lift the resolver into the seam as resolveExecutableBinary, and export the half that actually executes as projectSpawnInvocation: CreateProcess cannot run a .cmd/.bat, so the cmd.exe mediation is inseparable from the lookup and splitting them is how the copies accumulated. cmd.exe is invoked with an explicit argv array, never shell:true — CVE-2024-27980's vector and Node 26's DEP0190. execTool now resolves on win32. POSIX is a strict no-op by construction, which matters: execTool rates CRITICAL blast radius (167 symbols, 53 files). gsd-tools.cjs deletes its private scan and its private mediation and delegates. Two semantics grown beyond #3445's resolver, both additive: a name already carrying a PATHEXT-listed extension is tried as-is before the append loop, and a suffix outside PATHEXT is not treated as an extension. Refs #3411 * fix(#3411): keep mediating a declared .cmd that PATH resolution misses Standards review caught a narrowing against the code this replaces. gsd-tools.cjs computed `target = resolveSpawnBinary(binary) || binary` and keyed the shim test on `target`, so a declared .cmd mediated whether or not PATH resolution found it. That is load-bearing: resolveExecutableBinary scans PATH only, while `cmd.exe /c` also finds a batch file in the current directory. Mediation now keys on the target — resolved path, else declared name. The ENOENT contract still holds for BARE unresolved names, which is the case it was written for. P9/P10 pin both halves. Spec review found E1/E2/E3/E5 promised by 50-test-matrix.md but never written; added. E3 is the integration proof that the CVE-relevant mediation fires through execTool, not only through projectSpawnInvocation in isolation. Also adds the CONTEXT.md glossary entry for the seam's new resolution ownership (a PR gate) and the changeset fragment. Refs #3411 * fix(#3617): pass mediated cmd.exe arguments verbatim so metacharacters cannot inject The isolated security pass found the mediation shape carried an argument-injection surface. libuv's quote_cmd_arg force-quotes an argv element only when it contains a space, tab, or quote — never for a cmd metacharacter — and cmd.exe re-parses everything after /c. So an arg of a&calc arrived unquoted and cmd ran calc. Node's own CVE-2024-27980 escaping cannot help: it fires only when the spawned FILE is the .bat/.cmd, and here the file is cmd.exe. Caret-escaping is not a fix. It is correct only when libuv does not quote, and libuv quotes whenever the arg also contains a space — no per-arg transform is right in both cases. So build the command line and pass it through verbatim, the shape Rust's std adopted for the sibling CVE-2024-24576: one outer quote pair that cmd /c strips, every token inside force-quoted, embedded quotes doubled. An argument containing CR or LF is refused rather than mediated — a newline cannot be represented in a Windows command line, so mediating would silently truncate. Failing visibly is correct. Known limit, documented at the seam: %VAR% still expands inside a /c string and has no escape outside a batch file. That is information disclosure, not arbitrary execution, and is the same limit Rust's std documents. This was byte-for-byte the shape #3445 shipped, so the fix closes it for the reviewer-lane spawn path too, not only for execTool's newly reachable route. Refs #3411 * docs(#3617): document the subprocess-execution security posture Adds Layer 4 to the security model: why GSD never uses shell:true for binary invocation (CVE-2024-27980, Node 26 DEP0190), why resolution is explicit and never tries the bare name on Windows (the npm extensionless-shim trap behind #3275), and why .cmd/.bat mediation builds a verbatim force-quoted command line rather than relying on default escaping — Node's own CVE protection cannot fire once the started program is cmd.exe. The residual %VAR% expansion limit is stated plainly under Trade-offs rather than left implicit: it is information disclosure, not arbitrary execution, and callers passing untrusted text to a Windows .cmd should not assume the value arrives byte-identical. Docs-only; no code change. Refs #3411 * chore(#3617): backfill changeset pr number 3621 * fix(#3617): read PATH, PATHEXT and ComSpec case-insensitively The Windows CI lane on #3621 failed E5, and the root cause was a defect in the implementation, not the assertion. Windows names the variable Path, not PATH. process.env is a case-insensitive proxy, so process.env.PATH works — but execTool builds { ...process.env, ...opts.env } whenever a caller supplies opts.env, and spreading discards the proxy while keeping the OS's actual casing. The exact-case env['PATH'] lookup then returned undefined, the PATH scan saw zero segments, resolution returned null, and the change degraded to precisely the spawn ENOENT it exists to fix. ComSpec and PATHEXT had the same exposure. #3445's tests never caught it because they pass uppercase keys explicitly, and neither did the Linux remote runner — this is a defect only the Windows lane could see. _envGet resolves a variable by exact match first (so a canonical caller pays no scan) and falls back to a case-insensitive sweep. R23 and P16 pin it and were proven RED by execution: with the fix stashed and build:lib re-run, R23 returned null and P16 returned the cmd.exe default. R24 was rewritten because the first version was vacuous — it staged foo.CMD, so the default PATHEXT already contained .CMD and it passed against the broken code for the wrong reason. It now stages foo.XYZ, an extension absent from the default, and carries a negative control asserting that dropping the Pathext key yields null. Re-proven RED the same way. E5's assertion was corrected alongside the fix: 'PATH' in options.env expressed the wrong contract. It now checks case-insensitively for the key. Refs #3411 * fix(#3617): execTool spawns the declared name unless mediation is required The Windows full-test lane on #3621 failed tests/graphify.test.cjs — the python3 identity check asserted 'python3' and got the absolute resolved path C:\hostedtoolcache\windows\Python\3.12.10\x64\python3.EXE instead. Those tests are correct and the change was wrong. They pin a long-standing contract — execTool spawns the program name it was given — by spying on spawnSync's first argument, and routing every win32 call through the projected invocation broke it. Resolving a .exe buys nothing. libuv's CreateProcess path already performs PATH + PATHEXT search, which is why spawning a bare 'node' has always worked on Windows. The only case the OS genuinely cannot spawn is a .cmd/.bat. So execTool now adopts the projection only when mediation actually happened — windowsVerbatimArguments is exactly that flag — and otherwise passes the declared program and args through untouched. 40-design.md already rejected gratuitous change for this reason: symmetry is not worth a behavior change to 53 files that fixes nothing. That reasoning was applied to POSIX and missed the win32 non-batch case. Rows 5 and 20 now record it, and the CONTEXT.md glossary states the caller-choice rule. deps.spawn deliberately still adopts the resolved path: its hasBinary probe answers from the same resolver, so probe and spawn must agree on the exact file (#3445). The asymmetry is now documented at both call sites rather than latent. E7 pins the restored contract and was verified by executing execTool against a monkeypatched spawnSync: python3 in, python3 spawned. Refs #3411 --------- Co-authored-by: sim --- .changeset/sturdy-voles-run.md | 5 + CONTEXT.md | 2 +- docs/explanation/security-model.md | 55 ++ gsd-core/bin/gsd-tools.cjs | 47 +- src/shell-command-projection.cts | 243 ++++++- ...shell-command-projection-dispatch.test.cjs | 672 ++++++++++++++++++ 6 files changed, 992 insertions(+), 32 deletions(-) create mode 100644 .changeset/sturdy-voles-run.md diff --git a/.changeset/sturdy-voles-run.md b/.changeset/sturdy-voles-run.md new file mode 100644 index 000000000..79be1ccc8 --- /dev/null +++ b/.changeset/sturdy-voles-run.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3621 +--- +**Windows binary resolution now has one owner** — GSD resolves a command name to the file Windows can actually start, in the single platform seam, instead of four divergent copies. Reviewer lanes, `execTool`, and the capability spawn path all share it, so a `.cmd`/`.bat` shim resolves and runs where it previously failed with `spawn ENOENT`. macOS and Linux behavior is unchanged. (#3411) diff --git a/CONTEXT.md b/CONTEXT.md index b847c8491..4b7571310 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -825,7 +825,7 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr ## Shell Command Projection Module (expanded glossary entry, 2026-05-13) -Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). +Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. WINDOWS BINARY RESOLUTION — which file a declared command name actually names, and what must be handed to `spawnSync` to start it — is owned here as of #3411 (epic #3411 Phase 1), which found the seam declaration untrue for this axis: four divergent implementations had grown outside it (`execNpm`'s `shell:true`, `execTool`'s absence of any handling, a private PATH+PATHEXT scan in `gsd-core/bin/gsd-tools.cjs`, and a fourth candidate-extension array in `fallow-runner.cts`), and #3275's fix to one provably never reached the others. `resolveExecutableBinary(name, {platform, env})` returns the RESOLVED PATH (the prior `hasBinary` computed it and threw it away): on win32 it tries PATHEXT entries ONLY and never the bare name, because npm global installs drop an extensionless POSIX `sh` shim beside `foo.CMD` and a bare-name-first scan resolves to it, leaving the ENOENT unchanged (#3275); a name already carrying a PATHEXT-listed extension is tried as-is BEFORE the append loop, and a suffix outside PATHEXT is not an extension. On POSIX it answers existence only, and `execTool` does not consult it there at all — the bare name goes to `spawnSync` unchanged and Node's own PATH search does the work, keeping macOS/Linux byte-identical for a symbol whose blast radius is CRITICAL (167 affected symbols, 53 files). `projectSpawnInvocation(command, args, {platform, env})` is the inseparable second half: `CreateProcess` cannot execute a `.cmd`/`.bat` at all, so resolution alone does not fix Windows, and exporting only the resolver would leave every caller to re-derive the mediation — which is how the four copies accumulated. It mediates through `ComSpec` with an EXPLICIT argv array (`/d /s /c`), never `shell: true`: that is CVE-2024-27980's argument-injection vector and Node 26's DEP0190. Mediation keys on the target — the resolved path, or the declared name when resolution found nothing — so a `.cmd` resident in the current directory (which PATH-only resolution misses but `cmd.exe /c` still finds) keeps working, while a BARE unresolved name is passed through untouched so the spawn fails with ENOENT rather than cmd.exe's exit 9009, preserving `_spawnResult`'s `': not found'` contract. `gsd-core/bin/gsd-tools.cjs`'s `resolveSpawnBinary` is the `bin/` entry point onto this and holds no copy of the logic. CALLERS CHOOSE HOW MUCH OF THE PROJECTION TO ADOPT, and the asymmetry is deliberate: `windowsVerbatimArguments` marks the cases where mediation was REQUIRED (the caller must take `command` and `args` together, or the batch file cannot start at all), whereas a merely-RESOLVED path is an offer a caller may decline. `execTool` declines it and keeps spawning the DECLARED name — libuv's `CreateProcess` path already performs PATH+PATHEXT search, so resolving a `.exe` there buys nothing while changing what 167 dependents observe being spawned; `tests/graphify.test.cjs` pins that contract by spying on `spawnSync`'s first argument, and the Windows CI lane caught the violation when `execTool` briefly adopted the resolved path (`python3` arriving as `C:\…\python3.EXE`). `deps.spawn` accepts it, because that lane's `hasBinary` probe answers from the same resolver and probe and spawn must agree on the exact file (#3445). Env lookups go through a case-insensitive read: Windows names the variable `Path`, `process.env` is a case-insensitive proxy that hides this, and `execTool`'s `{...process.env, ...opts.env}` spread produces a PLAIN object that keeps the OS casing and loses the proxy — an exact-case `env['PATH']` there returns undefined and the scan silently sees nothing. Known gap at Phase 1: `src/fallow-runner.cts` still carries its own resolver (Phase 2), and no lint ratchet yet rejects a bare-binary spawn outside this seam (Phase 3). `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). Invariants: - Result shape: all exec* functions return `{ exitCode, stdout, stderr }`; never throw on non-zero exit code. diff --git a/docs/explanation/security-model.md b/docs/explanation/security-model.md index f9c6dbb54..ab5523b74 100644 --- a/docs/explanation/security-model.md +++ b/docs/explanation/security-model.md @@ -255,6 +255,51 @@ hide malicious content in diffs. --- +## Layer 4 — Subprocess execution + +GSD starts external programs constantly: git, npm, reviewer CLIs declared by +capabilities, and whatever a gate predicate names. Every one of those is a +place where an argument could become a command. One module owns the whole +question — `src/shell-command-projection.cts`, the single platform seam. + +**No `shell: true` for binary invocation.** Passing `shell: true` on Windows is +the mechanism behind CVE-2024-27980: the shell re-parses the argument list, so +a value containing `&` or `|` stops being data and becomes a second command. +Node 26 additionally deprecates `shell: true` alongside an argument array +(DEP0190), because arguments are concatenated rather than escaped. GSD resolves +binaries explicitly instead. + +**Explicit resolution, not shell lookup.** `resolveExecutableBinary` scans +`PATH` and, on Windows, the `PATHEXT` extensions, and returns the resolved +path. It never tries the bare name on Windows: npm global installs drop an +extensionless POSIX `sh` shim beside `foo.CMD`, and resolving to that shim is +how the reviewer lanes failed with `spawn ENOENT` (#3275). On macOS and Linux +the bare name goes to `spawnSync` unchanged, so the operating system's own +lookup keeps doing the work. + +**Mediating `.cmd` and `.bat` safely.** Windows `CreateProcess` cannot execute a +batch file at all, so one must be run through `cmd.exe`. That is where the +injection risk actually lives, and it is not solved by resolution alone. +`projectSpawnInvocation` builds the command line itself and passes it through +verbatim: one outer quote pair that `cmd /c` strips, every token inside +force-quoted, embedded quotes doubled. Force-quoting is the point — an unquoted +`a&calc` is split by `cmd` into two commands, while a quoted `"a&calc"` is one +literal argument. This is the shape Rust's standard library adopted for the +sibling CVE-2024-24576. + +Relying on the default argument escaping would not be enough. Node's own +CVE-2024-27980 protection fires only when the program being started is itself +the `.bat` or `.cmd`; once the program is `cmd.exe`, that check no longer +applies, and the underlying quoting only quotes arguments containing spaces, +tabs, or quotes — never one containing a bare `&`. + +An argument containing a carriage return or newline is refused rather than +mediated. A newline cannot be represented in a Windows command line, so +mediating it would silently truncate the argument; failing visibly is the +safer outcome. + +--- + ## Trade-offs and limits The security model described here meaningfully reduces the attack surface for @@ -292,6 +337,16 @@ research agents — but novel jailbreaks and low-signal injections may still pas undetected. Defence in depth means each layer makes the attack harder, not that any single layer makes it impossible. +**What subprocess execution does not eliminate:** `cmd.exe` expands `%VAR%` +inside a `/c` string, and there is no escape for `%` outside a batch file. An +argument containing `%FOO%` is therefore substituted with the environment +value before the target program sees it. That is information disclosure, not +arbitrary execution — the force-quoting still prevents an argument from +becoming a second command — and it is the same residual limit Rust's standard +library documents for its own batch-file handling. Callers that pass untrusted +text as an argument to a Windows `.cmd` or `.bat` should not assume the value +arrives byte-identical. + **Reporting vulnerabilities.** Report via private GitHub security advisory at `https://github.com/open-gsd/gsd-core/security/advisories/new`. Do not open public issues. See [SECURITY.md](../../SECURITY.md) for the response timeline diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index 311cfa444..4b79ac099 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1415,11 +1415,15 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // PATH search already worked there, and the #3275 acceptance contract // holds macOS/Linux behavior unchanged. A name that resolves to nothing // falls back to the declared name so the ENOENT still surfaces (#3086). - const isWin = process.platform === 'win32'; - const target = isWin ? (resolveSpawnBinary(binary) || binary) : binary; - const winShim = isWin && /\.(cmd|bat)$/i.test(path.basename(target)); - const spawnBinary = winShim ? (process.env.ComSpec || 'cmd.exe') : target; - const spawnArgv = winShim ? ['/d', '/s', '/c', target, ...argv] : argv; + // #3411: the resolve-then-mediate pair is one seam call now. Both halves had + // private copies here; `projectSpawnInvocation` owns them, so a fix to either + // reaches every spawn site instead of only this one. + // + // Unlike execTool, this lane adopts the RESOLVED path even for a non-batch + // binary: that is the behavior #3445 shipped and `deps.hasBinary` answers + // from the same resolver, so probe and spawn must agree on the exact file. + const { projectSpawnInvocation } = require('./lib/shell-command-projection.cjs'); + const { command: spawnBinary, args: spawnArgv, windowsVerbatimArguments } = projectSpawnInvocation(binary, argv); const r = cp.spawnSync(spawnBinary, spawnArgv, { input: opts.input, encoding: 'utf8', @@ -1431,6 +1435,7 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load // child only. Passing a fresh object leaves `process.env` untouched, so nothing leaks // into the orchestrating session or into the next lane. ...(opts.env ? { env: { ...process.env, ...opts.env } } : {}), + ...(windowsVerbatimArguments ? { windowsVerbatimArguments: true } : {}), }); return { status: r.status, @@ -3617,32 +3622,16 @@ function dispatchOverlayCapabilityCommand({ command, args, cwd, raw, error, load * * Path-like names (any '/' or '\') bypass the PATH scan: the name is already an * address, so it passes through when the file exists and is a file. + * + * #3411: the scan itself now lives in the declared platform seam + * (`src/shell-command-projection.cts` → `resolveExecutableBinary`). This function is + * the `bin/` entry point onto it and holds no copy of the logic — `CONTEXT.md` + * declares that file "All OS-facing I/O; single platform seam", and a private + * duplicate here is what made it untrue. */ function resolveSpawnBinary(name, platform = process.platform, env = process.env) { - if (!name) return null; - if (name.includes('/') || name.includes('\\')) { - try { return fs.statSync(name).isFile() ? name : null; } catch { return null; } - } - const segments = String(env.PATH || '').split(path.delimiter).filter(Boolean); - if (platform !== 'win32') { - for (const dir of segments) { - const candidate = path.join(dir, name); - try { - if (fs.statSync(candidate).isFile()) return candidate; - } catch { /* next candidate */ } - } - return null; - } - const exts = String(env.PATHEXT || '.EXE;.CMD;.BAT;.COM').split(';').filter(Boolean); - for (const dir of segments) { - for (const ext of exts) { - const candidate = path.join(dir, name + ext); - try { - if (fs.statSync(candidate).isFile()) return candidate; - } catch { /* next candidate */ } - } - } - return null; + const { resolveExecutableBinary } = require('./lib/shell-command-projection.cjs'); + return resolveExecutableBinary(name, { platform, env }); } const HOST_COMMAND_ROUTERS = { diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 5b7558b57..3dd1feb87 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -643,15 +643,254 @@ export function execNpm(args: string[], opts: { cwd?: string; timeout?: number } return _spawnResult(result, 'npm'); } +/** + * Default PATHEXT when Windows does not supply one. Matches the value + * `gsd-tools.cjs`'s `resolveSpawnBinary` shipped in #3275; kept identical so the + * delegation is a behavior-preserving move rather than a redefinition. + */ +const DEFAULT_PATHEXT = '.EXE;.CMD;.BAT;.COM'; + +/** Windows extensions that must be mediated through cmd.exe rather than spawned. */ +const CMD_MEDIATED_EXT = /\.(cmd|bat)$/i; + +function _isFile(candidate: string): boolean { + try { + return fs.statSync(candidate).isFile(); + } catch { + // Missing, unreadable (EACCES), or a broken link — all mean "not this one". + return false; + } +} + +/** + * Read an environment variable by name, case-insensitively. + * + * Windows environment variable names are case-insensitive and conventionally + * cased `Path` / `ComSpec`, and `process.env` is a case-insensitive proxy that + * hides the difference. Spreading it (`{ ...process.env, ...opts.env }`, which + * `execTool` does whenever a caller supplies `opts.env`) produces a PLAIN object + * that keeps the OS's actual casing and loses the proxy — so an exact-case + * `env['PATH']` lookup returns undefined there and the PATH scan silently sees + * nothing. Caught by the Windows CI lane on #3617; the #3445 tests never hit it + * because they pass uppercase keys explicitly. + * + * Exact match wins when present, so a caller who sets the canonical name pays + * no scan. + */ +function _envGet(env: NodeJS.ProcessEnv, name: string): string | undefined { + const exact = env[name]; + if (exact !== undefined) return exact; + const lower = name.toLowerCase(); + for (const key of Object.keys(env)) { + if (key.toLowerCase() === lower) return env[key]; + } + return undefined; +} + +/** cmd.exe's own quoting rule inside a `/c` string: a literal quote is doubled. */ +function _cmdQuoteToken(token: string): string { + return `"${token.replace(/"/g, '""')}"`; +} + +/** + * Build a single verbatim command-line string for `cmd.exe /c` where every + * token is force-quoted, then wrap the whole thing in one more outer pair. + * cmd.exe strips exactly one outer quote pair when the string begins with a + * quote and contains at least two — so the outer wrap disappears and what's + * left is a sequence of individually-quoted tokens. Force-quoting is the + * point: an unquoted `a&b` is split by cmd's own metacharacter parsing, but a + * quoted `"a&b"` is one literal argument. + */ +function _buildVerbatimCmdLine(target: string, args: string[]): string { + return `"${[target, ...args].map(_cmdQuoteToken).join(' ')}"`; +} + +/** + * #3411: resolve a DECLARED command name to the file a spawn can actually start. + * The single canonical answer to "where is this binary?" for the whole tree. + * + * This is the seam `CONTEXT.md` declares as "All OS-facing I/O; single platform + * seam". Four divergent implementations of this logic existed (#3411): `execNpm`'s + * `shell:true`, `execTool`'s absence of any handling, `gsd-tools.cjs`'s private + * scan, and `fallow-runner.cts`'s own candidate array. #3275 folded two of them + * together in `bin/`; this lifts that resolver into the seam so `bin/` delegates + * instead of owning a copy. + * + * **win32** tries PATHEXT entries ONLY — never the bare name. npm global installs + * drop an EXTENSIONLESS POSIX sh shim (`...\npm\codex`) next to `codex.CMD`; a + * bare-name-first scan resolves to it, the cmd.exe mediation gate sees no `.cmd`, + * and the ENOENT returns unchanged (field-reported on Windows 11 — see #3275). + * A name that ALREADY carries a PATHEXT-listed extension is tried as-is first, so + * `foo.exe` resolves to `foo.exe` rather than being probed as `foo.exe.EXE`; a + * suffix that is not in PATHEXT (`foo.txt`) is not an extension and only feeds the + * append loop. + * + * **POSIX** answers EXISTENCE by scanning PATH for the bare name. `execTool` does + * NOT consult this on POSIX — the bare name goes to spawnSync unchanged and Node's + * own PATH search does the work, so macOS/Linux behavior is untouched (#3275 + * acceptance contract). + * + * Path-like names (any `/` or `\`) bypass the PATH scan: the name is already an + * address, so it passes through when it names an existing file. + * + * @returns the resolved path, or `null` when nothing matched. Callers fall back to + * the declared name on `null` so a genuine ENOENT still surfaces (#3086). + */ +export function resolveExecutableBinary( + name: string | null | undefined, + opts: { platform?: string; env?: NodeJS.ProcessEnv } = {}, +): string | null { + if (!name) return null; + if (name.includes('/') || name.includes('\\')) { + return _isFile(name) ? name : null; + } + const env = opts.env ?? process.env; + const platform = opts.platform ?? process.platform; + const segments = String(_envGet(env, 'PATH') || '').split(path.delimiter).filter(Boolean); + + if (platform !== 'win32') { + for (const dir of segments) { + const candidate = path.join(dir, name); + if (_isFile(candidate)) return candidate; + } + return null; + } + + const exts = String(_envGet(env, 'PATHEXT') || DEFAULT_PATHEXT).split(';').filter(Boolean); + // A name already ending in a PATHEXT-listed extension is an address, not a stem: + // probing `foo.exe` as `foo.exe.EXE` would miss the file sitting right there. + // Compared case-insensitively because PATHEXT casing is not guaranteed. + const lower = name.toLowerCase(); + const carriesKnownExt = exts.some((ext) => lower.endsWith(ext.toLowerCase())); + + for (const dir of segments) { + if (carriesKnownExt) { + const asIs = path.join(dir, name); + if (_isFile(asIs)) return asIs; + } + for (const ext of exts) { + const candidate = path.join(dir, name + ext); + if (_isFile(candidate)) return candidate; + } + } + return null; +} + +/** + * #3411: project a declared `(command, args)` into the pair `spawnSync` can + * actually execute on this platform. + * + * Resolution alone does not make Windows work: `CreateProcess` cannot execute a + * `.cmd`/`.bat` at all, so the cmd.exe mediation is inseparable from the lookup. + * Exporting only the resolver would leave every caller to re-derive that half — + * which is precisely how #3411's four copies accumulated. + * + * cmd.exe is invoked as an ordinary program with an EXPLICIT argv array, never via + * `shell: true`. `shell:true` on Windows is the mechanism behind CVE-2024-27980 + * (argument injection through `.bat`/`.cmd`), and Node 26 deprecates it with an + * args array (DEP0190) because arguments are concatenated rather than escaped. + * + * Mediation fires when the target — the resolved path, or the declared name when + * resolution found nothing — carries a `.cmd`/`.bat` extension. A BARE name that + * resolved to nothing is passed through verbatim so the spawn fails with ENOENT; + * mediating it would turn `{exitCode:127, 'foo: not found'}` into cmd.exe's exit + * 9009 and silently change the not-found contract callers depend on. + * + * POSIX is a strict no-op: the declared command is returned unchanged and the + * environment is never consulted. + * + * The mediated command line is built VERBATIM rather than left to libuv: libuv's + * `quote_cmd_arg` only force-quotes an argument that contains a space, tab, or + * quote — it does not know about cmd.exe metacharacters (`&`, `|`, `>`, `<`, + * `^`, ...) at all, so an argument like `a&calc` reaches cmd.exe unquoted and + * gets re-parsed as two commands (the CVE-2024-27980 argument-injection class). + * Node's own CVE-2024-27980 escaping does not help here because it only fires + * when the spawned FILE itself is a `.bat`/`.cmd` — in this seam the spawned + * file is `cmd.exe`, not the target. Building the line ourselves and passing + * `windowsVerbatimArguments: true` (the shape Rust's std uses for the sibling + * CVE-2024-24576) means every token is force-quoted inside one outer pair, so + * a metacharacter inside a quoted token can never split the command line. + * + * KNOWN LIMIT: `%VAR%` still expands inside a cmd `/c` string, and there is no + * escape for `%` outside a batch file — an argument containing `%FOO%` is + * substituted with the environment value regardless of quoting. That is an + * information-disclosure limit, not arbitrary execution, and it's the same + * limit Rust's std documents for its own `CommandExt::raw_arg` escape hatch. + * + * CALLER CHOICE: this function's return value carries two independently + * adoptable pieces of information, and a caller may take either, both, or + * neither. `windowsVerbatimArguments: true` marks the cases where mediation + * was REQUIRED — the caller MUST adopt `command`+`args` together, since a + * `.cmd`/`.bat` genuinely cannot be spawned any other way. A merely-resolved + * `.exe` path (no mediation flag set) is only an OFFER: a caller may decline + * it and keep spawning the declared name instead, to hold its own observable + * contract stable. `execTool` (this file, below) is exactly such a caller — + * it adopts the mediated pair when `windowsVerbatimArguments` is set, but + * otherwise passes the declared `program`/`args` through untouched. + */ +export function projectSpawnInvocation( + command: string, + args: string[] = [], + opts: { platform?: string; env?: NodeJS.ProcessEnv } = {}, +): { command: string; args: string[]; resolved: string | null; windowsVerbatimArguments?: boolean } { + const platform = opts.platform ?? process.platform; + if (platform !== 'win32') return { command, args, resolved: null }; + + const env = opts.env ?? process.env; + const resolved = resolveExecutableBinary(command, { platform, env }); + // Mediate against the resolved path when we have one, else against the declared + // name. An unresolved name is mediated ONLY when it already declares .cmd/.bat: + // PATH-only resolution misses a batch file sitting in the current directory, + // which `cmd.exe /c` still finds — the behavior gsd-tools.cjs shipped before + // this consolidation, preserved here rather than silently narrowed. + const target = resolved ?? command; + if (!CMD_MEDIATED_EXT.test(path.basename(target))) { + // A BARE name that resolved to nothing is passed through untouched so the + // spawn fails with ENOENT. Mediating it would turn {exitCode:127, + // ': not found'} into cmd.exe's exit 9009 and silently change the + // not-found contract `_spawnResult` and its 53 dependent files rely on. + return resolved ? { command: resolved, args, resolved } : { command, args, resolved: null }; + } + // A CR/LF cannot be represented inside a Windows command line at all — cmd.exe + // treats it as a line terminator, so mediating it would silently truncate the + // argument rather than pass it through. Fail visibly instead: fall back to the + // unmediated shape so the spawn either fails with ENOENT (bare unresolved name) + // or hands the raw string to CreateProcess, whichever the caller was already + // prepared to see for a non-.cmd/.bat target. + if (/[\r\n]/.test(target) || args.some((a) => /[\r\n]/.test(a))) { + return resolved ? { command: resolved, args, resolved } : { command, args, resolved: null }; + } + return { + command: String(_envGet(env, 'ComSpec') || 'cmd.exe'), + args: ['/d', '/s', '/c', _buildVerbatimCmdLine(target, args)], + resolved, + windowsVerbatimArguments: true, + }; +} + export function execTool(program: string, args: string[], opts: { cwd?: string; env?: Record; timeout?: number } = {}): SpawnResultOutput { - const result = childProcess.spawnSync(program, args, { + // #3411: Windows cannot spawn a .cmd/.bat at all — CreateProcess refuses it — + // so those are mediated through cmd.exe. Everything else keeps the DECLARED + // program name: libuv's CreateProcess path already performs PATH + PATHEXT + // search, so resolving a .exe here would buy nothing and would change what + // this seam's 167 dependents observe being spawned. `tests/graphify.test.cjs` + // pins that contract by spying on spawnSync's first argument. POSIX never + // reaches the mediation branch at all. + const spawnEnv = opts.env ? { ...process.env, ...opts.env } : undefined; + const invocation = projectSpawnInvocation(program, args, { env: spawnEnv ?? process.env }); + const mediated = invocation.windowsVerbatimArguments === true; + const result = childProcess.spawnSync(mediated ? invocation.command : program, mediated ? invocation.args : args, { cwd: opts.cwd, - env: opts.env ? { ...process.env, ...opts.env } : undefined, + env: spawnEnv, encoding: 'utf-8', stdio: 'pipe', timeout: opts.timeout ?? 30_000, windowsHide: true, + ...(mediated ? { windowsVerbatimArguments: true } : {}), }); + // Stamp the DECLARED name, never the resolved path: `_spawnResult` renders + // `${program}: not found`, and callers across 53 files match on the string they + // passed. Resolution must not leak an absolute path into that message. return _spawnResult(result, program); } diff --git a/tests/shell-command-projection-dispatch.test.cjs b/tests/shell-command-projection-dispatch.test.cjs index f5d0457af..c68a45fa9 100644 --- a/tests/shell-command-projection-dispatch.test.cjs +++ b/tests/shell-command-projection-dispatch.test.cjs @@ -10,6 +10,8 @@ const { execGit, execNpm, execTool, + resolveExecutableBinary, + projectSpawnInvocation, probeTty, normalizeContent, platformWriteSync, @@ -113,6 +115,676 @@ describe('execTool', () => { }); }); +// ─── resolveExecutableBinary (#3411) ──────────────────────────────────────── +// Platform is always INJECTED via opts.platform/opts.env — every win32 case +// runs on macOS/Linux too. See .gsd/phase/feat-3411-windows-binary-seam/50-test-matrix.md. + +describe('resolveExecutableBinary (#3411)', () => { + test('R1: is exported as a function', () => { + assert.equal(typeof resolveExecutableBinary, 'function'); + }); + + test('R2: win32 resolves the .CMD', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(resolved, path.join(dir, 'foo.CMD')); + } finally { + cleanup(dir); + } + }); + + test('R3: win32 resolves the .EXE', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.EXE'), ''); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(resolved, path.join(dir, 'foo.EXE')); + } finally { + cleanup(dir); + } + }); + + test('R4: never the extensionless shim', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'codex'), ''); + fs.writeFileSync(path.join(dir, 'codex.CMD'), ''); + const resolved = resolveExecutableBinary('codex', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(resolved, path.join(dir, 'codex.CMD')); + assert.match(path.basename(resolved), /\.(cmd|bat)$/i); + } finally { + cleanup(dir); + } + }); + + test('R5: extensionless-only → null', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'codex'), ''); + const resolved = resolveExecutableBinary('codex', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R6: honors PATHEXT order', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'gem.CMD'), ''); + fs.writeFileSync(path.join(dir, 'gem.EXE'), ''); + const cmdFirst = resolveExecutableBinary('gem', { platform: 'win32', env: { PATH: dir, PATHEXT: '.CMD;.EXE' } }); + assert.equal(cmdFirst, path.join(dir, 'gem.CMD')); + const exeFirst = resolveExecutableBinary('gem', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(exeFirst, path.join(dir, 'gem.EXE')); + } finally { + cleanup(dir); + } + }); + + test('R7: lowercase PATHEXT', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'gem.cmd'), ''); + const resolved = resolveExecutableBinary('gem', { platform: 'win32', env: { PATH: dir, PATHEXT: '.cmd' } }); + assert.equal(resolved, path.join(dir, 'gem.cmd')); + } finally { + cleanup(dir); + } + }); + + test('R8: PATHEXT default', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'gem.CMD'), ''); + const resolved = resolveExecutableBinary('gem', { platform: 'win32', env: { PATH: dir } }); + assert.equal(resolved, path.join(dir, 'gem.CMD')); + } finally { + cleanup(dir); + } + }); + + test('R9: PATHEXT-suffixed name resolves as-is', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'tool.exe'), ''); + const resolved = resolveExecutableBinary('tool.exe', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE' } }); + assert.equal(resolved, path.join(dir, 'tool.exe')); + assert.notEqual(resolved, path.join(dir, 'tool.exe.EXE')); + } finally { + cleanup(dir); + } + }); + + test('R10: non-PATHEXT suffix is not an extension', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'note.txt'), ''); + const resolved = resolveExecutableBinary('note.txt', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE;.CMD' } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R11: append loop over a dotted name', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'note.txt.EXE'), ''); + const resolved = resolveExecutableBinary('note.txt', { platform: 'win32', env: { PATH: dir, PATHEXT: '.EXE' } }); + assert.equal(resolved, path.join(dir, 'note.txt.EXE')); + } finally { + cleanup(dir); + } + }); + + test('R12: PATH order', () => { + const dir1 = createTempDir(); + const dir2 = createTempDir(); + const dir3 = createTempDir(); + try { + fs.writeFileSync(path.join(dir2, 'foo.EXE'), ''); + const PATH = [dir1, dir2, dir3].join(path.delimiter); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH, PATHEXT: '.EXE' } }); + assert.equal(resolved, path.join(dir2, 'foo.EXE')); + } finally { + cleanup(dir1); + cleanup(dir2); + cleanup(dir3); + } + }); + + test('R13: empty PATH → null', () => { + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: '' } }); + assert.equal(resolved, null); + }); + + test('R14: empty PATH segments', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.EXE'), ''); + const PATH = dir + path.delimiter + path.delimiter + dir; + assert.doesNotThrow(() => { + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH, PATHEXT: '.EXE' } }); + assert.equal(resolved, path.join(dir, 'foo.EXE')); + }); + } finally { + cleanup(dir); + } + }); + + test('R15: path-like passthrough', () => { + const dir = createTempDir(); + try { + const staged = path.join(dir, 'foo.cmd'); + fs.writeFileSync(staged, ''); + const resolved = resolveExecutableBinary(staged, { platform: 'win32', env: { PATH: dir } }); + assert.equal(resolved, staged); + } finally { + cleanup(dir); + } + }); + + test('R16: missing path-like → null', () => { + const dir = createTempDir(); + try { + const missing = path.join(dir, 'nope.cmd'); + const resolved = resolveExecutableBinary(missing, { platform: 'win32', env: { PATH: dir } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R17: directory is not a binary', () => { + const dir = createTempDir(); + try { + const resolved = resolveExecutableBinary(dir, { platform: 'win32', env: { PATH: dir } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R18: empty name short-circuits', () => { + assert.equal(resolveExecutableBinary('', { platform: 'win32', env: { PATH: '/x' } }), null); + assert.equal(resolveExecutableBinary(null, { platform: 'win32', env: { PATH: '/x' } }), null); + assert.equal(resolveExecutableBinary(undefined, { platform: 'win32', env: { PATH: '/x' } }), null); + }); + + test('R19: posix resolves bare', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo'), ''); + const resolved = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dir } }); + assert.equal(resolved, path.join(dir, 'foo')); + } finally { + cleanup(dir); + } + }); + + test('R20: posix miss → null', () => { + const dir = createTempDir(); + try { + const resolved = resolveExecutableBinary('foo', { platform: 'linux', env: { PATH: dir } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R21: posix ignores PATHEXT', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'gem.CMD'), ''); + const resolved = resolveExecutableBinary('gem', { platform: 'linux', env: { PATH: dir, PATHEXT: '.CMD' } }); + assert.equal(resolved, null); + } finally { + cleanup(dir); + } + }); + + test('R22: unreadable candidate is skipped', () => { + const dir1 = createTempDir(); + const dir2 = createTempDir(); + const originalStatSync = fs.statSync; + try { + fs.writeFileSync(path.join(dir2, 'foo.EXE'), ''); + const firstCandidate = path.join(dir1, 'foo.EXE'); + fs.statSync = (candidate, ...rest) => { + if (candidate === firstCandidate) { + throw Object.assign(new Error('EACCES: permission denied'), { code: 'EACCES' }); + } + return originalStatSync(candidate, ...rest); + }; + const PATH = [dir1, dir2].join(path.delimiter); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH, PATHEXT: '.EXE' } }); + assert.equal(resolved, path.join(dir2, 'foo.EXE')); + } finally { + fs.statSync = originalStatSync; + cleanup(dir1); + cleanup(dir2); + } + }); + + test('R23: resolves when PATH is spelled Path (Windows casing)', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { Path: dir, PATHEXT: '.CMD' } }); + assert.equal(resolved, path.join(dir, 'foo.CMD')); + } finally { + cleanup(dir); + } + }); + + test('R24: resolves when PATHEXT is spelled Pathext', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.XYZ'), ''); + const resolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: dir, Pathext: '.XYZ' } }); + assert.equal(resolved, path.join(dir, 'foo.XYZ')); + + // Negative control: without the differently-cased Pathext key, '.XYZ' + // is not in the default PATHEXT, so resolution must fail. + const unresolved = resolveExecutableBinary('foo', { platform: 'win32', env: { PATH: dir } }); + assert.equal(unresolved, null); + } finally { + cleanup(dir); + } + }); + + test('R25: an exact-case key wins over a differently-cased one', () => { + const dirExact = createTempDir(); + const dirOther = createTempDir(); + try { + fs.writeFileSync(path.join(dirExact, 'foo.CMD'), ''); + const resolved = resolveExecutableBinary('foo', { + platform: 'win32', + env: { PATH: dirExact, Path: dirOther, PATHEXT: '.CMD' }, + }); + assert.equal(resolved, path.join(dirExact, 'foo.CMD')); + } finally { + cleanup(dirExact); + cleanup(dirOther); + } + }); +}); + +// ─── projectSpawnInvocation (#3411) ───────────────────────────────────────── + +// Mirrors the seam's own `_cmdQuoteToken` (a literal `"` is doubled) so +// expectations here are built the same way the implementation builds them, +// without importing the private helper. +function _q(token) { + return `"${String(token).replace(/"/g, '""')}"`; +} + +describe('projectSpawnInvocation (#3411)', () => { + test('P1: .CMD mediates through cmd.exe', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = path.join(dir, 'foo.CMD'); + const env = { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a', 'b'], { platform: 'win32', env }); + assert.deepEqual(result, { + command: 'C:\\Windows\\System32\\cmd.exe', + args: ['/d', '/s', '/c', `"${_q(resolved)} ${_q('a')} ${_q('b')}"`], + resolved, + windowsVerbatimArguments: true, + }); + } finally { + cleanup(dir); + } + }); + + test('P2: .BAT mediates', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.BAT'), ''); + const resolved = path.join(dir, 'foo.BAT'); + const env = { PATH: dir, PATHEXT: '.BAT', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a'], { platform: 'win32', env }); + assert.deepEqual(result, { + command: 'C:\\Windows\\System32\\cmd.exe', + args: ['/d', '/s', '/c', `"${_q(resolved)} ${_q('a')}"`], + resolved, + windowsVerbatimArguments: true, + }); + } finally { + cleanup(dir); + } + }); + + test('P3: .EXE spawns directly', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.EXE'), ''); + const resolved = path.join(dir, 'foo.EXE'); + const env = { PATH: dir, PATHEXT: '.EXE' }; + const result = projectSpawnInvocation('foo', ['a'], { platform: 'win32', env }); + assert.deepEqual(result, { command: resolved, args: ['a'], resolved }); + assert.equal(result.args.includes('/c'), false); + } finally { + cleanup(dir); + } + }); + + test('P4: ComSpec default', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD' }; + const result = projectSpawnInvocation('foo', [], { platform: 'win32', env }); + assert.equal(result.command, 'cmd.exe'); + } finally { + cleanup(dir); + } + }); + + test('P5: unresolved never mediates', () => { + const dir = createTempDir(); + try { + const env = { PATH: dir, PATHEXT: '.EXE;.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('missing-tool', ['a'], { platform: 'win32', env }); + assert.deepEqual(result, { command: 'missing-tool', args: ['a'], resolved: null }); + assert.equal(result.command.includes('cmd'), false); + } finally { + cleanup(dir); + } + }); + + test('P6: posix is a strict no-op', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a'], { platform: 'linux', env }); + assert.deepEqual(result, { command: 'foo', args: ['a'], resolved: null }); + } finally { + cleanup(dir); + } + }); + + test('P7: empty argv', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = path.join(dir, 'foo.CMD'); + const env = { PATH: dir, PATHEXT: '.CMD' }; + const result = projectSpawnInvocation('foo', [], { platform: 'win32', env }); + assert.deepEqual(result.args, ['/d', '/s', '/c', `"${_q(resolved)}"`]); + } finally { + cleanup(dir); + } + }); + + test('P8: argv is not shell-interpolated', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD' }; + const originalArgs = ['a b', 'x&y', 'q"z']; + const result = projectSpawnInvocation('foo', originalArgs, { platform: 'win32', env }); + // Each original argument content survives intact — quoted, never split or + // concatenated by cmd's own metacharacter parsing. + const line = result.args[3]; + assert.ok(line.includes(_q('a b'))); + assert.ok(line.includes(_q('x&y'))); + assert.ok(line.includes(_q('q"z'))); + } finally { + cleanup(dir); + } + }); + + test('P9: unresolved name declaring .cmd still mediates', () => { + const dir = createTempDir(); + try { + const env = { PATH: dir, PATHEXT: '.EXE', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('missing.cmd', ['a'], { platform: 'win32', env }); + assert.deepEqual(result, { + command: 'C:\\Windows\\System32\\cmd.exe', + args: ['/d', '/s', '/c', `"${_q('missing.cmd')} ${_q('a')}"`], + resolved: null, + windowsVerbatimArguments: true, + }); + } finally { + cleanup(dir); + } + }); + + test('P10: unresolved BARE name still does not mediate', () => { + const dir = createTempDir(); + try { + const env = { PATH: dir, PATHEXT: '.EXE', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('missing', ['a'], { platform: 'win32', env }); + assert.deepEqual(result, { command: 'missing', args: ['a'], resolved: null }); + } finally { + cleanup(dir); + } + }); + + test('P11: mediated args are force-quoted so cmd metacharacters cannot split', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a&calc', 'b|c', 'd>e'], { platform: 'win32', env }); + assert.equal(result.windowsVerbatimArguments, true); + assert.equal(result.args.length, 4); + const line = result.args[3]; + assert.ok(line.includes('"a&calc"')); + assert.ok(line.includes('"b|c"')); + assert.ok(line.includes('"d>e"')); + // The bare unquoted sequence must not appear outside of a quoted token — + // every occurrence of `a&calc` in the line is immediately preceded by a + // quote and followed by a quote. + let idx = -1; + while ((idx = line.indexOf('a&calc', idx + 1)) !== -1) { + assert.equal(line[idx - 1], '"'); + assert.equal(line[idx + 'a&calc'.length], '"'); + } + assert.equal(line.startsWith('"'), true); + assert.equal(line.endsWith('"'), true); + } finally { + cleanup(dir); + } + }); + + test('P12: embedded quotes are doubled', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD' }; + const result = projectSpawnInvocation('foo', ['he said "hi"'], { platform: 'win32', env }); + const line = result.args[3]; + assert.ok(line.includes('he said ""hi""')); + } finally { + cleanup(dir); + } + }); + + test('P13: an argument containing a newline is not mediated', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = path.join(dir, 'foo.CMD'); + const env = { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a\nb'], { platform: 'win32', env }); + assert.deepEqual(result, { command: resolved, args: ['a\nb'], resolved }); + assert.equal(result.args.includes('/d'), false); + assert.equal(result.windowsVerbatimArguments, undefined); + } finally { + cleanup(dir); + } + }); + + test('P14: non-mediated returns never set windowsVerbatimArguments', () => { + const dir = createTempDir(); + try { + // POSIX no-op. + const posixResult = projectSpawnInvocation('foo', ['a'], { platform: 'linux', env: {} }); + assert.equal(posixResult.windowsVerbatimArguments, undefined); + + // Unresolved bare name. + const bareEnv = { PATH: dir, PATHEXT: '.EXE', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const bareResult = projectSpawnInvocation('missing', ['a'], { platform: 'win32', env: bareEnv }); + assert.equal(bareResult.windowsVerbatimArguments, undefined); + + // Resolved .EXE. + fs.writeFileSync(path.join(dir, 'foo.EXE'), ''); + const exeEnv = { PATH: dir, PATHEXT: '.EXE' }; + const exeResult = projectSpawnInvocation('foo', ['a'], { platform: 'win32', env: exeEnv }); + assert.equal(exeResult.windowsVerbatimArguments, undefined); + } finally { + cleanup(dir); + } + }); + + test("P15: the mediated line round-trips through cmd's own outer-quote rule", () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = path.join(dir, 'foo.CMD'); + const env = { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a'], { platform: 'win32', env }); + const line = result.args[3]; + // Outer pair immediately followed by the target's own opening quote. + assert.equal(line.startsWith('""'), true); + // Stripping the outer pair leaves the individually-quoted tokens, whose + // first token is the quoted resolved path. + const stripped = line.slice(1, -1); + assert.equal(stripped.startsWith(_q(resolved)), true); + } finally { + cleanup(dir); + } + }); + + test('P16: ComSpec is read case-insensitively', () => { + const dir = createTempDir(); + try { + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const env = { PATH: dir, PATHEXT: '.CMD', COMSPEC: 'C:\\custom\\cmd.exe' }; + const result = projectSpawnInvocation('foo', ['a'], { platform: 'win32', env }); + assert.equal(result.command, 'C:\\custom\\cmd.exe'); + } finally { + cleanup(dir); + } + }); +}); + +// ─── execTool (#3411 windows resolution) ──────────────────────────────────── +// Drives spawnSync via mock.method(childProcess, 'spawnSync') — the seam +// imports childProcess as a namespace precisely so this interception works. + +describe('execTool (#3411 windows resolution)', () => { + test('E4: declared name survives into stderr', (t) => { + mock.method(childProcess, 'spawnSync', () => ({ + error: Object.assign(new Error('x'), { code: 'ENOENT' }), + })); + t.after(() => mock.restoreAll()); + + const result = execTool('some-absent-tool', []); + assert.equal(result.stderr, 'some-absent-tool: not found'); + assert.equal(result.exitCode, 127); + }); + + test('E6: posix passes the bare name', (t) => { + if (process.platform === 'win32') { t.skip('posix-only assertion'); return; } + + let receivedCommand = null; + mock.method(childProcess, 'spawnSync', (command) => { + receivedCommand = command; + return { status: 0, stdout: '', stderr: '', signal: null, error: null }; + }); + t.after(() => mock.restoreAll()); + + execTool('some-declared-name', []); + assert.equal(receivedCommand, 'some-declared-name'); + }); + + test('E7: execTool spawns the DECLARED name when no mediation is required', (t) => { + let received = null; + mock.method(childProcess, 'spawnSync', (command, args) => { + received = { command, args }; + return { status: 0, stdout: '', stderr: '', signal: null, error: null }; + }); + t.after(() => mock.restoreAll()); + + execTool('some-plain-tool', ['--x']); + + assert.equal(received.command, 'some-plain-tool'); + assert.deepEqual(received.args, ['--x']); + }); + + test('E1: posix execTool still runs a real subprocess', () => { + const result = execTool(process.execPath, ['-e', 'process.stdout.write("ok")']); + assert.equal(result.exitCode, 0); + assert.equal(result.stdout, 'ok'); + }); + + test('E2: posix ENOENT contract', (t) => { + if (process.platform === 'win32') { t.skip('posix ENOENT shape'); return; } + + const result = execTool('gsd-definitely-absent-binary-9f3a', []); + assert.equal(result.exitCode, 127); + assert.equal(result.stderr, 'gsd-definitely-absent-binary-9f3a: not found'); + }); + + test('E3: execTool mediates a .cmd through cmd.exe on win32', (t) => { + const dir = createTempDir(); + fs.writeFileSync(path.join(dir, 'foo.CMD'), ''); + const resolved = path.join(dir, 'foo.CMD'); + + let received = null; + mock.method(childProcess, 'spawnSync', (command, args) => { + received = { command, args }; + return { status: 0, stdout: '', stderr: '', signal: null, error: null }; + }); + t.after(() => { + mock.restoreAll(); + cleanup(dir); + }); + + execTool('foo', ['x'], { env: { PATH: dir, PATHEXT: '.CMD', ComSpec: 'C:\\Windows\\System32\\cmd.exe' } }); + + if (process.platform === 'win32') { + assert.deepEqual(received, { + command: 'C:\\Windows\\System32\\cmd.exe', + args: ['/d', '/s', '/c', `"${_q(resolved)} ${_q('x')}"`], + }); + } else { + // Non-win32: projectSpawnInvocation is a strict no-op (process.platform + // drives it, not the injected env), so execTool must hand spawnSync the + // bare declared name unchanged — the POSIX no-op contract. + assert.deepEqual(received, { command: 'foo', args: ['x'] }); + } + }); + + test('E5: options survive mediation', (t) => { + let receivedOptions = null; + mock.method(childProcess, 'spawnSync', (_command, _args, options) => { + receivedOptions = options; + return { status: 0, stdout: '', stderr: '', signal: null, error: null }; + }); + t.after(() => mock.restoreAll()); + + execTool('some-tool', [], { cwd: '/tmp/x', env: { FOO: 'bar' }, timeout: 1234 }); + + assert.equal(receivedOptions.cwd, '/tmp/x'); + assert.equal(receivedOptions.timeout, 1234); + assert.equal(receivedOptions.env.FOO, 'bar'); + // Case-insensitive: process.env's actual key casing is OS-dependent (Windows + // conventionally sets `Path`, not `PATH`), and the merged object here is a + // plain spread of process.env — it no longer benefits from the case-insensitive + // proxy behavior process.env itself has. + assert.equal(Object.keys(receivedOptions.env).some((k) => k.toLowerCase() === 'path'), true); + }); +}); + // ─── dispatchGsdCommand (#2102 Stage 2 — subprocess-shim dispatch to gsd-tools.cjs) ── // // The command-routing hub (`createHub()`) has no fully-populated factory