diff --git a/hooks/session-start-profiler-binary-resolution.test.ts b/hooks/session-start-profiler-binary-resolution.test.ts new file mode 100644 index 0000000..09c50a4 --- /dev/null +++ b/hooks/session-start-profiler-binary-resolution.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, test } from "bun:test"; +import { + binaryNeedsShell, + getBinaryPathCandidates, +} from "./src/session-start-profiler.mts"; + +// npm lays down three entries per global binary in %APPDATA%\npm: +// `vercel` (POSIX sh shim), `vercel.CMD` and `vercel.ps1`. Only the .CMD is +// usable from Node, so candidate ordering decides whether the CLI is found. +const NPM_STYLE_PATHEXT = ".COM;.EXE;.BAT;.CMD;.VBS;.JS;.PS1".split(";"); + +describe("getBinaryPathCandidates", () => { + test("returns the bare name on non-Windows platforms", () => { + expect(getBinaryPathCandidates("vercel", "linux")).toEqual(["vercel"]); + expect(getBinaryPathCandidates("vercel", "darwin")).toEqual(["vercel"]); + }); + + test("prefers a spawnable extension over npm's extensionless POSIX shim", () => { + const candidates = getBinaryPathCandidates("vercel", "win32", NPM_STYLE_PATHEXT); + + expect(candidates.indexOf("vercel.CMD")).toBeLessThan(candidates.indexOf("vercel")); + expect(candidates[candidates.length - 1]).toBe("vercel"); + }); + + test("never ranks a non-spawnable PATHEXT entry above a real executable", () => { + const candidates = getBinaryPathCandidates("vercel", "win32", NPM_STYLE_PATHEXT); + + for (const spawnable of ["vercel.COM", "vercel.EXE", "vercel.BAT", "vercel.CMD"]) { + for (const rejected of ["vercel.VBS", "vercel.JS", "vercel.PS1"]) { + expect(candidates.indexOf(spawnable)).toBeLessThan(candidates.indexOf(rejected)); + } + } + }); + + test("keeps every PATHEXT entry as a candidate", () => { + const candidates = getBinaryPathCandidates("vercel", "win32", NPM_STYLE_PATHEXT); + + expect(candidates.length).toBe(NPM_STYLE_PATHEXT.length + 1); + for (const extension of NPM_STYLE_PATHEXT) { + expect(candidates).toContain(`vercel${extension}`); + } + }); + + test("does not append extensions to an already-qualified name", () => { + expect(getBinaryPathCandidates("vercel.cmd", "win32", NPM_STYLE_PATHEXT)).toEqual([ + "vercel.cmd", + ]); + }); +}); + +describe("binaryNeedsShell", () => { + test("requires a shell for Windows batch wrappers", () => { + // Node rejects these with EINVAL when spawned directly (CVE-2024-27980 fix). + expect(binaryNeedsShell("C:\\npm\\vercel.CMD", "win32")).toBe(true); + expect(binaryNeedsShell("C:\\npm\\vercel.bat", "win32")).toBe(true); + }); + + test("spawns real executables directly", () => { + expect(binaryNeedsShell("C:\\tools\\vercel.exe", "win32")).toBe(false); + }); + + test("never asks for a shell off Windows", () => { + expect(binaryNeedsShell("/usr/local/bin/vercel", "linux")).toBe(false); + expect(binaryNeedsShell("/usr/local/bin/weird.cmd", "darwin")).toBe(false); + }); +}); diff --git a/hooks/session-start-profiler.mjs b/hooks/session-start-profiler.mjs index 8eac1c7..04c1a49 100644 --- a/hooks/session-start-profiler.mjs +++ b/hooks/session-start-profiler.mjs @@ -565,14 +565,38 @@ var SPAWN_STDIO = "ignore pipe ignore".split(" "); var EXEC_SYNC_TIMEOUT_MS = 3e3; var NUMERIC_VERSION_RE = /\d+(?:\.\d+)*/; var WINDOWS_EXECUTABLE_EXTENSIONS = (process.env.PATHEXT || ".EXE;.CMD;.BAT;.COM").split(";").filter(Boolean); -function getBinaryPathCandidates(binaryName) { - if (process.platform !== "win32") { +var WINDOWS_SPAWNABLE_EXTENSIONS = ".EXE;.COM;.CMD;.BAT".split(";"); +var WINDOWS_SHELL_SCRIPT_RE = /\.(?:cmd|bat)$/i; +function getBinaryPathCandidates(binaryName, platform = process.platform, pathExtensions = WINDOWS_EXECUTABLE_EXTENSIONS) { + if (platform !== "win32") { return [binaryName]; } const hasExecutableExtension = /\.[^./\\]+$/.test(binaryName); - const suffixes = hasExecutableExtension ? [""] : ["", ...WINDOWS_EXECUTABLE_EXTENSIONS]; + if (hasExecutableExtension) { + return [binaryName]; + } + const isSpawnable = (extension) => WINDOWS_SPAWNABLE_EXTENSIONS.includes(extension.toUpperCase()); + const suffixes = [ + ...pathExtensions.filter(isSpawnable), + ...pathExtensions.filter((extension) => !isSpawnable(extension)), + "" + ]; return suffixes.map((suffix) => `${binaryName}${suffix}`); } +function binaryNeedsShell(binaryPath, platform = process.platform) { + return platform === "win32" && WINDOWS_SHELL_SCRIPT_RE.test(binaryPath); +} +function runBinarySync(binaryPath, args) { + const needsShell = binaryNeedsShell(binaryPath); + const command = needsShell ? `"${binaryPath}"` : binaryPath; + return execFileSync(command, args, { + timeout: EXEC_SYNC_TIMEOUT_MS, + encoding: "utf-8", + stdio: SPAWN_STDIO, + shell: needsShell, + windowsHide: true + }).trim(); +} function resolveBinaryFromPath(binaryName) { try { const pathEntries = (process.env.PATH || "").split(delimiter).filter(Boolean); @@ -629,11 +653,7 @@ function checkVercelCli() { } let currentVersion; try { - const raw = execFileSync(vercelBinary, VERCEL_VERSION_ARGS, { - timeout: EXEC_SYNC_TIMEOUT_MS, - encoding: "utf-8", - stdio: SPAWN_STDIO - }).trim(); + const raw = runBinarySync(vercelBinary, VERCEL_VERSION_ARGS); const lines = raw.split("\n").map((l) => l.trim()).filter(Boolean); currentVersion = lines[lines.length - 1]; } catch (error) { @@ -641,7 +661,7 @@ function checkVercelCli() { command: vercelBinary, args: VERCEL_VERSION_ARGS.join(" ") }); - return { installed: false, needsUpdate: false }; + return { installed: true, needsUpdate: false }; } const npmBinary = resolveBinaryFromPath("npm"); if (!npmBinary) { @@ -649,12 +669,7 @@ function checkVercelCli() { } let latestVersion; try { - const raw = execFileSync(npmBinary, NPM_VIEW_ARGS, { - timeout: EXEC_SYNC_TIMEOUT_MS, - encoding: "utf-8", - stdio: SPAWN_STDIO - }).trim(); - latestVersion = raw; + latestVersion = runBinarySync(npmBinary, NPM_VIEW_ARGS); } catch (error) { logCaughtError(log, "session-start-profiler:npm-latest-version-check-failed", error, { command: npmBinary, @@ -888,12 +903,14 @@ if (isSessionStartProfilerEntrypoint) { main(); } export { + binaryNeedsShell, buildSessionStartProfilerEnvVars, buildSessionStartProfilerUserMessages, checkGreenfield, detectAgentHarness, detectSessionStartPlatform, formatSessionStartProfilerCursorOutput, + getBinaryPathCandidates, logBrokenSkillFrontmatterSummary, normalizeDetectedAgentHarness, normalizeSessionStartSessionId, diff --git a/hooks/src/session-start-profiler.mts b/hooks/src/session-start-profiler.mts index ba6b892..f36f7c0 100644 --- a/hooks/src/session-start-profiler.mts +++ b/hooks/src/session-start-profiler.mts @@ -339,16 +339,64 @@ const WINDOWS_EXECUTABLE_EXTENSIONS = (process.env.PATHEXT || ".EXE;.CMD;.BAT;.C .split(";") .filter(Boolean); -function getBinaryPathCandidates(binaryName: string): string[] { - if (process.platform !== "win32") { +// Extensions Node can hand to CreateProcess — directly, or through the shell for +// .cmd/.bat. Other PATHEXT entries (.PS1, .PY, .JS, ...) resolve to files +// spawnSync rejects with EFTYPE, so they must never outrank a real executable. +const WINDOWS_SPAWNABLE_EXTENSIONS: string[] = ".EXE;.COM;.CMD;.BAT".split(";"); +const WINDOWS_SHELL_SCRIPT_RE = /\.(?:cmd|bat)$/i; + +export function getBinaryPathCandidates( + binaryName: string, + platform: string = process.platform, + pathExtensions: string[] = WINDOWS_EXECUTABLE_EXTENSIONS, +): string[] { + if (platform !== "win32") { return [binaryName]; } const hasExecutableExtension = /\.[^./\\]+$/.test(binaryName); - const suffixes = hasExecutableExtension ? [""] : ["", ...WINDOWS_EXECUTABLE_EXTENSIONS]; + if (hasExecutableExtension) { + return [binaryName]; + } + + const isSpawnable = (extension: string): boolean => + WINDOWS_SPAWNABLE_EXTENSIONS.includes(extension.toUpperCase()); + // The bare name goes last. On Windows an extensionless entry sitting next to a + // .cmd is npm's POSIX shim — an sh script spawnSync fails on with ENOENT. + const suffixes = [ + ...pathExtensions.filter(isSpawnable), + ...pathExtensions.filter((extension: string) => !isSpawnable(extension)), + "", + ]; return suffixes.map((suffix: string) => `${binaryName}${suffix}`); } +/** + * Windows batch wrappers cannot be spawned directly: since the fix for + * CVE-2024-27980, Node rejects .cmd/.bat without `shell: true` (EINVAL). + */ +export function binaryNeedsShell( + binaryPath: string, + platform: string = process.platform, +): boolean { + return platform === "win32" && WINDOWS_SHELL_SCRIPT_RE.test(binaryPath); +} + +/** Run a resolved binary and return its trimmed stdout. */ +function runBinarySync(binaryPath: string, args: string[]): string { + const needsShell = binaryNeedsShell(binaryPath); + // Under `shell: true` the command is re-parsed by cmd.exe, which would + // otherwise split an unquoted path on its spaces. + const command = needsShell ? `"${binaryPath}"` : binaryPath; + return execFileSync(command, args, { + timeout: EXEC_SYNC_TIMEOUT_MS, + encoding: "utf-8", + stdio: SPAWN_STDIO, + shell: needsShell, + windowsHide: true, + }).trim(); +} + function resolveBinaryFromPath(binaryName: string): string | null { try { const pathEntries = (process.env.PATH || "").split(delimiter).filter(Boolean); @@ -422,11 +470,7 @@ function checkVercelCli(): VercelCliStatus { // 1. Check if vercel is installed let currentVersion: string | undefined; try { - const raw: string = execFileSync(vercelBinary, VERCEL_VERSION_ARGS, { - timeout: EXEC_SYNC_TIMEOUT_MS, - encoding: "utf-8", - stdio: SPAWN_STDIO, - }).trim(); + const raw: string = runBinarySync(vercelBinary, VERCEL_VERSION_ARGS); // Output may include extra lines; version is typically last non-empty line const lines: string[] = raw.split("\n").map((l: string) => l.trim()).filter(Boolean); currentVersion = lines[lines.length - 1]; @@ -435,7 +479,9 @@ function checkVercelCli(): VercelCliStatus { command: vercelBinary, args: VERCEL_VERSION_ARGS.join(" "), }); - return { installed: false, needsUpdate: false }; + // The binary is on PATH — only the version probe failed. Reporting "not + // installed" here would tell the user to install a CLI they already have. + return { installed: true, needsUpdate: false }; } const npmBinary = resolveBinaryFromPath("npm"); @@ -446,12 +492,7 @@ function checkVercelCli(): VercelCliStatus { // 2. Fetch latest version from npm registry let latestVersion: string | undefined; try { - const raw: string = execFileSync(npmBinary, NPM_VIEW_ARGS, { - timeout: EXEC_SYNC_TIMEOUT_MS, - encoding: "utf-8", - stdio: SPAWN_STDIO, - }).trim(); - latestVersion = raw; + latestVersion = runBinarySync(npmBinary, NPM_VIEW_ARGS); } catch (error) { logCaughtError(log, "session-start-profiler:npm-latest-version-check-failed", error, { command: npmBinary,