From 11e8eb34944f14ba28eec309f4f134df865d5d54 Mon Sep 17 00:00:00 2001 From: abose Date: Tue, 27 Oct 2026 14:51:24 +0530 Subject: [PATCH] fix(ai): resolve Windows npm shims before SDK launch Resolve Claude npm shims to their version-matched native executable and fall back to the bundled CLI when needed. Preserve terminal launch behavior. Cover Windows shim resolution and update the permission fixture's locator mock. --- src-node/ai-cli-capabilities.js | 16 ++--- src-node/claude-code-agent.js | 15 +++-- src-node/cli-locator.js | 78 +++++++++++++++++++++-- src-node/test/test-ai-file-permissions.js | 3 +- src-node/test/test-cli-locator.js | 76 ++++++++++++++++++++++ test/spec/CLILocator-test.js | 35 ++++++++++ 6 files changed, 202 insertions(+), 21 deletions(-) diff --git a/src-node/ai-cli-capabilities.js b/src-node/ai-cli-capabilities.js index 10bd5f337e..dceda5dc77 100644 --- a/src-node/ai-cli-capabilities.js +++ b/src-node/ai-cli-capabilities.js @@ -12,16 +12,14 @@ const CliLocator = require("./cli-locator"); * Resolve an npm Windows shim to the native binary shipped by that same installation. * Never silently switches a user-selected CLI to a different installed version. * @param {string} cli Provider id. - * @param {string} executable Located CLI path. + * @param {Object} located A CliLocator.locateCli result. * @return {string} Executable suitable for argument-array spawning. */ -function nativeExecutable(cli, executable) { - if (process.platform !== "win32" || !/\.(cmd|bat)$/i.test(executable)) { return executable; } - const base = path.dirname(executable); - const candidates = cli === "claude" ? [ - path.join(base, "node_modules", "@anthropic-ai", "claude-code", "bin", "claude.exe"), - path.join(base, "node_modules", "@anthropic-ai", "claude-code-win32-x64", "claude.exe") - ] : [ +function nativeExecutable(cli, located) { + // Set unless the path is a shim the locator could not follow, as it can for Claude Code. + if (located.execPath) { return located.execPath; } + const base = path.dirname(located.path); + const candidates = cli !== "codex" ? [] : [ path.join(base, "node_modules", "@openai", "codex", "vendor", "x86_64-pc-windows-msvc", "codex", "codex.exe"), path.join(base, "node_modules", "@openai", "codex-win32-x64", "vendor", "x86_64-pc-windows-msvc", "codex", "codex.exe") @@ -39,7 +37,7 @@ function nativeExecutable(cli, executable) { async function checkLaunch(cli, projectRoot) { const located = await CliLocator.locateCli(cli); if (!located.path) { throw new Error("CLI is no longer available."); } - const executable = nativeExecutable(cli, located.path); + const executable = nativeExecutable(cli, located); const help = await CliLocator.spawnCli(executable, ["--help"], {cwd: projectRoot, timeout: 5000, windowsHide: true}); const required = cli === "codex" ? ["--no-daemon"] : diff --git a/src-node/claude-code-agent.js b/src-node/claude-code-agent.js index 64523ab0b6..c5a557937f 100644 --- a/src-node/claude-code-agent.js +++ b/src-node/claude-code-agent.js @@ -2190,13 +2190,14 @@ async function _runQuery(requestId, prompt, projectPath, model, signal, locale, }; } - // Set Claude CLI path if found. The env the binary's origin calls for - // rides along: the SDK spawns the same executable the CLI pane does, so - // the bundled copy must not try to auto-update here either. - const located = await CliLocator.locateCli("claude"); - if (located.path) { - queryOptions.pathToClaudeCodeExecutable = located.path; - Object.assign(queryOptions.env, CliLocator.getSourceEnv(located.source)); + // Set Claude CLI path if found. The SDK spawns it without a shell, so a + // Windows npm .cmd shim is swapped for the binary behind it, or the + // bundled copy. The env the binary's origin calls for rides along: the + // bundled copy must not try to auto-update here either. + const executable = await CliLocator.getDirectExecutable(await CliLocator.locateCli("claude")); + if (executable) { + queryOptions.pathToClaudeCodeExecutable = executable.path; + Object.assign(queryOptions.env, CliLocator.getSourceEnv(executable.source)); } if (model) { diff --git a/src-node/cli-locator.js b/src-node/cli-locator.js index af9ff8dd40..ddc6dae67d 100644 --- a/src-node/cli-locator.js +++ b/src-node/cli-locator.js @@ -86,6 +86,8 @@ const CLI_REGISTRY = { bin: "claude", versionArgs: ["--version"], versionPattern: /^\d/, + // The package `npm i -g` installs, for following its Windows shim to the binary it runs. + npmPackage: "@anthropic-ai/claude-code", // claude.ai/install.sh and the desktop installer drop real binaries // here — no node/cli.js shim chain to break, so existence is enough. winNative: function () { @@ -445,6 +447,48 @@ async function _probeVersion(cli, cliPath) { return { ok: true, version, errorCode: null, stderr: "" }; } +/** + * The binary an npm shim runs, found without reading the shim. npm installs a + * global package under {prefix}\node_modules and writes the shim into + * {prefix} from the package's own `bin` entry, so follow that entry rather + * than parse a file whose format npm is free to change. + * @return {Promise} an existing `.exe` inside the package, or null + */ +async function _npmShimTarget(cli, shimPath) { + if (!cli.npmPackage) { return null; } + const pkgDir = path.join(path.dirname(shimPath), "node_modules", ...cli.npmPackage.split("/")); + try { + const pkg = JSON.parse(await fs.promises.readFile(path.join(pkgDir, "package.json"), "utf8")); + const bin = pkg.bin && typeof pkg.bin === "object" ? pkg.bin[cli.bin] : null; + if (typeof bin !== "string") { return null; } + const target = path.resolve(pkgDir, bin); + // Only a native binary inside the package: a JS entry point would + // need a Node that a desktop launch's PATH may not have. + if (!target.startsWith(pkgDir + path.sep) || !/\.exe$/i.test(target)) { return null; } + return await canAccess(target) ? target : null; + } catch { + return null; + } +} + +/** + * A path that runs without a shell. Node refuses to spawn a Windows + * `.cmd`/`.bat` shim shell-less (spawn EINVAL, CVE-2024-27980 hardening), so + * an npm shim is followed to the binary it runs; anything else already is one. + * @param {Object} cli - a CLI_REGISTRY entry + * @param {string} cliPath - a located, version-probed path + * @param {?string} version - its `--version` output + * @return {Promise} null for a shim with no binary of the same version behind it + */ +async function _execPathFor(cli, cliPath, version) { + if (!isWindows || !/\.(cmd|bat)$/i.test(cliPath)) { return cliPath; } + const target = await _npmShimTarget(cli, cliPath); + if (!target) { return null; } + // Same installation only: the placeholder a skipped postinstall leaves fails here. + const probe = await _probeVersion(cli, target); + return probe.ok && compareVersions(probe.version, version) === 0 ? target : null; +} + /** * Resolve and validate a user-configured override path. * @@ -605,7 +649,8 @@ async function _cacheHit(cliId, overrideSig) { if (!entry || entry.overrideSig !== overrideSig) { return null; } - if (entry.path !== null && !await canAccess(entry.path)) { + if (entry.path !== null && (!await canAccess(entry.path) || + (entry.execPath && !await canAccess(entry.execPath)))) { console.log("[Phoenix AI] " + cliId + " CLI is no longer at", entry.path); if (_cache.get(cliId) === entry) { _cache.delete(cliId); } return null; @@ -618,6 +663,7 @@ function _toResult(cliId, entry) { return { cli: cliId, path: entry.path, + execPath: entry.execPath || null, source: entry.source || null, version: entry.version || null, userInstalled: !!entry.userInstalled, @@ -635,9 +681,10 @@ function _toResult(cliId, entry) { * share) after a runtime spawn failure; `{override}` uses that path for * this call only instead of the stored one, so the settings UI can * preview a path without committing it. - * @return {Promise} `{cli, path, source, version, userInstalled, errorCode, override, - * searchedPaths}`. `userInstalled` says the user has a CLI of their own (found or configured), - * even when a newer bundled copy is the one `path` points to. + * @return {Promise} `{cli, path, execPath, source, version, userInstalled, errorCode, + * override, searchedPaths}`. `userInstalled` says the user has a CLI of their own (found or + * configured), even when a newer bundled copy is the one `path` points to. `execPath` is + * `path` made safe to spawn without a shell (see _execPathFor), or null when it cannot be. */ function locateCli(cliId, opts) { const cli = CLI_REGISTRY[cliId]; @@ -668,6 +715,7 @@ function locateCli(cliId, opts) { entry.override = report; if (report && report.valid) { entry.path = report.path; + entry.execPath = await _execPathFor(cli, report.path, report.version); entry.source = "override"; entry.version = report.version; entry.userInstalled = true; @@ -734,6 +782,7 @@ function locateCli(cliId, opts) { } if (found) { entry.path = found.path; + entry.execPath = await _execPathFor(cli, found.path, found.version); entry.source = found.source; entry.version = found.version; entry.at = Date.now(); @@ -843,6 +892,26 @@ function getSpawnProfile(cliPath, source) { return { command: cliPath, args: [], env }; } +/** + * What a caller that spawns without a shell, such as the Claude Agent SDK, + * should run for a locateCli result: its `execPath`, or, for a Windows shim + * with no binary found behind it, the bundled Claude Code. A shim must never + * reach such a caller: Node throws spawn EINVAL for it. + * @param {Object} located - a locateCli result + * @return {Promise} null when nothing can run + */ +async function getDirectExecutable(located) { + if (!located || !located.path) { return null; } + if (located.execPath) { + return { path: located.execPath, source: located.source }; + } + const bundled = located.cli === "claude" ? await _locateBundledClaude() : null; + if (!bundled) { return null; } + console.log("[Phoenix AI] No binary found behind " + located.path + "; using the bundled " + + bundled.version); + return { path: bundled.path, source: "bundled" }; +} + exports.CLI_IDS = CLI_IDS; exports.ERROR_CODES = ERROR_CODES; exports.canAccess = canAccess; @@ -851,6 +920,7 @@ exports.locateCli = locateCli; exports.validateCliPath = validateCliPath; exports.findDownloader = findDownloader; exports.getSpawnProfile = getSpawnProfile; +exports.getDirectExecutable = getDirectExecutable; exports.getSourceEnv = _cliSourceEnv; exports.compareVersions = compareVersions; exports.setBundledRoot = setBundledRoot; diff --git a/src-node/test/test-ai-file-permissions.js b/src-node/test/test-ai-file-permissions.js index 5c50c5099c..f7634a8797 100644 --- a/src-node/test/test-ai-file-permissions.js +++ b/src-node/test/test-ai-file-permissions.js @@ -104,7 +104,8 @@ async function exercise(params) { os: {tmpdir: () => temp}, path, "./mcp-editor-tools": {createEditorMcpServer: () => ({})}, - "./cli-locator": {locateCli: async () => ({}), getSourceEnv: () => ({})}, + "./cli-locator": {locateCli: async () => ({}), getDirectExecutable: async () => null, + getSourceEnv: () => ({})}, "./ai-image-preview": {}, "./ai-system-prompt": {buildSystemPrompt: () => "", buildEditorContextLine: () => ""}, "./ai-cli-connector": {setBrowserConnector: () => {}}, diff --git a/src-node/test/test-cli-locator.js b/src-node/test/test-cli-locator.js index 1ff544c79b..21c016463e 100644 --- a/src-node/test/test-cli-locator.js +++ b/src-node/test/test-cli-locator.js @@ -264,6 +264,81 @@ async function relocateAfterRemoval() { return { before, after }; } +const WIN_NPM = "C:\\Users\\me\\AppData\\Roaming\\npm"; +const WIN_SHIM = WIN_NPM + "\\claude.cmd"; +const WIN_PACKAGE = WIN_NPM + "\\node_modules\\@anthropic-ai\\claude-code"; +const WIN_BUNDLED_ROOT = "C:\\phoenix\\src-node"; +const WIN_BUNDLED = WIN_BUNDLED_ROOT + "\\node_modules\\@anthropic-ai\\claude-agent-sdk-win32-x64\\claude.exe"; + +// What npm leaves behind in each case. `bin` is the package's entry; `binary` +// is what that path holds: its --version, or null for a file that won't run. +const NPM_SHIM_SCENARIOS = { + resolved: { bin: "bin/claude.exe", binary: "2.1.280" }, + placeholder: { bin: "bin/claude.exe", binary: null }, // postinstall skipped + otherVersion: { bin: "bin/claude.exe", binary: "2.1.279" }, + jsEntry: { bin: "cli.js", binary: "2.1.280" }, + outsidePackage: { bin: "../../evil.exe", binary: "2.1.280" }, + noPackage: { bin: null, binary: null } +}; + +/** + * Locate claude on a simulated Windows machine whose only Claude Code of its + * own is an npm shim at the bundled version, then ask what a shell-less + * caller like the Agent SDK would run. Nothing is installed or read from disk. + * @param {{scenario: string, override: boolean}} params - an NPM_SHIM_SCENARIOS key; whether + * the shim is the configured path rather than discovered + * @return {Promise} The located result, the direct executable and the fixture paths + */ +async function locateWindowsNpmClaude({scenario, override}) { + const fixture = NPM_SHIM_SCENARIOS[scenario]; + const target = fixture.bin ? path.win32.resolve(WIN_PACKAGE, fixture.bin) : null; + const versions = { [WIN_SHIM]: "2.1.280", [WIN_BUNDLED]: "2.1.280" }; + const files = new Set(Object.keys(versions)); + if (target) { + files.add(target); + versions[target] = fixture.binary; + } + const env = { + APPDATA: "C:\\Users\\me\\AppData\\Roaming", LOCALAPPDATA: "C:\\Users\\me\\AppData\\Local", + USERPROFILE: "C:\\Users\\me", Path: "C:\\Windows" + }; + const spawned = []; + const locator = _loadLocator("win32", env, function (command) { + // spawnCli quotes a shim's name for the shell it runs it in + const file = command.replace(/^"(.*)"$/, "$1"); + spawned.push(file); + const version = versions[file]; + return childProcess.spawn(process.execPath, ["-e", version + ? "console.log(" + JSON.stringify(version + " (Claude Code)") + ")" + : "process.exit(1)"]); + }, { + fs: Object.assign({}, fs, { + promises: { + access: async function (candidate) { + if (!files.has(candidate)) { throw new Error("ENOENT"); } + }, + readFile: async function (file) { + if (fixture.bin && file === WIN_PACKAGE + "\\package.json") { + return JSON.stringify({ name: "@anthropic-ai/claude-code", bin: { claude: fixture.bin } }); + } + throw new Error("ENOENT"); + }, + readdir: async function () { return []; } + } + }), + execFile: function (_command, _args, _options, callback) { + callback(new Error("not on PATH"), ""); + } + }); + locator.setBundledRoot(WIN_BUNDLED_ROOT); + if (override) { + locator.setOverrides({ claude: WIN_SHIM }); + } + const located = await locator.locateCli("claude"); + const direct = await locator.getDirectExecutable(located); + return { located, direct, spawned, shim: WIN_SHIM, target, bundled: WIN_BUNDLED }; +} + /** * Report the downloader chosen for each simulated set of installed tools. * @param {{installed: Array>}} params - Tool names on PATH, one list per scenario @@ -362,6 +437,7 @@ exports.getSpawnProfile = getSpawnProfile; exports.locateWindowsStandaloneCodex = locateWindowsStandaloneCodex; exports.findDownloaders = findDownloaders; exports.locateWithBundled = locateWithBundled; +exports.locateWindowsNpmClaude = locateWindowsNpmClaude; exports.relocateAfterRemoval = relocateAfterRemoval; exports.probeWindowsShim = probeWindowsShim; exports.runNpmFixture = runNpmFixture; diff --git a/test/spec/CLILocator-test.js b/test/spec/CLILocator-test.js index da77bea7df..38131108ef 100644 --- a/test/spec/CLILocator-test.js +++ b/test/spec/CLILocator-test.js @@ -161,6 +161,7 @@ define(function (require, exports, module) { const newer = await nodeConnector.execPeer("locateWithBundled", { systemVersion: "2.2.0", bundledVersion: "2.1.141" }); expect(newer.path).withContext(JSON.stringify(newer)).toBe("/home/test/.local/bin/claude"); + expect(newer.execPath).toBe(newer.path); expect(newer.source).toBe("native"); expect(newer.userInstalled).toBeTrue(); const older = await nodeConnector.execPeer("locateWithBundled", @@ -184,6 +185,40 @@ define(function (require, exports, module) { expect(configured.userInstalled).toBeTrue(); }, 20000); + // Simulated Windows: npm's claude.cmd at the bundled version, so it wins the tie-break. + // The Agent SDK spawns without a shell, which Node refuses for a .cmd (spawn EINVAL). + [false, true].forEach(function (override) { + const how = override ? "a configured" : "a discovered"; + it("win32: should give the Agent SDK the binary behind " + how + " npm shim", async function () { + const result = await nodeConnector.execPeer("locateWindowsNpmClaude", + { scenario: "resolved", override }); + const context = JSON.stringify(result); + // the terminal keeps the shim, which it runs through cmd.exe + expect(result.located.path).withContext(context).toBe(result.shim); + expect(result.located.source).toBe(override ? "override" : "fallback"); + expect(result.located.execPath).toBe(result.target); + expect(result.direct).toEqual({ path: result.target, source: result.located.source }); + expect(result.spawned).toContain(result.target); + }, 15000); + }); + + [ + ["placeholder", "is the placeholder a skipped postinstall leaves"], + ["otherVersion", "reports another version"], + ["jsEntry", "is a script, not a binary"], + ["outsidePackage", "points outside the package"], + ["noPackage", "has no package"] + ].forEach(function ([scenario, reason]) { + it("win32: should give the Agent SDK the bundled copy when the npm shim's target " + reason, + async function () { + const result = await nodeConnector.execPeer("locateWindowsNpmClaude", { scenario }); + const context = JSON.stringify(result); + expect(result.located.path).withContext(context).toBe(result.shim); + expect(result.located.execPath).withContext(context).toBeNull(); + expect(result.direct).withContext(context).toEqual({ path: result.bundled, source: "bundled" }); + }, 15000); + }); + it("should notice a cached Claude Code has gone, and fall back to the bundled one", async function () { const result = await nodeConnector.execPeer("relocateAfterRemoval"); expect(result.before.source).withContext(JSON.stringify(result.before)).toBe("native");