diff --git a/changelog.d/fixes/7279-cli-detector-windows-drift.md b/changelog.d/fixes/7279-cli-detector-windows-drift.md new file mode 100644 index 0000000000..e6a8fd53df --- /dev/null +++ b/changelog.d/fixes/7279-cli-detector-windows-drift.md @@ -0,0 +1 @@ +- fix(cli): reuse cliRuntime's win32-aware `locateCommand`/`shell:true` probe in tool-detector so installed CLIs (npm `.cmd` shims) are no longer reported as absent on native Windows (#7279) diff --git a/src/lib/cli-helper/tool-detector.ts b/src/lib/cli-helper/tool-detector.ts index b6463132ca..48b03bb555 100644 --- a/src/lib/cli-helper/tool-detector.ts +++ b/src/lib/cli-helper/tool-detector.ts @@ -3,24 +3,24 @@ import path from "node:path"; import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { getCurrentHermesAgentRoles } from "./config-generator/hermes-agent"; -import { getCachedLoginShellPath, mergeShellPath } from "../../shared/services/loginShellPath"; +import { + getLookupEnv, + locateCommand, + shouldUseShellForCommand, +} from "../../shared/services/cliRuntime"; const execFileAsync = promisify(execFile); let execFileImpl = execFileAsync; - -// #3321: macOS GUI/Electron truncates PATH, so `which`/`--version` probes miss Homebrew/ -// nvm/volta CLIs and the doctor reports them "not installed". Build a lookup env enriched -// with the login-shell PATH (darwin-only, cached, fail-safe → returns process.env elsewhere). -function detectorEnv(): NodeJS.ProcessEnv { - const loginShellPath = getCachedLoginShellPath(); - if (!loginShellPath) return process.env; - return { ...process.env, PATH: mergeShellPath(process.env.PATH || "", loginShellPath) }; -} +let locateCommandImpl = locateCommand; export function __setExecFileImpl(fn: typeof execFileAsync): void { execFileImpl = fn; } +export function __setLocateCommandImpl(fn: typeof locateCommand): void { + locateCommandImpl = fn; +} + export interface DetectedTool { id: string; name: string; @@ -79,17 +79,52 @@ function isConfigured(content: string, baseUrl: string): boolean { ); } +// #968/#7279: on native Windows, npm installs CLI wrappers (claude/codex/opencode/…) +// as .cmd/.bat shims. Node's CVE-2024-27980 hardening makes execFile()/spawn() reject +// those without `shell: true`, and the `which` fallback below doesn't exist natively +// on Windows (no WSL/git-bash) — so both probes threw, both were swallowed, and an +// installed CLI was reported as absent. Reuse cliRuntime.ts's `locateCommand` +// (already win32-aware since #968: `where.exe` + `.cmd`/`.exe`/`.bat`/`.com` +// preference) for existence/path, then probe `--version` with `shell: true` when the +// resolved binary needs it. If this drifts again, check cliRuntime.ts first. +async function detectBinaryWindows( + binary: string, + env: NodeJS.ProcessEnv +): Promise<{ installed: boolean; version?: string }> { + const located = await locateCommandImpl(binary, env); + if (!located.installed || !located.commandPath) return { installed: false }; + + try { + const useShell = shouldUseShellForCommand(located.commandPath); + const { stdout } = await execFileImpl(located.commandPath, ["--version"], { + timeout: 5000, + env, + ...(useShell ? { shell: true } : {}), + }); + return { installed: true, version: stdout.trim().replace(/^v/, "") }; + } catch { + // Binary exists on PATH but the --version probe failed (unusual flag, slow + // startup, etc.) — still report it as installed since locateCommand confirmed it. + return { installed: true }; + } +} + async function detectBinary(name: string): Promise<{ installed: boolean; version?: string }> { const binary = BINARY_NAMES[name] || name; - const env = detectorEnv(); + const env = getLookupEnv(); + + if (process.platform === "win32") { + return detectBinaryWindows(binary, env); + } + try { const { stdout } = await execFileImpl(binary, ["--version"], { timeout: 5000, env }); const version = stdout.trim().replace(/^v/, ""); return { installed: true, version }; } catch { try { - // Try `which` as fallback - const { stdout } = await execFileAsync("which", [binary], { timeout: 5000, env }); + // Try `which` as fallback (routed through execFileImpl so it stays mockable) + const { stdout } = await execFileImpl("which", [binary], { timeout: 5000, env }); if (stdout.trim()) { return { installed: true }; } diff --git a/src/shared/services/cliRuntime.ts b/src/shared/services/cliRuntime.ts index c7e778a9a0..e6c8e437fc 100644 --- a/src/shared/services/cliRuntime.ts +++ b/src/shared/services/cliRuntime.ts @@ -742,7 +742,7 @@ const checkExplicitPath = async (commandPath: string) => { } }; -const locateCommand = async (command: string, env: Record) => { +export const locateCommand = async (command: string, env: Record) => { if (!command) { return { installed: false, commandPath: null, reason: "missing_command" }; } diff --git a/tests/unit/cli-helper/tool-detector-win32-7279.test.ts b/tests/unit/cli-helper/tool-detector-win32-7279.test.ts new file mode 100644 index 0000000000..f43833607f --- /dev/null +++ b/tests/unit/cli-helper/tool-detector-win32-7279.test.ts @@ -0,0 +1,80 @@ +import { describe, it, before, after } from "node:test"; +import assert from "node:assert"; +import * as toolDetector from "../../../src/lib/cli-helper/tool-detector.ts"; + +// #7279 (re-drift of #968) — detectBinary() in tool-detector.ts never checked +// process.platform and never passed shell:true, so on native Windows an +// installed CLI (npm installs claude/codex/opencode as .cmd shims) was reported +// as NOT installed: +// 1. execFileImpl(binary, ["--version"]) fails without shell:true for .cmd shims +// (Node's CVE-2024-27980 hardening). +// 2. the `which` fallback doesn't exist on native Windows (no WSL/git-bash). +// Both throw, both are swallowed by empty catches, detectBinary returns +// { installed: false }. cliRuntime.ts::locateCommand already solved this for +// the runtime-spawn path (#968); this fix reuses it here. +// +// Methodological note (see plan-file): the `which` fallback previously called +// the RAW execFileAsync, not the injected __setExecFileImpl hook, so it wasn't +// mockable and could silently "pass" using the real system `which`. Uses +// `hermes` (confirmed absent from PATH) to avoid that trap; also uses a +// dedicated __setLocateCommandImpl hook (mirrors __setExecFileImpl) so the +// win32 existence probe is deterministic here instead of depending on a real +// `where.exe`. + +describe("tool-detector — win32 (#7279)", () => { + const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + + function setPlatform(value: string) { + Object.defineProperty(process, "platform", { configurable: true, value }); + } + + before(() => { + setPlatform("win32"); + + toolDetector.__setLocateCommandImpl(async (command: string) => { + if (command === "hermes") { + return { + installed: true, + commandPath: "C:\\Users\\dev\\AppData\\Roaming\\npm\\hermes.cmd", + reason: null, + }; + } + return { installed: false, commandPath: null, reason: "not_found" }; + }); + + // @ts-expect-error - internal test hook + toolDetector.__setExecFileImpl(async (_cmd: string, _args: string[], opts?: { shell?: boolean }) => { + // Reproduces the real-world failure: without shell:true, spawning the + // .cmd shim throws (Node's CVE-2024-27980 hardening on Windows). + if (opts?.shell === true) { + return { stdout: "v0.75.3\n" }; + } + throw new Error("spawn hermes.cmd ENOENT (shell:true required on win32 for .cmd shims)"); + }); + }); + + after(() => { + // This is the only test file exercising these hooks — node:test isolates + // each file's module cache, so no further reset is needed for other suites. + if (originalPlatformDescriptor) { + Object.defineProperty(process, "platform", originalPlatformDescriptor); + } + }); + + it("reports an installed CLI as installed on native Windows (.cmd shim probed with shell:true)", async () => { + const result = await toolDetector.detectTool("hermes"); + assert.ok(result !== null); + assert.strictEqual( + result!.installed, + true, + "expected hermes to be detected as installed via locateCommand + shell:true probe on win32" + ); + assert.strictEqual(result!.version, "0.75.3"); + }); + + it("reports a genuinely absent CLI as not installed on native Windows", async () => { + const result = await toolDetector.detectTool("openclaw"); + assert.ok(result !== null); + assert.strictEqual(result!.installed, false); + }); +});