fix(windows): route .cmd shims through cmd.exe in dev and perf runners

CHANGELOG.md

@@ -12,12 +12,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Security
 
 - Claude plugin marketplace/install/update/uninstall in `bin/cli.js` now spawn via `execFileSync` with an argv array, so no plugin ID reaches a shell (#388).
+- The cmd.exe argument guard in `bin/cli.js` ended in `$`, which in JavaScript also matches before a trailing newline - so an id like `core@agentsys\n` passed as safe, and whatever followed the newline was dropped by cmd.exe rather than checked. It now asserts end of input. `planShimSpawn` refuses CR and LF in an argument for the same reason.
 
 ### Fixed
 
 - Windows: the Claude Code executable is now resolved from `where.exe` instead of assuming the npm shim. `execFileSync` does not apply PATHEXT, and the previous hardcoded `claude.cmd` did not exist for native-installer users who have `claude.exe` - those calls raised `ENOENT`, the error was swallowed, and the CLI reported success while installing nothing. A directly launchable `claude.exe` is preferred over a batch shim regardless of PATH order.
 - Windows: an npm-global `claude.cmd` shim is launched through `cmd.exe` rather than handed to `execFileSync`, which fails with `EINVAL` - Node's `src` has disallowed direct `.bat`/`.cmd` spawning since the CVE-2024-27980 fix in 18.20.2 / 20.12.2 / 21.7.3. Arguments are rejected unless they are free of whitespace and shell metacharacters, so the extra hop cannot reintroduce the injection surface #388 closed.
 - `agentsys install` no longer reports `[OK] Installed ... successfully` when Claude Code rejected a plugin. It names the plugins that failed, with the errno when the shim could not be spawned at all, and how to retry. The same now applies when the `claude` CLI is not on PATH at all (`~/.claude` alone was enough to mark the platform as installed) and when a dependency id would be rejected. Such a plugin is also no longer recorded against `claude` in `installed.json` - `agentsys list` and `agentsys remove` read those platforms back - though a registration recorded by an earlier successful install is preserved, since a failed re-install is not evidence the first one never landed. The process now exits non-zero, matching the `--tool` path so `agentsys install x && ...` stops.
+- Windows: `agentsys-dev test`, `runBenchmark`, and `runProfiling` all handed a `.cmd` shim straight to `spawnSync`/`execFileSync`, which fails with `EINVAL` for the same reason the Claude shim did - `resolveExecutableForPlatform` turns `npm` into `npm.cmd` and `node_modules/.bin/vitest` into `vitest.cmd`, so every benchmark, profile, and dev test run died on Windows. These now route through `cmd.exe` via a shared `planShimSpawn` in `lib/utils/command-parser.js`, which `bin/cli.js` uses as well so the two mechanisms cannot drift apart. Unlike the Claude call sites, these carry user-written commands, so arguments are quoted for `cmd.exe` rather than rejected for containing whitespace or metacharacters; a literal `"`, `%`, CR or LF is still refused, in the executable as well as the arguments, since none survives `cmd.exe` intact. The rewrite is win32-only: a repo-local `build.cmd` on Linux is spawnable as it stands, and routing it through a `cmd.exe` that is not there would only turn a working command into `ENOENT`.
 - `lib/utils/command-parser.js` used a raw null byte where `'\0'` was intended. Git and grep classified the file as binary, so changes to it could not be reviewed as a diff. Also normalized to LF, the only CRLF-encoded source file in the repo.
 
 ## [6.0.1] - 2026-07-22

__tests__/cli-args.test.js

@@ -229,34 +229,46 @@ describe('claudeSpawnPlan', () => {
   const args = ['plugin', 'install', 'agentsys-core@agentsys'];
 
   test('spawns a posix or native executable directly', () => {
-    expect(claudeSpawnPlan('claude', args)).toEqual({ file: 'claude', args });
-    expect(claudeSpawnPlan('C:\\bin\\claude.exe', args)).toEqual({ file: 'C:\\bin\\claude.exe', args });
+    expect(claudeSpawnPlan('claude', args)).toEqual({ file: 'claude', args, verbatim: false });
+    expect(claudeSpawnPlan('C:\\bin\\claude.exe', args)).toEqual({ file: 'C:\\bin\\claude.exe', args, verbatim: false });
   });
 
   test('routes a batch shim through cmd.exe, which execFileSync cannot spawn', () => {
     // Node's src disallows direct .bat/.cmd spawning since the CVE-2024-27980
     // fix, so a shim handed to execFileSync fails with EINVAL.
     const shim = 'C:\\npm\\claude.cmd';
-    expect(claudeSpawnPlan(shim, args)).toEqual({
+    expect(claudeSpawnPlan(shim, args, undefined, 'win32')).toEqual({
       file: 'cmd.exe',
-      args: ['/d', '/s', '/c', '""C:\\npm\\claude.cmd" plugin install agentsys-core@agentsys"'],
+      args: ['/d', '/s', '/c', '""C:\\npm\\claude.cmd" "plugin" "install" "agentsys-core@agentsys""'],
       verbatim: true
     });
-    expect(claudeSpawnPlan('C:\\npm\\claude.bat', args).file).toBe('cmd.exe');
+    expect(claudeSpawnPlan('C:\\npm\\claude.bat', args, undefined, 'win32').file).toBe('cmd.exe');
+  });
+
+  test('spawns a .cmd directly off Windows, where cmd.exe does not exist', () => {
+    // The extension alone is not evidence a file needs a shell: routing a
+    // repo-local build.cmd through a missing cmd.exe would only cause ENOENT.
+    expect(claudeSpawnPlan('./claude.cmd', args, undefined, 'linux'))
+      .toEqual({ file: './claude.cmd', args, verbatim: false });
   });
 
   test('honours COMSPEC when routing through a shell', () => {
-    expect(claudeSpawnPlan('claude.cmd', args, 'C:\\Windows\\system32\\cmd.exe').file)
+    expect(claudeSpawnPlan('claude.cmd', args, 'C:\\Windows\\system32\\cmd.exe', 'win32').file)
       .toBe('C:\\Windows\\system32\\cmd.exe');
   });
 
   test('refuses arguments cmd.exe would reparse', () => {
     // cmd.exe re-splits its command line, so an unquoted metacharacter would be
     // a command injection - the exact hazard behind CVE-2024-27980.
-    expect(() => claudeSpawnPlan('claude.cmd', ['plugin', 'install', 'x&calc'])).toThrow(/Refusing to pass/);
-    expect(() => claudeSpawnPlan('claude.cmd', ['plugin', 'install', 'a|b'])).toThrow(/Refusing to pass/);
-    expect(() => claudeSpawnPlan('claude.cmd', ['plugin', 'install', 'a b'])).toThrow(/Refusing to pass/);
-    expect(() => claudeSpawnPlan('claude.cmd', ['plugin', 'install', 'a"b'])).toThrow(/Refusing to pass/);
+    const reject = arg => () => claudeSpawnPlan('claude.cmd', ['plugin', 'install', arg], undefined, 'win32');
+    expect(reject('x&calc')).toThrow(/Refusing to pass/);
+    expect(reject('a|b')).toThrow(/Refusing to pass/);
+    expect(reject('a b')).toThrow(/Refusing to pass/);
+    expect(reject('a"b')).toThrow(/Refusing to pass/);
+    // JavaScript's $ also matches before a trailing newline, so the guard has to
+    // assert end of input or 'id\n' - and 'id\n&calc' - would pass as safe.
+    expect(reject('agentsys-core@agentsys\n')).toThrow(/Refusing to pass/);
+    expect(reject('a\n&calc')).toThrow(/Refusing to pass/);
   });
 
   test('passes the same arguments through unchecked when no shell is involved', () => {

__tests__/command-parser.test.js

@@ -1,6 +1,6 @@
 const fs = require('fs');
 const path = require('path');
-const { parseCommand, resolveExecutableForPlatform } = require('../lib/utils/command-parser');
+const { parseCommand, resolveExecutableForPlatform, planShimSpawn, shimSpawnOptions } = require('../lib/utils/command-parser');
 
 describe('command parser', () => {
   test('parses simple command into executable and args', () => {
@@ -70,6 +70,101 @@ describe('resolveExecutableForPlatform', () => {
   });
 });
 
+describe('planShimSpawn', () => {
+  // The .bat/.cmd rewrite is win32-only, so every shim case names the platform
+  // rather than depending on the host running the suite.
+  const plan = (executable, args, options = {}) =>
+    planShimSpawn(executable, args, { platform: 'win32', ...options });
+
+  test('leaves a directly spawnable executable alone', () => {
+    const args = ['run', 'bench'];
+    expect(plan('npm', args)).toEqual({ file: 'npm', args, verbatim: false });
+    expect(plan('node.exe', args)).toEqual({ file: 'node.exe', args, verbatim: false });
+    expect(plan('/usr/bin/node', args)).toEqual({ file: '/usr/bin/node', args, verbatim: false });
+  });
+
+  test('leaves a .cmd alone off Windows, where cmd.exe does not exist', () => {
+    // A repo-local build.cmd on Linux spawns as it stands; rewriting it to a
+    // cmd.exe that is not there would turn a working command into ENOENT.
+    const args = ['run', 'bench'];
+    for (const platform of ['linux', 'darwin']) {
+      expect(planShimSpawn('./build.cmd', args, { platform }))
+        .toEqual({ file: './build.cmd', args, verbatim: false });
+    }
+  });
+
+  test('routes a batch shim through cmd.exe, which spawn cannot launch', () => {
+    // Node's src disallows direct .bat/.cmd spawning since the CVE-2024-27980
+    // fix, so spawnSync/execFileSync fail with EINVAL on a shim.
+    expect(plan('npm.cmd', ['run', 'bench'])).toEqual({
+      file: 'cmd.exe',
+      args: ['/d', '/s', '/c', '""npm.cmd" "run" "bench""'],
+      verbatim: true
+    });
+    expect(plan('yarn.bat', []).args).toEqual(['/d', '/s', '/c', '""yarn.bat""']);
+  });
+
+  test('quotes arguments so cmd.exe cannot reinterpret them', () => {
+    // Inside double quotes cmd.exe leaves these alone, so a benchmark command
+    // carrying them runs instead of being split into extra commands.
+    const [, , , payload] = plan('npm.cmd', ['run', 'a && calc', 'x|y', 'a>b']).args;
+    expect(payload).toBe('""npm.cmd" "run" "a && calc" "x|y" "a>b""');
+  });
+
+  test('preserves an empty argument', () => {
+    const [, , , payload] = plan('npm.cmd', ['run', '']).args;
+    expect(payload).toBe('""npm.cmd" "run" """');
+  });
+
+  test('doubles trailing backslashes so the closing quote survives', () => {
+    // "C:\dir\" would read as an escaped quote when the child parses argv back.
+    const [, , , payload] = plan('npm.cmd', ['--cwd', 'C:\\dir\\']).args;
+    expect(payload).toBe('""npm.cmd" "--cwd" "C:\\dir\\\\""');
+  });
+
+  test('doubles a long run of backslashes without backtracking', () => {
+    const [, , , payload] = plan('npm.cmd', ['\\'.repeat(5000)]).args;
+    expect(payload).toBe(`""npm.cmd" "${'\\'.repeat(10000)}""`);
+  });
+
+  test('refuses arguments cmd.exe cannot carry faithfully', () => {
+    // % is expanded even inside quotes, and a literal " ends the quoting.
+    expect(() => plan('npm.cmd', ['run', '%PATH%'])).toThrow(/not representable/);
+    expect(() => plan('npm.cmd', ['run', 'say "hi"'])).toThrow(/not representable/);
+    // A newline ends the command line, so what follows is dropped or run alone.
+    expect(() => plan('npm.cmd', ['run', 'bench\ncalc'])).toThrow(/not representable/);
+    expect(() => plan('npm.cmd', ['run', 'bench\r'])).toThrow(/not representable/);
+    expect(() => plan('npm.cmd', ['run', 'a\0b'])).toThrow(/null byte/);
+    expect(() => plan('npm.cmd', ['run', 42])).toThrow(/must be a string/);
+  });
+
+  test('refuses a shim path cmd.exe would rewrite before resolving it', () => {
+    // The executable lands on the same command line as the arguments, and
+    // cmd.exe expands %TEMP% there too - the path it opens is then not the one
+    // the caller named.
+    expect(() => plan('C:\\build%TEMP%\\.bin\\vitest.cmd', ['run']))
+      .toThrow(/executable path.*not representable/s);
+  });
+
+  test('leaves those arguments alone when no shell is involved', () => {
+    // Nothing reparses an execFileSync argv, so the restriction is shim-only.
+    expect(plan('npm', ['run', '%PATH%', 'say "hi"']).args)
+      .toEqual(['run', '%PATH%', 'say "hi"']);
+  });
+
+  test('honours an explicit comspec', () => {
+    expect(plan('npm.cmd', [], { comspec: 'C:\\Windows\\system32\\cmd.exe' }).file)
+      .toBe('C:\\Windows\\system32\\cmd.exe');
+  });
+
+  test('shimSpawnOptions adds windowsVerbatimArguments only for a shim', () => {
+    expect(shimSpawnOptions(plan('npm.cmd', ['run']), { cwd: '/tmp' }))
+      .toEqual({ cwd: '/tmp', windowsVerbatimArguments: true });
+    expect(shimSpawnOptions(plan('npm', ['run']), { cwd: '/tmp' }))
+      .toEqual({ cwd: '/tmp' });
+  });
+});
+
 describe('command-parser source hygiene', () => {
   test('rejects a null byte in an argument', () => {
     expect(() => parseCommand('node --eval a\0b')).toThrow(/null byte/);

__tests__/dev-cli.test.js

@@ -245,10 +245,17 @@ describe('dev-cli module', () => {
 
   test('test command resolves npm executable without shell interpolation', () => {
     expect(cliSource).toContain('resolveExecutableForPlatform');
-    expect(cliSource).toContain('spawnSync(npmExecutable');
     expect(cliSource).toContain('shell: false');
   });
 
+  test('test command routes the npm.cmd shim through a spawn plan', () => {
+    // npm resolves to npm.cmd on Windows, and a direct spawn of it fails with
+    // EINVAL since the CVE-2024-27980 fix, so the plan must not be bypassed.
+    expect(cliSource).toContain('planShimSpawn(npmExecutable');
+    expect(cliSource).toContain('spawnSync(plan.file, plan.args, shimSpawnOptions(plan');
+    expect(cliSource).not.toContain('spawnSync(npmExecutable');
+  });
+
   test('exports parseArgs, COMMANDS, VALIDATE_SUBCOMMANDS, NEW_SUBCOMMANDS, route', () => {
     expect(cliSource).toContain('module.exports');
     expect(typeof parseArgs).toBe('function');

__tests__/perf-benchmark-runner.test.js

@@ -109,6 +109,45 @@ describe('runBenchmark', () => {
     }));
   });
 
+  it('runs a batch shim through cmd.exe instead of spawning it directly', () => {
+    // Node fails with EINVAL on a direct .cmd spawn since the CVE-2024-27980
+    // fix, so an npm-based benchmark command needs the cmd.exe hop. The hop is
+    // win32-only, so the platform is faked rather than skipping off Windows.
+    execFileSync.mockImplementation(() => 'output');
+    const platform = process.platform;
+    Object.defineProperty(process, 'platform', { value: 'win32', configurable: true });
+
+    try {
+      runBenchmark('npm.cmd run bench', { allowShort: true });
+    } finally {
+      Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+    }
+
+    expect(execFileSync).toHaveBeenCalledWith(
+      'cmd.exe',
+      ['/d', '/s', '/c', '""npm.cmd" "run" "bench""'],
+      expect.objectContaining({ windowsVerbatimArguments: true })
+    );
+  });
+
+  it('spawns a .cmd directly off Windows, where cmd.exe does not exist', () => {
+    execFileSync.mockImplementation(() => 'output');
+    const platform = process.platform;
+    Object.defineProperty(process, 'platform', { value: 'linux', configurable: true });
+
+    try {
+      runBenchmark('./build.cmd bench', { allowShort: true });
+    } finally {
+      Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+    }
+
+    expect(execFileSync).toHaveBeenCalledWith(
+      './build.cmd',
+      ['bench'],
+      expect.not.objectContaining({ windowsVerbatimArguments: true })
+    );
+  });
+
   it('sets PERF_RUN_DURATION env variable by default', () => {
     execFileSync.mockImplementation(() => 'output');
 

__tests__/perf-profiling-runner.test.js

@@ -19,6 +19,41 @@ describe('perf profiling runner', () => {
   });
 });
 
+it('runs a batch shim profiler command through cmd.exe', () => {
+  // A direct .cmd spawn fails with EINVAL since the CVE-2024-27980 fix.
+  jest.resetModules();
+  const execFileSync = jest.fn();
+  jest.doMock('child_process', () => ({ execFileSync }));
+  jest.doMock('../lib/perf/profilers', () => ({
+    selectProfiler: () => ({
+      id: 'fake',
+      buildCommand: () => 'npx.cmd clinic doctor',
+      parseOutput: () => ({ tool: 'fake', hotspots: [], artifacts: [] })
+    })
+  }));
+
+  const runner = require('../lib/perf/profiling-runner');
+  // The cmd.exe hop is win32-only, so fake the platform instead of skipping.
+  const platform = process.platform;
+  Object.defineProperty(process, 'platform', { value: 'win32', configurable: true });
+
+  try {
+    expect(runner.runProfiling().ok).toBe(true);
+  } finally {
+    Object.defineProperty(process, 'platform', { value: platform, configurable: true });
+  }
+
+  expect(execFileSync).toHaveBeenCalledWith(
+    'cmd.exe',
+    ['/d', '/s', '/c', '""npx.cmd" "clinic" "doctor""'],
+    expect.objectContaining({ windowsVerbatimArguments: true })
+  );
+
+  jest.dontMock('child_process');
+  jest.dontMock('../lib/perf/profilers');
+  jest.resetModules();
+});
+
 it('does not enforce timeout when timeoutMs is not provided', () => {
   jest.resetModules();
   const execFileSync = jest.fn();

bin/cli.js

@@ -20,7 +20,7 @@ const VERSION = require('../package.json').version;
 const PACKAGE_DIR = path.join(__dirname, '..');
 const discovery = require('../lib/discovery');
 const transforms = require('../lib/adapter-transforms');
-const { resolveExecutableForPlatform } = require('../lib/utils/command-parser');
+const { resolveExecutableForPlatform, planShimSpawn, shimSpawnOptions } = require('../lib/utils/command-parser');
 
 // Valid tool names
 const VALID_TOOLS = ['claude', 'opencode', 'codex', 'cursor', 'kiro'];
@@ -72,7 +72,9 @@ const WINDOWS_DIRECT_EXEC = /\.(exe|com)$/i;
 // to execFileSync fails with EINVAL rather than running it.
 const WINDOWS_BATCH_SHIM = /\.(cmd|bat)$/i;
 // Arguments safe to hand to cmd.exe: no whitespace, no shell metacharacters.
-const CMD_SAFE_ARG = /^[A-Za-z0-9@._:\\/+-]+$/;
+// Ends with (?![\s\S]) rather than $, which in JavaScript also matches before a
+// trailing newline - so 'plugin\n' would otherwise pass as safe.
+const CMD_SAFE_ARG = /^[A-Za-z0-9@._:\\/+-]+(?![\s\S])/;
 
 /**
  * Pick the Claude Code executable from a `where.exe claude` result.
@@ -100,23 +102,21 @@ function pickClaudeExecutable(platform, whereOutput) {
 /**
  * Build the file and argv for one Claude Code invocation.
  *
- * A batch shim cannot be handed to execFileSync at all, so it is launched
- * through cmd.exe. cmd.exe re-parses its command line, so every argument is
- * checked first: callers only ever pass literal subcommands and validated
- * `<plugin>@<marketplace>` ids, so anything else is a bug rather than a string
- * to escape. The quoting mirrors how Node wraps a shell command - /s strips the
- * outer quote pair, leaving the quoted shim path as the first token.
+ * planShimSpawn does the cmd.exe routing a batch shim needs, on win32 only.
+ * Every argument is checked against a stricter rule than that quoting requires:
+ * callers only ever pass literal subcommands and validated
+ * `<plugin>@<marketplace>` ids, so anything carrying whitespace or a shell
+ * metacharacter is a bug rather than a string to escape. The platform parameter
+ * exists so the win32 path stays testable off Windows.
  */
-function claudeSpawnPlan(executable, args, comspec) {
-  if (!WINDOWS_BATCH_SHIM.test(executable)) {
-    return { file: executable, args };
-  }
-  const unsafe = args.find(arg => !CMD_SAFE_ARG.test(arg));
-  if (unsafe !== undefined) {
-    throw new Error(`Refusing to pass ${JSON.stringify(unsafe)} to cmd.exe`);
+function claudeSpawnPlan(executable, args, comspec, platform = process.platform) {
+  if (platform === 'win32' && WINDOWS_BATCH_SHIM.test(executable)) {
+    const unsafe = args.find(arg => !CMD_SAFE_ARG.test(arg));
+    if (unsafe !== undefined) {
+      throw new Error(`Refusing to pass ${JSON.stringify(unsafe)} to cmd.exe`);
+    }
   }
-  const command = [`"${executable}"`, ...args].join(' ');
-  return { file: comspec || 'cmd.exe', args: ['/d', '/s', '/c', `"${command}"`], verbatim: true };
+  return planShimSpawn(executable, args, { comspec, platform });
 }
 
 let claudeBinCache;
@@ -144,7 +144,7 @@ function claudeExecutable() {
  */
 function claudeSpawn(args, options) {
   const plan = claudeSpawnPlan(claudeExecutable(), args, process.env.comspec);
-  return execFileSync(plan.file, plan.args, plan.verbatim ? { ...options, windowsVerbatimArguments: true } : options);
+  return execFileSync(plan.file, plan.args, shimSpawnOptions(plan, options));
 }
 
 function copyDirRecursive(src, dest) {