Merge pull request #1045 from JoJohanse/fix/windows-spawn-cmd-shims

fix(compat): resolve bare commands via PATH/PATHEXT on Windows; spawn .cmd shims through cmd.exe

scripts/e2e-windows-spawn.ts

@@ -0,0 +1,67 @@
+/**
+ * Manual end-to-end check for the Windows crossSpawn fix.
+ *
+ * Reproduces the auto-updater failure: `bun` on PATH only as npm `.cmd`
+ * shims (no real bun.exe in any PATH directory). Temporarily hides any
+ * bun.exe in the npm shim directory, then runs crossSpawn(['bun', ...]).
+ *
+ * Run with: bun scripts/e2e-windows-spawn.ts
+ * Expected on Windows: exit 0 and a printed bun version.
+ * Before the fix: spawn error ENOENT for 'bun'.
+ */
+import { renameSync } from 'node:fs';
+import { join } from 'node:path';
+import { crossSpawn } from '../src/utils/compat';
+
+function main(): void {
+  if (process.platform !== 'win32') {
+    console.log('skip: not win32');
+    return;
+  }
+  const shimDir = join(process.env.APPDATA ?? '', 'npm');
+  const realExe = join(shimDir, 'bun.exe');
+  const hidden = `${realExe}.e2e-hidden`;
+  let restored = false;
+  const restore = (): void => {
+    if (restored) return;
+    try {
+      renameSync(hidden, realExe);
+    } catch {
+      /* nothing to restore */
+    }
+    restored = true;
+  };
+
+  try {
+    renameSync(realExe, hidden);
+    console.log('hid bun.exe; PATH now exposes bun.cmd shims only');
+  } catch {
+    console.log('no bun.exe in shim dir; nothing to hide');
+  }
+
+  const proc = crossSpawn(['bun', '--version'], {
+    stdout: 'pipe',
+    stderr: 'pipe',
+  });
+  proc.exited
+    .then(async (code) => {
+      if (code === 0) {
+        console.log(
+          `bun resolved via shim, version=${(await proc.stdout()).trim()}`,
+        );
+      } else {
+        console.log(
+          `bun exited ${code}: ${(await proc.stderr()).trim().slice(0, 200)}`,
+        );
+      }
+      restore();
+      process.exit(code === 0 ? 0 : 1);
+    })
+    .catch((err: Error) => {
+      console.log(`spawn failed: ${err.message}`);
+      restore();
+      process.exit(1);
+    });
+}
+
+main();

src/hooks/auto-update-checker/index.ts

@@ -388,7 +388,9 @@ export function getAutoUpdateInstallDir(): string {
 
 /**
  * Spawns a background process to run 'bun install'.
- * Includes a 60-second timeout to prevent stalling OpenCode.
+ * Includes a timeout to prevent stalling OpenCode. The install runs in
+ * the background and does not block startup, so the limit is generous:
+ * a cold bun cache on a slow registry link can exceed a minute.
  * @param installDir The directory whose package manager context should be refreshed.
  * @returns True if the installation succeeded within the timeout.
  */
@@ -401,7 +403,7 @@ async function runBunInstallSafe(installDir: string): Promise<boolean> {
     });
 
     const timeoutPromise = new Promise<'timeout'>((resolve) =>
-      setTimeout(() => resolve('timeout'), 60_000),
+      setTimeout(() => resolve('timeout'), 300_000),
     );
     const exitPromise = proc.exited.then(() => 'completed' as const);
     const result = await Promise.race([exitPromise, timeoutPromise]);

src/utils/compat.test.ts

@@ -1,9 +1,13 @@
 import { afterAll, describe, expect, it } from 'bun:test';
-import { mkdirSync, readFileSync, rmSync } from 'node:fs';
+import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
 import { runInNewContext } from 'node:vm';
-import { crossWrite } from './compat';
+import {
+  buildWindowsCommandLine,
+  crossWrite,
+  resolveWindowsCommand,
+} from './compat';
 
 const TEST_DIR = path.join(os.tmpdir(), `compat-test-${process.pid}`);
 
@@ -55,3 +59,186 @@ describe('crossWrite', () => {
     expect(readFileSync(filePath)).toEqual(Buffer.from([0x7e, 0x7f]));
   });
 });
