fix(providers): use shell:true on win32 for zcode .cmd/.bat shim spawn (#13963)

changelog.d/fixes/13963-zcode-win32-shell-shim.md

@@ -0,0 +1 @@
+- fix(providers): use shell:true on win32 for zcode .cmd/.bat shim spawn (#13963)

open-sse/executors/zcodeProtocol.ts

@@ -1,4 +1,5 @@
 import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
+import { shouldUseShellForCommand } from "@/shared/services/cliRuntime";
 
 const HEADER_SIZE = 13;
 const REGULAR_MESSAGE = 1;
@@ -154,6 +155,21 @@ export function encodeZcodeRpcCall(
   return frame;
 }
 
+/**
+ * Whether spawn() must go through the shell to launch `command`.
+ *
+ * On win32, npm installs global CLI wrappers (e.g. the `zcode`/ZCODE_BIN
+ * shim) as `.cmd`/`.bat` files. Since Node's CVE-2024-27980 fix, `spawn()`
+ * refuses to launch a `.cmd`/`.bat` target without `shell: true`, throwing
+ * ENOENT/EINVAL instead (#13963, same class of bug as #8590/Qoder). The
+ * bundled ZCODE_SERVER_NODE runtime path spawns a bare `node`/`node.exe`
+ * binary (no `.cmd`/`.bat` extension) and must keep `shell: false` even on
+ * win32 — `shouldUseShellForCommand()` already encodes that extension check.
+ */
+export function shouldUseShellForZcodeCommand(command: string): boolean {
+  return shouldUseShellForCommand(command);
+}
+
 function errorFromPayload(payload: unknown, fallback: string): Error {
   if (payload && typeof payload === "object") {
     const record = payload as JsonRecord;
@@ -212,7 +228,8 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
         cwd: this.cwd,
         env: this.env ? { ...process.env, ...this.env } : process.env,
         stdio: ["pipe", "pipe", "pipe"],
-        shell: false,
+        // shell:true on win32 for a .cmd/.bat ZCode shim — see #13963/#8590.
+        shell: shouldUseShellForZcodeCommand(this.command),
         windowsHide: true,
       });
     } catch (error) {
@@ -260,7 +277,11 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     });
 
     try {
-      await this.withTimeout(readyPromise, this.startupTimeoutMs, "ZCode app-server handshake timed out");
+      await this.withTimeout(
+        readyPromise,
+        this.startupTimeoutMs,
+        "ZCode app-server handshake timed out"
+      );
       this.ready = true;
     } catch (error) {
       await this.disposeChild(child);
@@ -279,9 +300,10 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     this.pendingChunks.push(chunk);
     let total = 0;
     for (const part of this.pendingChunks) total += part.byteLength;
-    const buffer = total === chunk.byteLength && this.pendingChunks.length > 0
-      ? chunk
-      : Buffer.concat(this.pendingChunks);
+    const buffer =
+      total === chunk.byteLength && this.pendingChunks.length > 0
+        ? chunk
+        : Buffer.concat(this.pendingChunks);
     this.pendingChunks = [buffer];
 
     if (!this.handshakeDone) {
@@ -307,11 +329,13 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
       }
       const child = this.child;
       if (!child) return;
-      child.stdin.write(`${JSON.stringify({
-        type: "zcode-hello-ack",
-        version: "omniroute",
-        clientId: `omniroute-${process.pid}`,
-      })}\n`);
+      child.stdin.write(
+        `${JSON.stringify({
+          type: "zcode-hello-ack",
+          version: "omniroute",
+          clientId: `omniroute-${process.pid}`,
+        })}\n`
+      );
       this.handshakeDone = true;
     }
     this.consumeFrames();
@@ -366,10 +390,12 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     if (type === RESPONSE_MESSAGE) {
       request.resolve(payload);
     } else {
-      request.reject(errorFromPayload(
-        payload,
-        type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
-      ));
+      request.reject(
+        errorFromPayload(
+          payload,
+          type === ERROR_MESSAGE ? "ZCode RPC request failed" : "ZCode RPC request canceled"
+        )
+      );
     }
   }
 
@@ -437,7 +463,11 @@ export class ZcodeAppServerClient implements ZcodeClientLike {
     }
   }
 
