fix: route Windows .cmd shims through shell and default windowsHide to true

- Add WINDOWS_CMD_SHIMS allowlist + shouldUseShellForCommand helper so
  pnpm/npm/npx hit a shell on win32 (Node post-CVE-2024-27980 rejects
  direct .cmd/.bat spawn) while native exes like git keep shell:false.
- Apply the helper to packages/server provider runtime, scripts/shared
  process utilities, and publish-cli; drop the dead resolveSpawnCommand
  passthrough.
- Default windowsHide to true everywhere so background child processes
  don't flash a console window.
- Fix CI Windows runtime lane referencing the renamed test file.
- Cover the new behavior with unit tests.

Author: pallyoung

.github/workflows/ci.yml

@@ -82,7 +82,7 @@ jobs:
         run: pnpm --filter @coder-studio/providers exec vitest run src/claude/definition.test.ts src/codex/definition.test.ts
 
       - name: Run targeted Windows server tests
-        run: pnpm --filter @coder-studio/server exec vitest run src/__tests__/provider-runtime/command-check.test.ts src/__tests__/provider-runtime/exec-file.test.ts src/__tests__/provider-runtime/install-manager.test.ts src/__tests__/workspace/runtime-check.test.ts src/git/cli.windows.test.ts src/supervisor/evaluator.windows.test.ts src/__tests__/server-provider-install-wiring.test.ts src/__tests__/session-commands.test.ts src/__tests__/session-integration.test.ts
+        run: pnpm --filter @coder-studio/server exec vitest run src/__tests__/provider-runtime/command-check.test.ts src/__tests__/provider-runtime/command-runner.test.ts src/__tests__/provider-runtime/install-manager.test.ts src/__tests__/workspace/runtime-check.test.ts src/git/cli.windows.test.ts src/supervisor/evaluator.windows.test.ts src/__tests__/server-provider-install-wiring.test.ts src/__tests__/session-commands.test.ts src/__tests__/session-integration.test.ts
 
       - name: Run targeted Windows CLI tests
         run: pnpm --filter @spencer-kit/coder-studio exec vitest run src/browser.test.ts src/bin.test.ts src/pm2-control.test.ts src/server-control.test.ts

packages/server/src/__tests__/provider-runtime/command-runner.test.ts

@@ -1,6 +1,6 @@
 import { Buffer } from "node:buffer";
 import { EventEmitter } from "node:events";
