From af4f10573f4e91da89cd18b4db971331f7e0c480 Mon Sep 17 00:00:00 2001 From: gitikavj Date: Mon, 26 Oct 2026 15:53:09 +0000 Subject: [PATCH] fix(io): resolve Windows commands from explicit PATH entries Resolve bare Windows commands using absolute PATH entries before launching them. Route browser launch and process cleanup through the shared launcher, and propagate the Windows lookup setting to child processes. --- src/index.ts | 2 + src/io/exec-arguments.test.ts | 18 +++- src/io/exec.test.ts | 55 ++++++++++++ src/io/exec.ts | 163 ++++++++++++++++++++++++++++++++-- src/io/openBrowser.ts | 4 +- 5 files changed, 233 insertions(+), 9 deletions(-) diff --git a/src/index.ts b/src/index.ts index 1e84b570cb..67296bbb20 100644 --- a/src/index.ts +++ b/src/index.ts @@ -24,7 +24,9 @@ import { PACKAGE_VERSION } from "./constants"; import { CommandRunMetricEventKey, ValueContext } from "./router"; import { NpmCliVersionManager, printUpdateNotice } from "./cliVersionManager"; import { CliVersionManagerKey } from "./handlers/keys"; +import { disableCwdExecutableSearch } from "./io/exec"; +disableCwdExecutableSearch(); // Suppress only the AWS SDK's Node.js version support warning before constructing clients. process.env.AWS_SDK_JS_NODE_VERSION_SUPPORT_WARNING_DISABLED = "true"; diff --git a/src/io/exec-arguments.test.ts b/src/io/exec-arguments.test.ts index 85bf541c63..cb6be549d5 100644 --- a/src/io/exec-arguments.test.ts +++ b/src/io/exec-arguments.test.ts @@ -2,7 +2,13 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join, relative } from "node:path"; -import { runInheritedProcess, runProcess, streamProcess, toolAvailable } from "./exec"; +import { + ProcessFailedError, + runInheritedProcess, + runProcess, + streamProcess, + toolAvailable, +} from "./exec"; const runners = [ { @@ -87,4 +93,14 @@ describe.skipIf(process.platform !== "win32")("Windows npm command shims", () => await runProcess([relative(directory, shimPath), output, ...literalArgs], { cwd: directory }); expect(JSON.parse(await readFile(output, "utf8"))).toEqual(literalArgs); }); + + test("does not resolve a bare command from the child working directory", async () => { + await writeFile(join(directory, "cwd-only.cmd"), "@exit /b 0\r\n"); + await expect( + runProcess(["cwd-only"], { + cwd: directory, + env: { ...process.env, PATH: "", Path: "", PATHEXT: ".CMD" }, + }), + ).rejects.toBeInstanceOf(ProcessFailedError); + }); }); diff --git a/src/io/exec.test.ts b/src/io/exec.test.ts index 5bfb77fbff..ecc5d683fd 100644 --- a/src/io/exec.test.ts +++ b/src/io/exec.test.ts @@ -8,6 +8,7 @@ import { MissingToolError, ProcessFailedError, requireTool, + resolveExecutable, runInheritedProcess, runProcess, streamProcess, @@ -15,6 +16,60 @@ import { type ProcessEvent, } from "./exec"; +describe("resolveExecutable", () => { + const files = new Set( + [ + "C:\\Program Files\\nodejs\\npm.cmd", + "C:\\Program Files\\nodejs\\node.exe", + "C:\\Windows\\System32\\where.exe", + ].map((path) => path.toLowerCase()), + ); + const isExecutableFile = (path: string) => files.has(path.toLowerCase()); + const resolve = (command: string, env: NodeJS.ProcessEnv) => + resolveExecutable(command, { env, platform: "win32", isExecutableFile }); + + test("resolves bare commands from PATH with PATHEXT", () => { + const env = { + Path: "C:\\Program Files\\nodejs;C:\\Windows\\System32", + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }; + + expect(resolve("npm", env)?.toLowerCase()).toBe("c:\\program files\\nodejs\\npm.cmd"); + expect(resolve("node", env)?.toLowerCase()).toBe("c:\\program files\\nodejs\\node.exe"); + expect(resolve("where", env)?.toLowerCase()).toBe("c:\\windows\\system32\\where.exe"); + }); + + test("does not add the current directory to PATH lookup", () => { + expect(resolve("npm", { Path: "C:\\Windows\\System32", PATHEXT: ".CMD" })).toBeNull(); + }); + + test("skips empty and relative PATH entries", () => { + const cwdFiles = new Set(["npm.cmd", ".\\npm.cmd", "bin\\npm.cmd"]); + expect( + resolveExecutable("npm", { + env: { Path: ";.;bin;", PATHEXT: ".CMD" }, + platform: "win32", + isExecutableFile: (path) => cwdFiles.has(path), + }), + ).toBeNull(); + }); + + test("handles quoted and duplicate-case PATH keys consistently", () => { + expect( + resolve("npm", { + Path: "C:\\Windows\\System32", + PATH: '"C:\\Program Files\\nodejs"', + PATHEXT: ".CMD", + })?.toLowerCase(), + ).toBe("c:\\program files\\nodejs\\npm.cmd"); + }); + + test("leaves explicit paths unchanged", () => { + expect(resolve(".\\tools\\npm.cmd", {})).toBe(".\\tools\\npm.cmd"); + expect(resolve("C:\\tools\\npm.cmd", {})).toBe("C:\\tools\\npm.cmd"); + }); +}); + // Run Node scripts from files so these tests work with both Node and Bun. const scriptsDir = await mkdtemp(join(tmpdir(), "agentcore-exec-")); async function script(name: string, source: string): Promise { diff --git a/src/io/exec.ts b/src/io/exec.ts index 11ef4e32a3..46bd67ac41 100644 --- a/src/io/exec.ts +++ b/src/io/exec.ts @@ -1,14 +1,21 @@ // Local subprocess execution. Uses node:child_process (not Bun.$/Bun.spawn) // because the npm bundle targets Node — Bun APIs are unavailable there. -import { type ChildProcess, execFileSync, type SpawnOptions } from "node:child_process"; +import { ChildProcess, type SpawnOptions, type SpawnSyncOptions } from "node:child_process"; +import { accessSync, constants as fsConstants, statSync } from "node:fs"; import { constants as osConstants } from "node:os"; +import { posix as posixPath, win32 as win32Path } from "node:path"; import { createInterface } from "node:readline"; +import { PassThrough } from "node:stream"; import crossSpawn from "cross-spawn"; import { AgentCoreCLIError, ERROR_SOURCE } from "../errors"; const isWindows = process.platform === "win32"; const KILL_GRACE_MS = 2000; const MAX_ERROR_OUTPUT_LINES = 20; +const DEFAULT_WINDOWS_PATHEXT = ".COM;.EXE;.BAT;.CMD"; +const WINDOWS_ENOENT_ERRNO = -4058; + +export const NO_CWD_EXE_SEARCH_ENV = "NoDefaultCurrentDirectoryInExePath"; /** Error raised when a required executable is not found on PATH. */ export class MissingToolError extends AgentCoreCLIError { @@ -30,10 +37,150 @@ export class ProcessFailedError extends AgentCoreCLIError { } } -function spawnCommand(executable: string, args: string[], options: SpawnOptions): ChildProcess { - // Keep arguments separate. cross-spawn resolves Windows wrappers and applies - // the extra escaping required when a node_modules/.bin shim forwards them. - return crossSpawn(executable, args, options); +export type ResolveExecutableOptions = { + env?: NodeJS.ProcessEnv; + platform?: NodeJS.Platform; + isExecutableFile?: (path: string, platform: NodeJS.Platform) => boolean; +}; + +function defaultIsExecutableFile(path: string, platform: NodeJS.Platform): boolean { + try { + if (!statSync(path).isFile()) return false; + if (platform !== "win32") accessSync(path, fsConstants.X_OK); + return true; + } catch { + return false; + } +} + +function environmentValue( + env: NodeJS.ProcessEnv, + name: string, + platform: NodeJS.Platform, +): string | undefined { + if (platform !== "win32") return env[name]; + const key = Object.keys(env) + .filter((candidate) => candidate.toUpperCase() === name.toUpperCase()) + .sort()[0]; + return key === undefined ? undefined : env[key]; +} + +/** + * Resolves a bare command using absolute PATH entries without an implicit + * current-directory candidate. Commands containing a path separator are + * explicit and remain unchanged. + */ +export function resolveExecutable( + command: string, + options: ResolveExecutableOptions = {}, +): string | null { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const isExecutableFile = options.isExecutableFile ?? defaultIsExecutableFile; + const pathImpl = platform === "win32" ? win32Path : posixPath; + + if (!command) return null; + if (command.includes("/") || (platform === "win32" && command.includes("\\"))) { + return command; + } + + const directories = (environmentValue(env, "PATH", platform) ?? "") + .split(platform === "win32" ? ";" : ":") + .map((directory) => (platform === "win32" ? directory.replace(/^"(.*)"$/, "$1") : directory)) + .filter((directory) => directory.length > 0 && pathImpl.isAbsolute(directory)); + + let candidates = [command]; + if (platform === "win32") { + const extensions = (environmentValue(env, "PATHEXT", platform) ?? DEFAULT_WINDOWS_PATHEXT) + .split(";") + .map((extension) => extension.trim()) + .filter(Boolean); + const hasKnownExtension = extensions.some((extension) => + command.toLowerCase().endsWith(extension.toLowerCase()), + ); + candidates = hasKnownExtension + ? [command] + : [ + ...(pathImpl.extname(command) ? [command] : []), + ...extensions.map((extension) => command + extension), + ]; + } + + for (const directory of directories) { + for (const candidate of candidates) { + const resolved = pathImpl.join(directory, candidate); + if (isExecutableFile(resolved, platform)) return resolved; + } + } + return null; +} + +function childEnvironment(env: NodeJS.ProcessEnv | undefined): NodeJS.ProcessEnv | undefined { + if (!isWindows) return env; + return { ...(env ?? process.env), [NO_CWD_EXE_SEARCH_ENV]: "1" }; +} + +/** Applies the Windows process lookup setting inherited by third-party child processes. */ +export function disableCwdExecutableSearch(): void { + if (isWindows) process.env[NO_CWD_EXE_SEARCH_ENV] = "1"; +} + +function missingExecutableError(command: string, args: readonly string[]) { + return Object.assign(new Error(`spawn ${command} ENOENT`), { + code: "ENOENT", + errno: WINDOWS_ENOENT_ERRNO, + syscall: `spawn ${command}`, + path: command, + spawnargs: [...args], + }); +} + +function missingChildProcess(command: string, args: readonly string[]): ChildProcess { + const child = new ChildProcess(); + (child as unknown as { _handle: unknown })._handle = null; + const stdin = new PassThrough(); + const stdout = new PassThrough(); + const stderr = new PassThrough(); + Object.defineProperties(child, { + stdin: { value: stdin }, + stdout: { value: stdout }, + stderr: { value: stderr }, + stdio: { value: [stdin, stdout, stderr] }, + }); + const error = missingExecutableError(command, args); + process.nextTick(() => { + child.emit("error", error); + stdout.end(); + stderr.end(); + child.emit("close", error.errno, null); + }); + return child; +} + +/** + * Spawns a command while preserving argument boundaries and Windows command-shim support. + */ +export function spawnCommand( + executable: string, + args: string[], + options: SpawnOptions, +): ChildProcess { + const resolved = isWindows ? resolveExecutable(executable, { env: options.env }) : executable; + if (resolved === null) return missingChildProcess(executable, args); + return crossSpawn(resolved, args, { ...options, env: childEnvironment(options.env) }); +} + +function spawnCommandSync( + executable: string, + args: string[], + options: SpawnSyncOptions, +): ReturnType { + const resolved = isWindows ? resolveExecutable(executable, { env: options.env }) : executable; + if (resolved === null) throw missingExecutableError(executable, args); + return crossSpawn.sync(resolved, args, { + ...options, + env: childEnvironment(options.env), + }); } /** Returns true if running `tool` with probeArgs (`--version` by default) exits 0. */ @@ -304,7 +451,11 @@ function killTree(child: ChildProcess, signal: NodeJS.Signals): void { if (!child.pid) return; try { if (isWindows) { - execFileSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); + const result = spawnCommandSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { + stdio: "ignore", + }); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error(`taskkill exited with code ${result.status}`); } else { process.kill(-child.pid, signal); } diff --git a/src/io/openBrowser.ts b/src/io/openBrowser.ts index 9e4990c405..6e6fdc0be9 100644 --- a/src/io/openBrowser.ts +++ b/src/io/openBrowser.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { spawnCommand } from "./exec"; export type BrowserOpener = (url: string) => Promise; @@ -16,7 +16,7 @@ export const openBrowser: BrowserOpener = (url) => { : ["xdg-open", [url]]; return new Promise((resolve) => { - const child = spawn(command!, args, { stdio: "ignore", detached: true }); + const child = spawnCommand(command!, args, { stdio: "ignore", detached: true }); child.on("error", () => resolve()); child.on("spawn", () => { child.unref();