-  private async withTimeout<T>(promise: Promise<T>, timeoutMs: number, message: string): Promise<T> {
+  private async withTimeout<T>(
+    promise: Promise<T>,
+    timeoutMs: number,
+    message: string
+  ): Promise<T> {
     let timer: ReturnType<typeof setTimeout> | undefined;
     try {
       return await Promise.race([

tests/unit/zcode-win32-spawn-13963.test.ts

@@ -0,0 +1,78 @@
+/**
+ * Regression test for #13963 — Windows: `zc`/`zcode` provider always fails
+ * with `spawn zcode ENOENT` even when ZCODE_BIN is set.
+ *
+ * Root cause: ZcodeAppServerClient.start() (open-sse/executors/zcodeProtocol.ts)
+ * spawns the ZCode CLI with a hardcoded `shell: false`, regardless of
+ * process.platform or the file extension of the resolved command. On
+ * Windows, npm installs global CLI wrappers as `.cmd`/`.bat` shims, and
+ * since Node's CVE-2024-27980 fix, `spawn()` refuses to launch a `.cmd`/
+ * `.bat` target without `shell: true`, throwing ENOENT/EINVAL instead. This
+ * is the same class of bug as #8590 (Qoder), already fixed elsewhere in
+ * this repo (devin-cli.ts, auggie.ts, cliRuntime.ts's
+ * shouldUseShellForCommand()).
+ *
+ * `shouldUseShellForZcodeCommand()` is a small, pure, exported helper so
+ * this can be asserted directly without needing to intercept the live ESM
+ * `spawn` binding (node:child_process.spawn is unmockable via mock.method()
+ * without --experimental-test-module-mocks, which is not enabled in
+ * `npm run test:unit` — see tests/unit/windows-hide-child-process-spawns-8131.test.ts).
+ */
+
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { shouldUseShellForZcodeCommand } =
+  await import("@omniroute/open-sse/executors/zcodeProtocol");
+
+/** Temporarily override process.platform for the duration of `fn`. */
+function withPlatform<T>(platform: string, fn: () => T): T {
+  const original = Object.getOwnPropertyDescriptor(process, "platform")!;
+  Object.defineProperty(process, "platform", { value: platform, configurable: true });
+  try {
+    return fn();
+  } finally {
+    Object.defineProperty(process, "platform", original);
+  }
+}
+
+test("shouldUseShellForZcodeCommand returns true on win32 for a .cmd shim", () => {
+  const result = withPlatform("win32", () =>
+    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.cmd")
+  );
+  assert.equal(
+    result,
+    true,
+    "spawn() must use shell:true on win32 when the resolved ZCode binary is a " +
+      ".cmd/.bat shim, or launching it throws ENOENT/EINVAL (Node CVE-2024-27980 fix) " +
+      "— see #8590 (Qoder) for the same class of bug already fixed elsewhere in this repo"
+  );
+});
+
+test("shouldUseShellForZcodeCommand returns true on win32 for a .bat shim", () => {
+  const result = withPlatform("win32", () =>
+    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\AppData\\Roaming\\npm\\zcode.bat")
+  );
+  assert.equal(result, true);
+});
+
+test("shouldUseShellForZcodeCommand stays false on win32 for the bundled node runtime path", () => {
+  // The ZCODE_SERVER_NODE bundled-runtime path (open-sse/executors/zcode.ts:96-101)
+  // spawns a bare `node`/`node.exe` binary, not a .cmd/.bat shim — must stay shell:false.
+  const result = withPlatform("win32", () =>
+    shouldUseShellForZcodeCommand("C:\\Users\\aaaaa\\.zcode\\server\\node.exe")
+  );
+  assert.equal(result, false);
+});
+
+test("shouldUseShellForZcodeCommand returns false on linux even for a .cmd-named command", () => {
+  const result = withPlatform("linux", () =>
+    shouldUseShellForZcodeCommand("/usr/local/bin/zcode.cmd")
+  );
+  assert.equal(result, false);
+});
+
+test("shouldUseShellForZcodeCommand returns false on darwin for the plain zcode binary", () => {
+  const result = withPlatform("darwin", () => shouldUseShellForZcodeCommand("zcode"));
+  assert.equal(result, false);
+});