fix(#2667): run-with-timeout mediates .cmd/.bat spawns on Windows (CVE-2024-27980); fallow pre-pass names failure kind (#2897)

* fix(#2667): mediate .cmd/.bat/.exe spawns on Windows; split fallow pre-pass failure diagnostic

run-with-timeout spawned .cmd/.bat/.exe commands without shell:true on Windows,
tripping Node's CVE-2024-27980 EINVAL (April 2024 security hardening). The fallow
structural pre-pass then no-op'd silently — a hard execution failure read the same
as 'optional dependency absent'.

(A) gsd-core/bin/gsd-tools.cjs runWithTimeout: gate shell:true on
    (win32 && command ends in .cmd/.bat/.exe). Narrow by design — never fires for
    the 7 `bash -c` callers (command is `bash`, no such suffix), so the recorded
    no-shell-for-argv-array security contract (DEFECT.UNBOUNDED-SUBPROCESS) is
    preserved; cmdArgs stays an array. POSIX untouched.
(B) code-review.md fallow pre-pass: name the failure KIND (timeout / spawn failure
    / crash / not-found) so a Windows .cmd spawn failure is not mistaken for an
    absent binary.

Regression test in tests/run-with-timeout.test.cjs gated to win32 (.cmd/.bat/.exe
shims run with exit 0 + non-empty stdout; pre-fix EINVAL → exit 125/empty). POSIX
negative-space test guards the unchanged bash -c callers.

* chore(#2667): changeset fragment

* chore(#2667): backfill changeset PR 2897 + correct body (cmd.exe array, not shell:true)

* fix(#2667): exclude .exe from the win32 spawn-mediation gate; ack code-review.md growth

CI caught two failures on the first push:

1. windows-24: 'exits 124 when the wall-clock budget is exceeded' regressed. The
   gate matched .exe, so the HANG command (node.exe -e 'setTimeout(...)') was
   wrapped in 'cmd.exe /c node.exe ...' — the wrapped child escaped the timeout
   cap's process-group reap (exit 124 never fired; hit the 30s harness backstop)
   AND cmd.exe risked mis-parsing the -e script arg. .exe is INTENTIONALLY
   excluded now: real PE executables spawn fine directly; only .cmd/.bat are the
   CVE-2024-27980 EINVAL cases. The .exe test becomes a negative-space test
   (node.exe spawned directly, exit 0).

2. ubuntu-22: emitted-attribution — code-review.md grew 1177 bytes from the
   #2667 fallow pre-pass failure-KIND case statement; acknowledge it.

---------

Co-authored-by: Test <test@example.com>

.changeset/gallant-koalas-forage.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 2897
+---
+**Fallow structural pre-pass no longer silently no-ops on Windows** — `run-with-timeout` now mediates `.cmd`/`.bat`/`.exe` spawns via an explicit `cmd.exe /c` argv array (Node's CVE-2024-27980 hardening requires a shell for these on Windows), and the fallow pre-pass names the failure kind so a Windows spawn failure is not mistaken for an absent binary. The existing `bash -c` callers and POSIX behavior are unchanged. (#2667)

gsd-core/bin/gsd-tools.cjs

@@ -3033,6 +3033,29 @@ function runWithTimeout(argv) {
   const detached = !isWin && secs > 0;
   const spawnFailureCode = (err) =>
     (err && err.code === 'ENOENT' ? 127 : err && err.code === 'EACCES' ? 126 : 125);
+  // #2667: on Windows, a `.cmd`/`.bat`/`.exe` command cannot be spawned directly
+  // — Node's CVE-2024-27980 hardening (April 2024, all active lines incl. 22.x)
+  // throws EINVAL when child_process.spawn is given a `.cmd`/`.bat` without a
+  // shell, so e.g. `run-with-timeout 120 -- node_modules/.bin/fallow.cmd` silently
+  // produced empty stdout + exit 125 and the fallow pre-pass no-op'd.
+  //
+  // We do NOT use `shell: true` for this: with `shell:true`, Node space-joins the
+  // unescaped cmdArgs into a cmd.exe command string (DEP0190) — that would re-open
+  // a shell-injection surface and violate the recorded no-shell-for-argv-array
+  // contract (DEFECT.UNBOUNDED-SUBPROCESS, CONTEXT.md:772). Instead we spawn
+  // `cmd.exe /c <cmd> <args>` with an explicit argv ARRAY, which is what Node's
+  // own exec does internally and keeps every arg a discrete, un-interpolated
+  // token. The gate is NARROW: it fires ONLY for the Windows shim extensions,
+  // never for the `bash -c` callers (command is `bash`, no such suffix), so the 7
+  // bash callers keep their array-only argv on every platform. POSIX untouched.
+  // NOTE: .exe is INTENTIONALLY excluded — real PE executables (node.exe, etc.)
+  // spawn fine directly and mediating them through cmd.exe /c breaks the timeout
+  // cap's process-group kill (the wrapped child escapes reap → exit 124 never
+  // fires) and risks cmd.exe mis-parsing an arg like `-e "setTimeout(()=>{})"`.
+  // Only .cmd/.bat are the CVE-2024-27980 EINVAL cases that require mediation.
+  const winShim = isWin && /\.(cmd|bat)$/i.test(path.basename(cmd));
+  const spawnCmd = winShim ? (process.env.ComSpec || 'cmd.exe') : cmd;
+  const spawnArgs = winShim ? ['/d', '/s', '/c', cmd, ...cmdArgs] : cmdArgs;
   // Node's setTimeout delay is a 32-bit signed ms int; a larger value silently
   // clamps to 1ms → a spurious immediate timeout. Cap the budget (~24.8 days).
   const timerMs = Math.min(Math.round(secs * 1000), 2 ** 31 - 1);
@@ -3043,7 +3066,11 @@ function runWithTimeout(argv) {
   return new Promise((resolve) => {
     let child;
     try {
-      child = spawn(cmd, cmdArgs, { stdio: 'inherit', detached });
+      // #2667: on win32 `.cmd`/`.bat`/`.exe`, spawn cmd.exe with an explicit argv
+      // array (spawnCmd/spawnArgs) rather than the shim directly — preserves the
+      // array-only, no-shell-string argv contract. `detached` is always false on
+      // win32, so it never co-occurs with the cmd.exe mediation.
+      child = spawn(spawnCmd, spawnArgs, { stdio: 'inherit', detached });
     } catch (err) {
       process.stderr.write(`run-with-timeout: ${cmd}: ${err && err.message ? err.message : 'failed to start'}\n`);
       resolve(spawnFailureCode(err));

gsd-core/workflows/code-review.md

@@ -464,7 +464,22 @@ FALLOW_OK=$(FALLOW_TMP=\"${FALLOW_JSON_PATH}.tmp\" node -e \"
 if [ \"$FALLOW_OK\" != \"1\" ]; then
   FALLOW_STDERR_SUMMARY=$(head -5 \"$FALLOW_STDERR_TMP\")
   rm -f \"${FALLOW_JSON_PATH}.tmp\" \"$FALLOW_STDERR_TMP\"
-  echo \"WARNING: fallow structural pre-pass failed (exit ${FALLOW_EXIT}): ${FALLOW_STDERR_SUMMARY}\"
+  # #2667: distinguish a hard EXECUTION failure (the binary was found at step 1
+  # but would not run) from the binary-missing path (step 2). Exit 124 = timeout,
+  # 2 = usage error, 125 = spawn failure (e.g. Windows EINVAL on a .cmd shim —
+  # CVE-2024-27980, now mediated by run-with-timeout), 126/127 = not executable /
+  # not found. A non-zero exit here with a resolved binary means fallow is
+  # installed but did not produce a report — surface that loudly so a Windows
+  # user does not mistake it for "fallow absent".
+  case \"$FALLOW_EXIT\" in
+    124) FALLOW_FAIL_KIND=\"timed out\" ;;
+    2)   FALLOW_FAIL_KIND=\"usage error\" ;;
+    125) FALLOW_FAIL_KIND=\"spawn failure (the binary was found but did not start — e.g. a Windows .cmd shim; run-with-timeout mediates this)\" ;;
+    126) FALLOW_FAIL_KIND=\"not executable\" ;;
+    127) FALLOW_FAIL_KIND=\"not found\" ;;
+    *)   FALLOW_FAIL_KIND=\"crashed\" ;;
+  esac
+  echo \"WARNING: fallow structural pre-pass failed (${FALLOW_FAIL_KIND}, exit ${FALLOW_EXIT}): ${FALLOW_STDERR_SUMMARY}\"
   FALLOW_JSON_PATH=\"\"
 else
   mv \"${FALLOW_JSON_PATH}.tmp\" \"$FALLOW_JSON_PATH\"
@@ -472,7 +487,7 @@ else
 fi
 ```
 
-On any failure of the structural pre-pass (binary missing, timeout, empty output, or unparseable JSON), the workflow continues with no `<structural_findings>` injection; the reviewer agent receives a normal review request.
+On any failure of the structural pre-pass (binary missing at step 2, or an execution failure here — timeout, spawn failure, crash, empty output, or unparseable JSON), the workflow continues with no `<structural_findings>` injection; the reviewer agent receives a normal review request. The WARNING above names the failure KIND so a hard execution failure (e.g. a Windows `.cmd` spawn failure) is not mistaken for an absent optional dependency.
 
 4) Optional MCP bridge path (runtime-dependent):
 - If `FALLOW_MCP=true`, set reviewer input mode to MCP-backed structural findings.

tests/emitted-drift-ack.json

@@ -14,7 +14,7 @@
       "reason": "#2800 — same derived-flag-loop change as autonomous.md. next.md needed no relocation (its launcher preamble already precedes the loop), so the growth here is only the loop plus the comment recording that --all and --text stay literal because they are convergence controls, not reviewer lanes."
     },
     "code-review.md": {
-      "reason": "#2666: the Tier-2 SUMMARY extractor predicate is relaxed to accept root-level and extensionless build files (Dockerfile/Makefile/etc.), and the Tier-3 git-diff fallback is converted from an eq-zero gate into an intersect-and-warn that cross-checks the SUMMARY scope against `git diff --name-only` with exact whole-line matching (grep -Fxq) and warns about + adds any changed files the extractor missed. Growth is the relaxed predicate + the portable cross-check branch + their explanatory comments."
+      "reason": "#2667: the fallow structural pre-pass failure WARNING now names the failure KIND (timeout / spawn failure / crash / not-found) via a case statement, so a Windows .cmd spawn failure (CVE-2024-27980) is not mistaken for an absent binary. Growth is the case statement + the explanatory prose."
     }
   }
 }

tests/run-with-timeout.test.cjs

@@ -199,6 +199,61 @@ describe('#2351 run-with-timeout — kill semantics (POSIX process groups)', ()
   });
 });
 
+describe('#2667 run-with-timeout — Windows .cmd/.bat/.exe spawn mediation (CVE-2024-27980)', () => {
+  // Node's CVE-2024-27980 hardening throws EINVAL when child_process.spawn is
+  // given a .cmd/.bat without a shell. run-with-timeout now mediates .cmd/.bat
+  // on win32 via an explicit `cmd.exe /d /s /c <cmd> ...args` argv ARRAY (not
+  // shell:true — that space-joins unescaped args per DEP0190), while leaving
+  // every `bash`/argv-array caller unchanged (the recorded no-shell-for-argv-
+  // array contract). .exe is INTENTIONALLY excluded — real PEs spawn fine
+  // directly and mediating them breaks the timeout reap + risks arg mis-parse.
+  const isWin = process.platform === 'win32';
+
+  test('win32 RED: a .cmd shim runs (exit 0, non-empty stdout) — pre-fix this threw EINVAL → exit 125 / empty stdout', { skip: !isWin ? 'win32-only' : false }, () => {
+    const dir = createTempDir('rwt-2667-cmd');
+    try {
+      // A .cmd shim that echoes JSON to stdout (mimics fallow.cmd audit --format json).
+      const shim = path.join(dir, 'fake.cmd');
+      fs.writeFileSync(shim, '@echo {"verdict":"clean"}\r\n', 'utf8');
+      const r = runVerb(['10', '--', shim]);
+      assert.equal(r.status, 0, `expected the .cmd shim to run (exit 0); got ${r.status}. stderr: ${r.stderr}`);
+      assert.ok((r.stdout || '').includes('clean'), `expected non-empty JSON stdout from the .cmd shim; got: ${r.stdout}`);
+    } finally {
+      cleanup(dir);
+    }
+  });
+
+  test('win32: a .bat shim is also mediated (exit 0, non-empty stdout)', { skip: !isWin ? 'win32-only' : false }, () => {
+    const dir = createTempDir('rwt-2667-bat');
+    try {
+      const shim = path.join(dir, 'fake.bat');
+      fs.writeFileSync(shim, '@echo {"verdict":"clean"}\r\n', 'utf8');
+      const r = runVerb(['10', '--', shim]);
+      assert.equal(r.status, 0, `expected the .bat shim to run (exit 0); got ${r.status}. stderr: ${r.stderr}`);
+      assert.ok((r.stdout || '').length > 0, 'expected non-empty stdout from the .bat shim');
+    } finally {
+      cleanup(dir);
+    }
+  });
+
+  test('win32 negative-space: a .exe (node.exe) is spawned DIRECTLY, not mediated — no cmd.exe wrap', { skip: !isWin ? 'win32-only' : false }, () => {
+    // .exe is intentionally excluded from the gate: real PE executables spawn
+    // fine directly, and wrapping them in cmd.exe /c breaks the timeout cap's
+    // process-group reap AND risks cmd.exe mis-parsing args (e.g. -e "code()").
+    // node.exe -e "process.exit(0)" must exit 0 directly.
+    const r = runVerb(['10', '--', process.execPath, '-e', 'process.exit(0)']);
+    assert.equal(r.status, 0, `expected node.exe to run directly (exit 0); got ${r.status}. stderr: ${r.stderr}`);
+  });
+
+  test('POSIX negative-space: a bash -c caller is unchanged (no shell:true added) — argv stays array-only', { skip: isWin ? 'posix-only' : false }, () => {
+    // The fix's gate (win32 && .cmd/.bat) skips `bash` on POSIX: behavior
+    // must be identical to before. `bash -c 'echo ok'` exits 0 with stdout "ok".
+    const r = runVerb(['10', '--', 'bash', '-c', 'echo ok']);
+    assert.equal(r.status, 0, `expected bash caller to still work (exit 0); got ${r.status}`);
+    assert.equal((r.stdout || '').trim(), 'ok', 'expected stdout "ok" from the unchanged bash caller');
+  });
+});
+
 describe('#2351 run-with-timeout — coreutils independence (the regression)', () => {
   // The whole point: no dependency on GNU `timeout`/`gtimeout`. Prove it by
   // scrubbing PATH so neither could be found, and driving the child by absolute