fix: cross-spawn for Windows .cmd resolution - #1
Open
NexusProperty wants to merge 1 commit into
Open
NexusProperty wants to merge 1 commit into
NexusProperty wants to merge 1 commit into
Conversation
On Windows + Node ≥18.19/20.10/22, `child_process.spawn("qwen", ...)` with
`shell: false` returns ENOENT against the `.cmd` shim that npm installs for
globally-installed CLIs. This is CVE-2024-27980's mitigation refusing to
resolve `.cmd` files without an explicit shell. The setup probe in
`lib/process.mjs::runCommand` masks the bug — it uses `shell: true` on
Windows — but `runQwenTurn` in `lib/qwen.mjs` uses the streaming `spawn`
directly, so `qwen-companion task` and `qwen-companion rescue` die with
`spawn qwen ENOENT` before producing any output.
Empirically reproducible:
> where qwen
C:\Users\Jackc\AppData\Roaming\npm\qwen.cmd
> node qwen-companion.mjs setup --json
{ "ready": true, "qwen": { "available": true } ... } # probe lies
> node qwen-companion.mjs task "Say hi"
spawn qwen ENOENT
The fix adds `cross-spawn@7.0.6` as the project's first runtime dependency.
It resolves the `.cmd` shim to its absolute path and exec's it through
Node's spawn primitives, bypassing cmd.exe entirely (so no shell-injection
risk from user prompts passed as argv to `runQwenTurn`).
Single patch site:
- `plugins/qwen/scripts/lib/qwen.mjs` — replace `import { spawn } from
"node:child_process"` with a `crossSpawn` wrapper. All existing
`spawn(...)` call sites work unchanged.
Other spawn sites verified immune:
- `qwen-companion.mjs:630` spawns `process.execPath` (node.exe, true exe)
- `stop-review-gate-hook.mjs:106` uses `spawnSync(process.execPath, ...)`
- `lib/process.mjs:5` (probe path) uses `shell: win32 ? ... : false`
No new test added — the existing `runQwenTurn:` tests in
`tests/runtime.test.mjs` already use `installFakeQwen()` which writes a
`qwen.cmd` shim on Windows (`fake-qwen-fixture.mjs:162-164`), so those
tests already exercise the streaming-spawn-against-.cmd path this fix
targets. Pre-patch on Windows + Node 22 they ENOENT'd; post-patch all
8 `runQwenTurn:` tests pass.
Version bumped 1.1.2 → 1.1.3 (backwards-compatible bugfix).
Verification (Windows + Node v22.14.0):
- All `runQwenTurn:` tests pass post-patch
- `npm audit signatures`: 6/6 verified
- `cross-spawn@7.0.6` supply-chain CLEAN (Socket vulnerability=100,
supplyChain=99; single maintainer `satazor` since 2014;
GHSA-3xgq-45jj-v275 ReDoS patched in 7.0.5; no 2026 publish activity)
- NOT FIXED BY THIS PATCH: 4 pre-existing `process.test.mjs` failures
(`runCommand`, `runCommandChecked`, `terminateProcessTree: ESRCH`) and
a 90s timeout in `tests/stop-gate.test.mjs` reproduce on pristine main
pre-patch — unrelated to spawn ENOENT and out of scope here.
Mirrors the canonical fix shipped in sibling plugin cli-companion
(commit 5f7d65e at NexusProperty/cli-companion, PR #106).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows + Node ≥18.19/20.10/22,
taskandrescuemodes silently fail withspawn qwen ENOENTwhilesetup --jsonreports the plugin as ready. Root cause is CVE-2024-27980's mitigation refusing to resolve.cmdshims (every npm-globally-installed CLI) without an explicit shell. Setup probe useslib/process.mjs::runCommandwhich already doesshell: trueon Windows;runQwenTurnuses the streamingspawndirectly, so it dies before producing any output.Empirical repro (this machine, Node v22.14.0 + Windows 10)
Fix
Single import change in
plugins/qwen/scripts/lib/qwen.mjs:cross-spawnresolves the.cmdshim to its absolute path and exec's it through Node's spawn primitives, bypassing cmd.exe entirely. No shell-injection risk from user prompts passed as argv torunQwenTurn.Test plan
runQwenTurn:tests intests/runtime.test.mjspass (8/8). These already useinstallFakeQwen()which writes aqwen.cmdshim on Windows (tests/fake-qwen-fixture.mjs:162-164), so they exercise the streaming-spawn-against-.cmd path this fix targets. Pre-patch they ENOENT'd; post-patch they pass — no new regression test needed.npm audit signatures: 6/6 verifiedcross-spawn@7.0.6: CLEAN (Socket: vulnerability=100, supplyChain=99; single maintainersatazorsince 2014; GHSA-3xgq-45jj-v275 ReDoS patched in 7.0.5; no 2026 publish activity = not in current Shai-Hulud / Mini Shai-Hulud waves)Out of scope (pre-existing, NOT caused by this patch)
Pristine
mainalready has these failures on Windows + Node 22 — confirmed bygit stash && node --test tests/process.test.mjs:process.test.mjs: 4 failures (runCommand: captures stdout on success,runCommandChecked: throws on non-zero exit,runCommandChecked: returns result on success,terminateProcessTree: ESRCH on non-existent pid is handled gracefully)tests/stop-gate.test.mjs: 90s timeoutHappy to file a separate issue/PR for these if you'd like — they look like Windows-shell-config-sensitivity in
lib/process.mjswhereshell: process.env.SHELL || truemay pick up git-bash'ssh.exe.Other spawn sites verified immune
qwen-companion.mjs:630spawnsprocess.execPath(node.exe, true executable)stop-review-gate-hook.mjs:106usesspawnSync(process.execPath, ...)Version bumped 1.1.2 → 1.1.3 (backwards-compatible bugfix).
Mirrors the canonical fix from sibling plugin cli-companion (commit
5f7d65e, PR #106 there). Same Node 22 + Windows class, identical patch shape.