chore(quality): re-point the zcodeProtocol public-creds freeze at the literal's new line (#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)

changelog.d/fixes/14070-agentbridge-rootca-cert-trust.md

@@ -1 +0,0 @@
-- fix(mitm): resolve AgentBridge cert-trust check against the active cert model instead of hard-coded server.crt (#14070)

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([

scripts/check/check-public-creds.mjs

@@ -90,18 +90,18 @@ const ENV_KEY_RE = /(clientId|clientSecret|apiKey)Env\s*:/;
 //   The MiniMax family was extracted from services/usage.ts into services/usage/minimax.ts
 //   (god-file decomposition), so the FP moved with the getMiniMaxUsage signature.
 //
-//   open-sse/executors/zcodeProtocol.ts L313: `clientId: \`omniroute-${process.pid}\``
+//   open-sse/executors/zcodeProtocol.ts L336: `clientId: \`omniroute-${process.pid}\``
 //   is the per-process identifier in the local ZCode app-server handshake. It is
 //   generated from the process PID, is not an upstream OAuth/client credential, and
 //   must remain visible in the wire contract. Frozen by file:line:value key.
 //   NOTE: the key includes the LINE, so any edit that shifts this statement breaks
 //   the gate twice over — a stale-entry error plus a "new violation" for the same
-//   literal. That is what happened here (L302 -> L313). Re-point the line; do not
-//   remove the entry.
+//   literal. That happened again at L313 -> L336 (#13963, win32 shell:true spawn
+//   fix). Re-point the line; do not remove the entry.
 export const KNOWN_LITERAL_CREDS = new Set([
   "open-sse/services/usage/minimax.ts:213:minimax", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
   "open-sse/services/usage/minimax.ts:213:minimax-cn", // TODO(6A.8): pre-existing FP — TS fn-param type, not a credential (getMiniMaxUsage signature)
-  "open-sse/executors/zcodeProtocol.ts:313:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
+  "open-sse/executors/zcodeProtocol.ts:336:omniroute-${process.pid}", // local per-process ZCode handshake ID, not an upstream credential
 ]);
 
 /**

src/app/api/tools/agent-bridge/diagnose/route.ts

@@ -19,7 +19,6 @@ import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 import { createErrorResponse } from "@/lib/api/errorResponse";
 import { getMitmStatus } from "@/mitm/manager";
 import { checkCertInstalled } from "@/mitm/cert/install";
-import { resolveActiveCertPath } from "@/mitm/cert/activeCert";
 import { resolveMitmDataDir } from "@/mitm/dataDir";
 import { summarizeDiagnostics } from "@/mitm/inspector/diagnostics";
 import { getAllAgentBridgeStates } from "@/lib/db/agentBridgeState";
@@ -47,12 +46,7 @@ export async function GET(request: Request): Promise<Response> {
   try {
     const agentId = new URL(request.url).searchParams.get("agentId") ?? undefined;
     const status = await getMitmStatus(agentId);
-    // #14070: resolve the file the active migration decision actually
-    // installs (ca.crt under the root-CA model) instead of always
-    // hard-coding the legacy server.crt path.
-    const certDir = path.join(resolveMitmDataDir(), "mitm");
-    const rootCaEnabled = process.env.MITM_ROOT_CA_ENABLED === "true";
-    const { certPath } = resolveActiveCertPath(certDir, rootCaEnabled);
+    const certPath = path.join(resolveMitmDataDir(), "mitm", "server.crt");
     const certExists = fs.existsSync(certPath);
     const certTrusted = certExists ? await checkCertInstalled(certPath) : false;
     const port =

src/app/api/tools/agent-bridge/server/route.ts

@@ -7,9 +7,8 @@
  */
 import { AgentBridgeServerActionSchema } from "@/shared/schemas/agentBridge";
 import { getCachedPassword, setCachedPassword } from "@/mitm/manager";
-import { installCertResult, installCaCert, checkCertInstalled } from "@/mitm/cert/install";
+import { installCertResult, checkCertInstalled } from "@/mitm/cert/install";
 import { generateCert } from "@/mitm/cert/generate";
-import { resolveActiveCertPath } from "@/mitm/cert/activeCert";
 import { resolveMitmDataDir } from "@/mitm/dataDir";
 import {
   isMitmSudoPasswordRequired,
@@ -105,17 +104,8 @@ export async function POST(request: Request): Promise<Response> {
       if (isMitmSudoPasswordRequired(sudoPassword)) {
         return createErrorResponse({ status: 400, message: "Missing sudoPassword" });
       }
-      // #14070: resolve + trust the file the active migration decision
-      // actually installs (ca.crt via installCaCert() under the root-CA
-      // model) instead of always hard-coding/trusting the legacy
-      // server.crt — mirrors manager.ts's own branch (startMitmInternal).
-      const certDir = path.join(resolveMitmDataDir(), "mitm");
-      const rootCaEnabled = process.env.MITM_ROOT_CA_ENABLED === "true";
-      const { certPath, mode } = resolveActiveCertPath(certDir, rootCaEnabled);
-      const result =
-        mode === "use-root-ca"
-          ? await installCaCert(sudoPassword, certPath)
-          : await installCertResult(sudoPassword, certPath);
+      const certPath = path.join(resolveMitmDataDir(), "mitm", "server.crt");
+      const result = await installCertResult(sudoPassword, certPath);
       if (result.installed) {
         const suppliedPassword =
           typeof raw.sudoPassword === "string"

src/app/api/tools/agent-bridge/state/route.ts

@@ -15,7 +15,6 @@ import { getAllAgentBridgeStates } from "@/lib/db/agentBridgeState";
 import { getAllBypassPatterns } from "@/lib/db/agentBridgeBypass";
 import { getMappingsForAgent } from "@/lib/db/agentBridgeMappings";
 import { checkCertInstalled } from "@/mitm/cert/install";
-import { resolveActiveCertPath } from "@/mitm/cert/activeCert";
 import { resolveMitmDataDir } from "@/mitm/dataDir";
 import { ALL_TARGETS } from "@/mitm/targets/index";
 import path from "path";
@@ -43,13 +42,9 @@ export async function GET(): Promise<Response> {
     );
     const mappings = Object.fromEntries(mappingsEntries);
 
-    // Compute REAL certTrusted (OS trust store check, not just file exists).
-    // #14070: resolve the file the active migration decision actually
-    // installs (ca.crt under the root-CA model) instead of always
-    // hard-coding the legacy server.crt path.
+    // Compute REAL certTrusted (OS trust store check, not just file exists)
     const certDir = path.join(resolveMitmDataDir(), "mitm");
-    const rootCaEnabled = process.env.MITM_ROOT_CA_ENABLED === "true";
-    const { certPath } = resolveActiveCertPath(certDir, rootCaEnabled);
+    const certPath = path.join(certDir, "server.crt");
     const certExists = fs.existsSync(certPath);
     const certTrusted = certExists ? await checkCertInstalled(certPath) : false;
 

src/mitm/cert/activeCert.ts

@@ -1,34 +0,0 @@
-import path from "path";
-import { decideCertMigration, type CertMigrationDecision } from "./migration.ts";
-
-// #14070: single source of truth for "which cert file is actually active
-// (installed + trusted) for this run". `decideCertMigration()` only answers
-// which MODEL is active; every caller that needs the concrete file path used
-// to independently re-derive it, and three of the four call sites (the
-// AgentBridge `state`/`diagnose`/`trust-cert` routes) hard-coded the legacy
-// `server.crt` path regardless of the decision — so once a install adopted
-// the root-CA model (`ca.crt`), those routes kept checking/trusting the
-// wrong file forever. `startMitmInternal()` (`../manager.ts`) is the only
-// place that got this right; this helper factors that logic out so every
-// caller (including `manager.ts` itself) resolves the same path the same
-// way.
-
-/** The cert file actually active for this run, plus which model produced it. */
-export interface ActiveCertInfo {
-  certPath: string;
-  mode: CertMigrationDecision;
-}
-
-/**
- * Resolve the cert file the active migration decision installs/trusts for
- * `certDir`. Mirrors `rootCa.ts`'s `ca.crt` filename under `"use-root-ca"`
- * and `generate.ts`'s `server.crt` filename under `"use-legacy-leaf"` — pure
- * path arithmetic, no filesystem I/O beyond what `decideCertMigration()`
- * itself performs (existence checks only).
- */
-export function resolveActiveCertPath(certDir: string, rootCaEnabled: boolean): ActiveCertInfo {
-  const mode = decideCertMigration(certDir, rootCaEnabled);
-  const certPath =
-    mode === "use-root-ca" ? path.join(certDir, "ca.crt") : path.join(certDir, "server.crt");
-  return { certPath, mode };
-}

src/mitm/manager.ts

@@ -12,7 +12,7 @@ import { provisionDnsEntries } from "./dns/provision.ts";
 import { generateCert } from "./cert/generate.ts";
 import { installCertResult, installCaCert } from "./cert/install.ts";
 import { loadOrCreateMitmCa, resolveMitmCertDir } from "./cert/rootCa.ts";
-import { resolveActiveCertPath } from "./cert/activeCert.ts";
+import { decideCertMigration } from "./cert/migration.ts";
 import { ALL_TARGETS } from "./targets/index.ts";
 import { detectAgent } from "./detection/index.ts";
 import type { AgentId, DetectionResult, MitmTarget } from "./types.ts";
@@ -419,13 +419,9 @@ export async function getMitmStatus(agentId?: string): Promise<{
     // Ignore
   }
 
-  // Check cert. #14070: resolve the file the active migration decision
-  // actually installs (ca.crt under the root-CA model), not always the
-  // legacy server.crt — otherwise a root-CA install with no leaf ever
-  // generated would wrongly report certExists:false.
+  // Check cert
   const certDir = path.join(resolveMitmDataDir(), "mitm");
-  const rootCaEnabledForStatus = process.env.MITM_ROOT_CA_ENABLED === "true";
-  const certExists = fs.existsSync(resolveActiveCertPath(certDir, rootCaEnabledForStatus).certPath);
+  const certExists = fs.existsSync(path.join(certDir, "server.crt"));
 
   return {
     running,
@@ -520,7 +516,7 @@ async function startMitmInternal(
   //    `tproxy/dynamicCert.ts`).
   const certDir = resolveMitmCertDir();
   const rootCaEnabled = process.env.MITM_ROOT_CA_ENABLED === "true";
-  const { mode: migrationDecision } = resolveActiveCertPath(certDir, rootCaEnabled);
+  const migrationDecision = decideCertMigration(certDir, rootCaEnabled);
   let certPath: string;
   if (migrationDecision === "use-legacy-leaf") {
     certPath = path.join(resolveMitmDataDir(), "mitm", "server.crt");

tests/unit/agent-bridge-cert-trust-mismatch.test.ts

@@ -1,129 +0,0 @@
-// Regression test for issue #14070 — "[Windows] AgentBridge reports
-// certificate untrusted and DNS off despite manual configuration".
-//
-// Root cause: the AgentBridge dashboard's cert-trust check
-// (/api/tools/agent-bridge/state, /diagnose, and the "trust-cert" manual
-// action) always read the LEGACY leaf server.crt from disk and computed its
-// OS-trust-store fingerprint from that file — regardless of which cert model
-// (cert/migration.ts::decideCertMigration) is actually active for this run.
-// startMitmInternal() (src/mitm/manager.ts) picks the correct model per-run
-// and, under "use-root-ca", installs/trusts ca.crt instead — a DIFFERENT
-// keypair with a DIFFERENT fingerprint.
-//
-// Fix: src/mitm/cert/activeCert.ts::resolveActiveCertPath() is the single
-// source of truth for "which cert file is active for this run", reused by
-// manager.ts and by all three AgentBridge route handlers.
-import { test } from "node:test";
-import assert from "node:assert/strict";
-import fs from "node:fs";
-import path from "node:path";
-import os from "node:os";
-
-import { decideCertMigration } from "../../src/mitm/cert/migration.ts";
-import { loadOrCreateMitmCa } from "../../src/mitm/cert/rootCa.ts";
-import { certutilThumbprint } from "../../src/mitm/cert/install.ts";
-import { generateCert } from "../../src/mitm/cert/generate.ts";
-import { resolveActiveCertPath } from "../../src/mitm/cert/activeCert.ts";
-
-function tmpDataDir(): string {
-  return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-agentbridge-"));
-}
-
-test("AgentBridge cert-trust check now targets the cert file the active migration decision actually installs (root-CA model)", async () => {
-  const dataDir = tmpDataDir();
-  const previousDataDir = process.env.DATA_DIR;
-  process.env.DATA_DIR = dataDir;
-
-  try {
-    const certDir = path.join(dataDir, "mitm");
-    fs.mkdirSync(certDir, { recursive: true });
-
-    // Recreate the reporter's on-disk state: a legacy leaf from an earlier
-    // run, PLUS a persisted CA pair — the combination that activates the
-    // root-CA model this run (decideCertMigration()).
-    await generateCert();
-    const legacyLeafPath = path.join(certDir, "server.crt");
-    assert.equal(fs.existsSync(legacyLeafPath), true, "server.crt must exist for this scenario");
-
-    const ca = await loadOrCreateMitmCa(certDir);
-    assert.equal(fs.existsSync(ca.certPath), true, "ca.crt must exist for this scenario");
-
-    const migrationDecision = decideCertMigration(certDir, false);
-    assert.equal(
-      migrationDecision,
-      "use-root-ca",
-      "with both a legacy leaf and a CA pair on disk, manager.ts activates the root-CA model this run"
-    );
-    const actuallyInstalledCertPath = ca.certPath;
-
-    // This is what the dashboard's cert-trust check now resolves to.
-    const { certPath: dashboardCheckedCertPath, mode } = resolveActiveCertPath(certDir, false);
-
-    assert.equal(mode, "use-root-ca");
-    assert.equal(
-      dashboardCheckedCertPath,
-      actuallyInstalledCertPath,
-      "FIX: the dashboard's cert-trust check must target the same file (ca.crt) manager.ts actually installed/trusted for this run's active cert model"
-    );
-
-    const dashboardCheckedThumbprint = certutilThumbprint(dashboardCheckedCertPath);
-    const actuallyTrustedThumbprint = certutilThumbprint(actuallyInstalledCertPath);
-    assert.equal(
-      dashboardCheckedThumbprint,
-      actuallyTrustedThumbprint,
-      "FIX: the fingerprint the dashboard looks up in the OS store must be the fingerprint of the cert that was actually installed there"
-    );
-  } finally {
-    if (previousDataDir === undefined) delete process.env.DATA_DIR;
-    else process.env.DATA_DIR = previousDataDir;
-    fs.rmSync(dataDir, { recursive: true, force: true });
-  }
-});
-
-test("AgentBridge cert-trust check still targets server.crt for a legacy-only install (no CA pair, no behavior change)", async () => {
-  const dataDir = tmpDataDir();
-  const previousDataDir = process.env.DATA_DIR;
-  process.env.DATA_DIR = dataDir;
-
-  try {
-    const certDir = path.join(dataDir, "mitm");
-    fs.mkdirSync(certDir, { recursive: true });
-
-    await generateCert();
-    const legacyLeafPath = path.join(certDir, "server.crt");
-    assert.equal(fs.existsSync(legacyLeafPath), true, "server.crt must exist for this scenario");
-    assert.equal(
-      fs.existsSync(path.join(certDir, "ca.crt")),
-      false,
-      "no CA pair must exist for this scenario"
-    );
-
-    const { certPath, mode } = resolveActiveCertPath(certDir, false);
-
-    assert.equal(mode, "use-legacy-leaf");
-    assert.equal(
-      certPath,
-      legacyLeafPath,
-      "a pre-existing legacy-only install must keep resolving to server.crt — no silent upgrade"
-    );
-  } finally {
-    if (previousDataDir === undefined) delete process.env.DATA_DIR;
-    else process.env.DATA_DIR = previousDataDir;
-    fs.rmSync(dataDir, { recursive: true, force: true });
-  }
-});
-
-test("resolveActiveCertPath: fresh install with the flag on resolves to ca.crt (matches decideCertMigration)", () => {
-  const dataDir = tmpDataDir();
-  try {
-    const certDir = path.join(dataDir, "mitm");
-    fs.mkdirSync(certDir, { recursive: true });
-
-    const { certPath, mode } = resolveActiveCertPath(certDir, true);
-
-    assert.equal(mode, "use-root-ca");
-    assert.equal(certPath, path.join(certDir, "ca.crt"));
-  } finally {
-    fs.rmSync(dataDir, { recursive: true, force: true });
-  }
-});

tests/unit/check-public-creds.test.ts

@@ -68,10 +68,10 @@ test("allowlist freezes a literal by file:line:value key", () => {
 });
 
 test("allowlist preserves the local ZCode handshake client ID without weakening credential detection", () => {
-  // 312 newlines puts the statement on line 313, which is where it lives in
+  // 335 newlines puts the statement on line 336, which is where it lives in
   // zcodeProtocol.ts today. The allowlist key carries the line number, so this
-  // literal has to be kept in step with the source (it moved 302 -> 313).
-  const src = `${"\n".repeat(312)}clientId: \`omniroute-\${process.pid}\`,`;
+  // literal has to be kept in step with the source (it moved 302 -> 313 -> 336).
+  const src = `${"\n".repeat(335)}clientId: \`omniroute-\${process.pid}\`,`;
   assert.deepEqual(
     findLiteralCreds(src, KNOWN_LITERAL_CREDS, "open-sse/executors/zcodeProtocol.ts"),
     []

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);
+});