-import { describe, expect, it, vi } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 
 const { spawnMock } = vi.hoisted(() => ({
   spawnMock: vi.fn(),
@@ -29,6 +29,14 @@ function createChildProcessMock() {
 }
 
 describe("runCommandAsString", () => {
+  const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
+
+  afterEach(() => {
+    if (originalPlatform) {
+      Object.defineProperty(process, "platform", originalPlatform);
+    }
+  });
+
   it("normalizes stdout and stderr to strings", async () => {
     spawnMock.mockImplementation(
       (_file: string, _args: string[], _options: { windowsHide?: boolean }) => {
@@ -50,4 +58,53 @@ describe("runCommandAsString", () => {
       windowsHide: true,
     });
   });
+
+  it("defaults windowsHide to true when caller omits the option", async () => {
+    spawnMock.mockImplementation(() => {
+      const child = createChildProcessMock();
+      queueMicrotask(() => child.emit("close", 0));
+      return child;
+    });
+
+    await runCommandAsString("demo", []);
+
+    expect(spawnMock).toHaveBeenCalledWith("demo", [], {
+      shell: false,
+      windowsHide: true,
+    });
+  });
+
+  it("routes Windows .cmd shims through a shell so Node does not reject them", async () => {
+    Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
+    spawnMock.mockImplementation(() => {
+      const child = createChildProcessMock();
+      queueMicrotask(() => child.emit("close", 0));
+      return child;
+    });
+
+    await runCommandAsString("npm", ["install", "-g", "@openai/codex"]);
+
+    expect(spawnMock).toHaveBeenCalledWith(
+      "npm",
+      ["install", "-g", "@openai/codex"],
+      expect.objectContaining({ shell: true, windowsHide: true })
+    );
+  });
+
+  it("keeps native Windows executables off the shell path", async () => {
+    Object.defineProperty(process, "platform", { configurable: true, value: "win32" });
+    spawnMock.mockImplementation(() => {
+      const child = createChildProcessMock();
+      queueMicrotask(() => child.emit("close", 0));
+      return child;
+    });
+
+    await runCommandAsString("git", ["--version"]);
+
+    expect(spawnMock).toHaveBeenCalledWith(
+      "git",
+      ["--version"],
+      expect.objectContaining({ shell: false, windowsHide: true })
+    );
+  });
 });

packages/server/src/__tests__/server-provider-install-wiring.test.ts

@@ -151,6 +151,11 @@ describe("createServer provider install wiring", () => {
     expect(spawnMock).toHaveBeenCalledWith(
       "npm",
       ["install", "-g", "@openai/codex"],
+      expect.objectContaining({ shell: true, windowsHide: true })
+    );
+    expect(spawnMock).toHaveBeenCalledWith(
+      "winget",
+      ["install", "--id", "OpenJS.NodeJS.LTS", "--exact", "--silent"],
       expect.objectContaining({ shell: false, windowsHide: true })
     );
   });

packages/server/src/provider-runtime/command-runner.ts

@@ -13,15 +13,24 @@ export type CommandRunner = (
   options?: CommandRunnerOptions
 ) => Promise<CommandRunnerResult>;
 
+// Windows ships these as .cmd shims that Node refuses to spawn directly post
+// CVE-2024-27980. Routing them through cmd.exe via shell:true is the only
+// approach that survives both ENOENT (bare name) and EINVAL (full .cmd path).
+const WINDOWS_CMD_SHIMS = new Set(["pnpm", "npm", "npx"]);
+
+function shouldUseShellForCommand(file: string, platform: NodeJS.Platform): boolean {
+  return platform === "win32" && WINDOWS_CMD_SHIMS.has(file.toLowerCase());
+}
+
 export async function runCommandAsString(
   file: string,
   args: string[],
   options?: CommandRunnerOptions
 ): Promise<CommandRunnerResult> {
   return new Promise((resolve, reject) => {
     const child = spawn(file, args, {
-      shell: false,
-      windowsHide: options?.windowsHide ?? false,
+      shell: shouldUseShellForCommand(file, process.platform),
+      windowsHide: options?.windowsHide ?? true,
     });
 
     const stdoutChunks: Buffer[] = [];

scripts/publish-cli.ts

@@ -10,7 +10,7 @@ import { readdir, readFile, stat } from "node:fs/promises";
 import { dirname, resolve } from "node:path";
 import { build } from "./build.js";
 import { CLI_DIR, error, info, step, success } from "./shared/index.js";
-import { isDirectExecution, resolveSpawnCommand } from "./shared/process.js";
+import { isDirectExecution, shouldUseShellForCommand } from "./shared/process.js";
 
 export interface PublishCliOptions {
   access: string;
@@ -262,11 +262,12 @@ export async function execCommand(
 ): Promise<ExecResult> {
   return new Promise((resolvePromise, reject) => {
     const stdio = options.stdio ?? "inherit";
-    const child = spawn(resolveSpawnCommand(command), args, {
+    const child = spawn(command, args, {
       cwd: options.cwd,
       env: process.env,
-      shell: false,
+      shell: shouldUseShellForCommand(command),
       stdio: stdio === "pipe" ? ["ignore", "pipe", "pipe"] : "inherit",
+      windowsHide: true,
     });
 
     let stdout = "";

scripts/shared/process.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { isDirectExecution, resolveSpawnCommand, shouldUseShellForCommand } from "./process.js";
+import { isDirectExecution, shouldUseShellForCommand } from "./process.js";
 
 describe("isDirectExecution", () => {
   it("matches direct execution for POSIX script paths", () => {
@@ -17,20 +17,6 @@ describe("isDirectExecution", () => {
   });
 });
 
-describe("resolveSpawnCommand", () => {
-  it("keeps pnpm unresolved on Windows so cmd can resolve the shim", () => {
-    expect(resolveSpawnCommand("pnpm", "win32")).toBe("pnpm");
-  });
-
-  it("does not rewrite native Windows executables like git", () => {
-    expect(resolveSpawnCommand("git", "win32")).toBe("git");
-  });
-
-  it("leaves commands unchanged on POSIX platforms", () => {
-    expect(resolveSpawnCommand("pnpm", "linux")).toBe("pnpm");
-  });
-});
-
 describe("shouldUseShellForCommand", () => {
   it("uses a shell for pnpm on Windows because pnpm.cmd is not directly executable", () => {
     expect(shouldUseShellForCommand("pnpm", "win32")).toBe(true);

scripts/shared/process.ts

@@ -79,13 +79,6 @@ export function isDirectExecution(moduleUrl: string, argv1: string | undefined =
   return modulePath === normalizeArgvPath(argv1);
 }
 
-export function resolveSpawnCommand(
-  command: string,
-  _platform: NodeJS.Platform = process.platform
-): string {
-  return command;
-}
-
 export function shouldUseShellForCommand(
   command: string,
   platform: NodeJS.Platform = process.platform
@@ -103,6 +96,7 @@ function createSpawnOptions({
     env: options.env ?? process.env,
     stdio: options.stdio ?? "inherit",
     shell: shouldUseShellForCommand(command, platform),
+    windowsHide: true,
   };
 }
 
@@ -115,11 +109,7 @@ export function run(
   options: ProcessOptions = {}
 ): Promise<void> {
   return new Promise((resolve, reject) => {
-    const child = spawn(
-      resolveSpawnCommand(command),
-      args,
-      createSpawnOptions({ command, options })
-    );
+    const child = spawn(command, args, createSpawnOptions({ command, options }));
 
     child.on("close", (code) => {
       if (code === 0) {
@@ -143,7 +133,7 @@ export function runBackground(
   args: string[] = [],
   options: ProcessOptions = {}
 ): ChildProcess {
-  const child = spawn(resolveSpawnCommand(command), args, createSpawnOptions({ command, options }));
+  const child = spawn(command, args, createSpawnOptions({ command, options }));
 
   return child;
 }