+
+describe('resolveWindowsCommand', () => {
+  const RESOLVE_DIR = path.join(TEST_DIR, 'resolve');
+
+  function fixtureDir(name: string, files: string[]): string {
+    const dir = path.join(RESOLVE_DIR, name);
+    mkdirSync(dir, { recursive: true });
+    for (const file of files) {
+      writeFileSync(path.join(dir, file), '');
+    }
+    return dir;
+  }
+
+  function joinPathEnv(dirs: string[]): string {
+    return dirs.join(path.delimiter);
+  }
+
+  it('prefers .exe over .cmd within the same PATH entry', () => {
+    const dir = fixtureDir('mixed', ['bun.cmd', 'bun.exe']);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      joinPathEnv([dir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(dir, 'bun.exe'));
+    expect(resolved?.viaCmdShell).toBe(false);
+  });
+
+  it('honours PATH order: an earlier .cmd beats a later .exe', () => {
+    // Matches cmd.exe semantics — the first directory containing any
+    // match wins, so PATH order trumps extension priority across dirs.
+    const cmdDir = fixtureDir('shim-only', ['bun.cmd']);
+    const exeDir = fixtureDir('real-only', ['bun.exe']);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      joinPathEnv([cmdDir, exeDir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(cmdDir, 'bun.cmd'));
+    expect(resolved?.viaCmdShell).toBe(true);
+  });
+
+  it('falls back to a .cmd shim when no real executable exists', () => {
+    const dir = fixtureDir('npm-shims', ['bun', 'bun.cmd', 'bun.ps1']);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      joinPathEnv([dir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(dir, 'bun.cmd'));
+    expect(resolved?.viaCmdShell).toBe(true);
+  });
+
+  it('respects a custom PATHEXT order', () => {
+    const dir = fixtureDir('custom-ext', ['bun.exe', 'bun.cmd']);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      joinPathEnv([dir]),
+      '.CMD;.EXE',
+    );
+    expect(resolved?.file).toBe(path.join(dir, 'bun.cmd'));
+    expect(resolved?.viaCmdShell).toBe(true);
+  });
+
+  it('keeps the extension of a command that already carries one', () => {
+    const dir = fixtureDir('explicit-ext', ['bun.exe', 'bun.cmd']);
+    const resolved = resolveWindowsCommand(
+      'bun.exe',
+      joinPathEnv([dir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(dir, 'bun.exe'));
+    expect(resolved?.viaCmdShell).toBe(false);
+  });
+
+  it('matches extensionless commands case-insensitively', () => {
+    const dir = fixtureDir('case-insensitive', ['BUN.EXE']);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      joinPathEnv([dir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(dir, 'BUN.EXE'));
+  });
+
+  it('returns undefined when nothing on PATH matches', () => {
+    const dir = fixtureDir('empty', []);
+    const resolved = resolveWindowsCommand(
+      'definitely-missing-omos-cmd',
+      joinPathEnv([dir]),
+      '.COM;.EXE;.BAT;.CMD',
+    );
+    expect(resolved).toBeUndefined();
+  });
+
+  it('treats an empty PATH component as the current directory', () => {
+    // cmd.exe semantics: a PATH entry that is empty after splitting
+    // points at the cwd, so `dir1;;dir2` searches cwd between them.
+    const marker = `omos-cwd-probe-${process.pid}.cmd`;
+    writeFileSync(marker, '');
+    try {
+      const resolved = resolveWindowsCommand(
+        marker.replace(/\.cmd$/, ''),
+        ['', ''].join(path.delimiter),
+        '.CMD',
+      );
+      expect(resolved?.file).toBe(marker);
+      expect(resolved?.viaCmdShell).toBe(true);
+    } finally {
+      rmSync(marker, { force: true });
+    }
+  });
+
+  it('keeps a quoted PATH component containing separators intact', () => {
+    // Quoted entries may contain ';'; splitting on raw ';' would shred
+    // the directory name and miss the shim cmd.exe finds.
+    const dir = fixtureDir('semi;colon', ['bun.cmd']);
+    const resolved = resolveWindowsCommand('bun', `"${dir}"`, '.CMD');
+    expect(resolved?.file).toBe(path.join(dir, 'bun.cmd'));
+  });
+
+  it('strips surrounding quotes from individual PATH components', () => {
+    const quotedDir = fixtureDir('quoted-entry', ['bun.cmd']);
+    const plainDir = fixtureDir('plain-entry', []);
+    const resolved = resolveWindowsCommand(
+      'bun',
+      `"${quotedDir}"${path.delimiter}${plainDir}`,
+      '.CMD',
+    );
+    expect(resolved?.file).toBe(path.join(quotedDir, 'bun.cmd'));
+  });
+});
+
+describe('buildWindowsCommandLine', () => {
+  it('wraps the whole line in one outer quote pair for cmd /s /c', () => {
+    // cmd's /s handling strips the first and the last quote of the /c
+    // payload; the outer pair absorbs that so per-argument quotes keep
+    // their meaning.
+    expect(buildWindowsCommandLine('bun', ['install'])).toBe('"bun install"');
+  });
+
+  it('quotes arguments containing spaces with Windows escaping', () => {
+    expect(buildWindowsCommandLine('tar', ['-xf', 'C:\\my file.zip'])).toBe(
+      '"tar -xf "C:\\my file.zip""',
+    );
+  });
+
+  it('double-escapes trailing backslashes inside quoted arguments', () => {
+    expect(buildWindowsCommandLine('bun', ['C:\\my dir\\'])).toBe(
+      '"bun "C:\\my dir\\\\""',
+    );
+  });
+
+  it('quotes cmd metacharacters so cmd.exe treats them literally', () => {
+    // Unquoted, `&` would let cmd chain a second command — the argument
+    // must end up inside double quotes on the final command line.
+    expect(buildWindowsCommandLine('bun', ['run', 'a&b'])).toBe(
+      '"bun run "a&b""',
+    );
+    expect(buildWindowsCommandLine('bun', ['run', 'a|b', 'c^d'])).toBe(
+      '"bun run "a|b" "c^d""',
+    );
+  });
+
+  it('rejects percent signs that cmd.exe would expand even quoted', () => {
+    expect(() => buildWindowsCommandLine('bun', ['100%'])).toThrow(/'%'/);
+    expect(() => buildWindowsCommandLine('bun', ['a%PATH%b'])).toThrow();
+  });
+
+  it('rejects double quotes that would toggle the cmd quoted region', () => {
+    // `\"` is not an escape for cmd.exe; a quote followed by a
+    // metacharacter would expose command syntax (verified as injection).
+    expect(() => buildWindowsCommandLine('bun', ['a"&b'])).toThrow(/'"'/);
+    expect(() => buildWindowsCommandLine('bun', ['a"b'])).toThrow();
+    expect(() => buildWindowsCommandLine('bun', ['he said "hi"'])).toThrow();
+  });
+
+  it('rejects control characters that corrupt the cmd line', () => {
+    expect(() => buildWindowsCommandLine('bun', ['a\nb'])).toThrow();
+    expect(() => buildWindowsCommandLine('bun', ['a\rb'])).toThrow();
+    expect(() => buildWindowsCommandLine('bun', ['a\u0000b'])).toThrow();
+  });
+});

src/utils/compat.ts

@@ -1,6 +1,8 @@
-import type { ChildProcess } from 'node:child_process';
+import type { ChildProcess, SpawnOptions } from 'node:child_process';
 import { spawn as nodeSpawn } from 'node:child_process';
+import { existsSync, readdirSync, statSync } from 'node:fs';
 import { writeFile as fsWriteFile } from 'node:fs/promises';
+import * as path from 'node:path';
 
 export interface CrossSpawnResult {
   proc: ChildProcess;
@@ -33,9 +35,210 @@ function collectStream(
     });
 }
 
+const WINDOWS_PATH_EXT_DEFAULT = '.COM;.EXE;.BAT;.CMD';
+const DIRECT_EXECUTION_EXTENSIONS = new Set(['.exe', '.com']);
+
+export interface ResolvedWindowsCommand {
+  /** Absolute path of the executable (or shim) that was found. */
+  file: string;
+  /**
+   * True when `file` is a `.cmd`/`.bat` shim that only cmd.exe can
+   * interpret; the caller must spawn it through cmd.exe with a quoted
+   * command line instead of passing it to spawn() directly.
+   */
+  viaCmdShell: boolean;
+}
+
+function isRegularFile(candidate: string): boolean {
+  try {
+    return existsSync(candidate) && statSync(candidate).isFile();
+  } catch {
+    return false;
+  }
+}
+
+function splitList(value: string, separator: string): string[] {
+  return value
+    .split(separator)
+    .map((entry) => entry.trim())
+    .filter((entry) => entry.length > 0);
+}
+
+/**
+ * Splits a Windows PATH the way cmd.exe reads it: a `;` inside a quoted
+ * component does not separate entries, the surrounding quotes are
+ * stripped, and an empty component stands for the current directory.
+ * Plain `String.split(';')` would shred quoted entries whose directory
+ * names contain `;` and silently drop current-directory entries.
+ */
+function splitWindowsPath(pathEnv: string): string[] {
+  const parts: string[] = [];
+  let current = '';
+  let inQuotes = false;
+  for (const char of pathEnv) {
+    if (char === '"') {
+      inQuotes = !inQuotes;
+      continue;
+    }
+    if (char === path.delimiter && !inQuotes) {
+      parts.push(current);
+      current = '';
+      continue;
+    }
+    current += char;
+  }
+  parts.push(current);
+  return parts.map((part) => (part === '' ? '.' : part));
+}
+
+/**
+ * Resolve a bare command name against PATH and PATHEXT the way cmd.exe
+ * does, so spawn() can launch it on Windows.
+ *
+ * child_process.spawn only starts real executables; it cannot run the
+ * extensionless sh and `.cmd` shims that npm-style installs leave on PATH
+ * (for example `bun` installed via `npm install -g bun` exposes only
+ * `bun.cmd` next to the real `bun.exe` buried in node_modules). A raw
+ * spawn('bun') then fails with ENOENT even though bun runs fine in a
+ * shell, which silently breaks bun-based flows such as the auto-updater.
+ *
+ * Walks PATH entries in order; within each entry, tries PATHEXT
+ * extensions in declared order. PATH entries are split the way cmd.exe
+ * reads them (quoted entries may contain `;`, empty entries mean the
+ * current directory — see splitWindowsPath). The first directory
+ * containing any match wins, and the matched extension decides whether
+ * the file is directly spawnable (`.exe`/`.com`) or must run through
+ * cmd.exe (`.cmd`/`.bat`).
+ */
+export function resolveWindowsCommand(
+  command: string,
+  pathEnv: string = process.env.PATH ?? '',
+  pathExtEnv: string = process.env.PATHEXT ?? WINDOWS_PATH_EXT_DEFAULT,
+): ResolvedWindowsCommand | undefined {
+  const extensions = splitList(pathExtEnv, ';').map((ext) =>
+    ext.startsWith('.') ? ext : `.${ext}`,
+  );
+  const extensionsLower = new Set(extensions.map((ext) => ext.toLowerCase()));
+  const commandExt = path.extname(command);
+  const candidates =
+    commandExt && extensionsLower.has(commandExt.toLowerCase())
+      ? [command]
+      : extensions.map((ext) => `${command}${ext}`);
+  const candidatesLower = candidates.map((candidate) =>
+    candidate.toLowerCase(),
+  );
+
+  for (const dir of splitWindowsPath(pathEnv)) {
+    let entries: string[];
+    try {
+      entries = readdirSync(dir);
+    } catch {
+      continue;
+    }
+    // Directory listing gives us the on-disk casing, so matching stays
+    // case-insensitive even on case-sensitive filesystems.
+    const byLowerName = new Map(
+      entries.map((entry) => [entry.toLowerCase(), entry]),
+    );
+    for (const candidateLower of candidatesLower) {
+      const actualName = byLowerName.get(candidateLower);
+      if (actualName === undefined) continue;
+      const candidatePath = path.join(dir, actualName);
+      if (!isRegularFile(candidatePath)) continue;
+      const resolvedExt = path.extname(actualName).toLowerCase();
+      return {
+        file: candidatePath,
+        viaCmdShell: !DIRECT_EXECUTION_EXTENSIONS.has(resolvedExt),
+      };
+    }
+  }
+  return undefined;
+}
+
+/**
+ * cmd.exe metacharacters neutralised by double-quoting the argument.
+ * Inside double quotes, `& | < > ( ) ^ !` are literal to cmd; unquoted
+ * they split or chain commands (verified on Windows: passing `a&echo x`
+ * as a bare token makes cmd execute the second command). `"` is not
+ * listed — arguments containing it are rejected by
+ * isCmdUnsafeArgument instead.
+ */
+const CMD_METACHARACTERS = /[\s&|<>()^!]/;
+
+/**
+ * Detects characters that cannot be passed through `cmd.exe /c` at
+ * all:
+ *
+ * - `%` expands environment variables even inside double quotes and
+ *   has no escape on the cmd command line.
+ * - `"` toggles cmd's quoted region no matter what precedes it —
+ *   cmd.exe, unlike the MSVCRT argument parser, does not treat `\` as
+ *   an escape. Emitting `\"` therefore closes the quoted region and
+ *   exposes any following metacharacter as command syntax (verified:
+ *   `a"&echo x` injected the second command). Doubling quotes instead
+ *   (`a""&b`) stays safe at the cmd layer but is ambiguous to the
+ *   child: node's argv parser reads `""` as a literal quote while
+ *   bun's splits the argument in two (verified), so no single
+ *   escaping works across targets.
+ * - Control characters corrupt the command line.
+ *
+ * Node.js refuses to spawn `.cmd`/`.bat` files with any arguments at
+ * all for the same class of reasons (EINVAL since CVE-2024-27980
+ * hardening); we keep the provably-safe subset and throw otherwise.
+ */
+function isCmdUnsafeArgument(arg: string): boolean {
+  if (arg.includes('%') || arg.includes('"')) return true;
+  for (let i = 0; i < arg.length; i++) {
+    if (arg.charCodeAt(i) <= 0x1f) return true;
+  }
+  return false;
+}
+
+/**
+ * Quotes one argument for a `cmd.exe /c` command line. Arguments that
+ * contain no metacharacters are passed through untouched — cmd's /s
+ * stripping mangles gratuitously quoted tokens. Quoted arguments have
+ * no `"` left to escape (those throw in isCmdUnsafeArgument); only a
+ * trailing backslash run must be doubled so the closing quote is not
+ * read as an escape by the child's argument parser.
+ *
+ * Throws on arguments that cmd.exe cannot represent faithfully (`%`,
+ * `"`, control characters) so callers fail loudly instead of
+ * executing an altered command line.
+ */
+function escapeWindowsArgument(arg: string): string {
+  if (isCmdUnsafeArgument(arg)) {
+    throw new Error(
+      `cannot pass ${JSON.stringify(arg)} through a .cmd shim: cmd.exe reinterprets '%', '"', and control characters even inside quotes`,
+    );
+  }
+  if (!CMD_METACHARACTERS.test(arg)) {
+    return arg;
+  }
+  const escaped = arg.replace(/(\\*)$/, '$1$1');
+  return `"${escaped}"`;
+}
+
+/**
+ * Builds the full command line handed to `cmd.exe /d /s /c`. The whole
+ * line is wrapped in one outer pair of quotes because cmd's /s
+ * processing strips the first and the last quote of the /c payload:
+ * without the outer pair, a spaced path like `"C:\Program
+ * Files\...\bun.cmd"` loses its quotes and the spawn fails (verified on
+ * Windows). Exported for unit tests only.
+ */
+export function buildWindowsCommandLine(file: string, args: string[]): string {
+  return `"${[file, ...args].map(escapeWindowsArgument).join(' ')}"`;
+}
+
 /**
  * Cross-runtime spawn that works in both Bun and Node.js.
  * API mimics Bun.spawn but uses node:child_process internally.
+ *
+ * On Windows, bare command names are resolved against PATH/PATHEXT first
+ * (see resolveWindowsCommand) so npm-installed CLIs that only expose
+ * `.cmd` shims still run. Non-Windows platforms and explicit paths are
+ * passed through unchanged.
  */
 export function crossSpawn(
   command: string[],
@@ -48,7 +251,23 @@ export function crossSpawn(
   },
 ): CrossSpawnResult {
   const [cmd, ...args] = command;
-  const proc = nodeSpawn(cmd, args, {
+  let file = cmd;
+  const fileArgs = args;
+  let viaCmdShell = false;
+
+  if (
+    process.platform === 'win32' &&
+    !cmd.includes('/') &&
+    !cmd.includes('\\')
+  ) {
+    const resolved = resolveWindowsCommand(cmd);
+    if (resolved) {
+      file = resolved.file;
+      viaCmdShell = resolved.viaCmdShell;
+    }
+  }
+
+  const spawnOptions: SpawnOptions = {
     stdio: [
       options?.stdin ?? 'ignore',
       options?.stdout ?? 'pipe',
@@ -56,7 +275,18 @@ export function crossSpawn(
     ],
     cwd: options?.cwd,
     env: options?.env as NodeJS.ProcessEnv,
-  });
+  };
+
+  const proc: ChildProcess = viaCmdShell
+    ? nodeSpawn(
+        process.env.ComSpec ?? 'cmd.exe',
+        ['/d', '/s', '/c', buildWindowsCommandLine(file, fileArgs)],
+        // The command line is pre-quoted by buildWindowsCommandLine, so
+        // it must reach cmd.exe verbatim — without this flag Node/Bun
+        // re-escape the quotes and cmd strips the backslashes.
+        { ...spawnOptions, windowsVerbatimArguments: true },
+      )
+    : nodeSpawn(file, fileArgs, spawnOptions);
 
   const stdoutCollector = collectStream(proc.stdout);
   const stderrCollector = collectStream(proc.stderr);