From 21708f58ceb0429dbc3fcdb1cd2eefeb4c767e0a Mon Sep 17 00:00:00 2001 From: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> Date: Mon, 2 Mar 2026 10:14:38 -0600 Subject: [PATCH] fix(exec): resolve PATH key case-insensitively for Windows pathPrepend (#25399) (#31879) Co-authored-by: Glucksberg --- src/agents/bash-tools.exec-runtime.ts | 9 +++-- src/agents/bash-tools.test.ts | 55 +++++++++++++++++++++++++++ src/infra/path-prepend.ts | 27 +++++++++++-- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/src/agents/bash-tools.exec-runtime.ts b/src/agents/bash-tools.exec-runtime.ts index 570763daf7e..360912643c0 100644 --- a/src/agents/bash-tools.exec-runtime.ts +++ b/src/agents/bash-tools.exec-runtime.ts @@ -4,12 +4,12 @@ import { Type } from "@sinclair/typebox"; import type { ExecAsk, ExecHost, ExecSecurity } from "../infra/exec-approvals.js"; import { requestHeartbeatNow } from "../infra/heartbeat-wake.js"; import { isDangerousHostEnvVarName } from "../infra/host-env-security.js"; -import { mergePathPrepend } from "../infra/path-prepend.js"; +import { findPathKey, mergePathPrepend } from "../infra/path-prepend.js"; import { enqueueSystemEvent } from "../infra/system-events.js"; import type { ProcessSession } from "./bash-process-registry.js"; import type { ExecToolDetails } from "./bash-tools.exec-types.js"; import type { BashSandboxConfig } from "./bash-tools.shared.js"; -export { applyPathPrepend, normalizePathPrepend } from "../infra/path-prepend.js"; +export { applyPathPrepend, findPathKey, normalizePathPrepend } from "../infra/path-prepend.js"; import { logWarn } from "../logger.js"; import type { ManagedRun } from "../process/supervisor/index.js"; import { getProcessSupervisor } from "../process/supervisor/index.js"; @@ -210,9 +210,10 @@ export function applyShellPath(env: Record, shellPath?: string | if (entries.length === 0) { return; } - const merged = mergePathPrepend(env.PATH, entries); + const pathKey = findPathKey(env); + const merged = mergePathPrepend(env[pathKey], entries); if (merged) { - env.PATH = merged; + env[pathKey] = merged; } } diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index 4841038ff30..d69fdadbe53 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -1,5 +1,6 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { applyPathPrepend, findPathKey } from "../infra/path-prepend.js"; import { peekSystemEvents, resetSystemEventsForTest } from "../infra/system-events.js"; import { captureEnv } from "../test-utils/env.js"; import { getFinishedSession, resetProcessRegistryForTests } from "./bash-process-registry.js"; @@ -547,3 +548,57 @@ describe("exec PATH handling", () => { } }); }); + +describe("findPathKey", () => { + it("returns PATH when key is uppercase", () => { + expect(findPathKey({ PATH: "/usr/bin" })).toBe("PATH"); + }); + + it("returns Path when key is mixed-case (Windows style)", () => { + expect(findPathKey({ Path: "C:\\Windows\\System32" })).toBe("Path"); + }); + + it("returns PATH as default when no PATH-like key exists", () => { + expect(findPathKey({ HOME: "/home/user" })).toBe("PATH"); + }); + + it("prefers uppercase PATH when both PATH and Path exist", () => { + expect(findPathKey({ PATH: "/usr/bin", Path: "C:\\Windows" })).toBe("PATH"); + }); +}); + +describe("applyPathPrepend with case-insensitive PATH key", () => { + it("prepends to Path key on Windows-style env (no uppercase PATH)", () => { + const env: Record = { Path: "C:\\Windows\\System32" }; + applyPathPrepend(env, ["C:\\custom\\bin"]); + // Should write back to the same `Path` key, not create a new `PATH` + expect(env.Path).toContain("C:\\custom\\bin"); + expect(env.Path).toContain("C:\\Windows\\System32"); + expect("PATH" in env).toBe(false); + }); + + it("preserves all existing entries when prepending via Path key", () => { + // Use platform-appropriate paths and delimiters + const delim = path.delimiter; + const existing = isWin + ? ["C:\\Windows\\System32", "C:\\Windows", "C:\\Program Files\\nodejs"] + : ["/usr/bin", "/usr/local/bin", "/opt/node/bin"]; + const prepend = isWin ? ["C:\\custom\\bin"] : ["/custom/bin"]; + const existingPath = existing.join(delim); + const env: Record = { Path: existingPath }; + applyPathPrepend(env, prepend); + const parts = env.Path.split(delim); + expect(parts[0]).toBe(prepend[0]); + for (const entry of existing) { + expect(parts).toContain(entry); + } + }); + + it("respects requireExisting option with Path key", () => { + const env: Record = { HOME: "/home/user" }; + applyPathPrepend(env, ["C:\\custom\\bin"], { requireExisting: true }); + // No Path/PATH key exists, so nothing should be written + expect("PATH" in env).toBe(false); + expect("Path" in env).toBe(false); + }); +}); diff --git a/src/infra/path-prepend.ts b/src/infra/path-prepend.ts index df3e2a5951e..95a261648cf 100644 --- a/src/infra/path-prepend.ts +++ b/src/infra/path-prepend.ts @@ -1,5 +1,22 @@ import path from "node:path"; +/** + * Find the actual key used for PATH in the env object. + * On Windows, `process.env` stores it as `Path` (not `PATH`), + * and after copying to a plain object the original casing is preserved. + */ +export function findPathKey(env: Record): string { + if ("PATH" in env) { + return "PATH"; + } + for (const key of Object.keys(env)) { + if (key.toUpperCase() === "PATH") { + return key; + } + } + return "PATH"; +} + export function normalizePathPrepend(entries?: string[]) { if (!Array.isArray(entries)) { return []; @@ -48,11 +65,15 @@ export function applyPathPrepend( if (!Array.isArray(prepend) || prepend.length === 0) { return; } - if (options?.requireExisting && !env.PATH) { + // On Windows the PATH key may be stored as `Path` (case-insensitive env vars). + // After coercing to a plain object the original casing is preserved, so we must + // look up the actual key to read the existing value and write the merged result back. + const pathKey = findPathKey(env); + if (options?.requireExisting && !env[pathKey]) { return; } - const merged = mergePathPrepend(env.PATH, prepend); + const merged = mergePathPrepend(env[pathKey], prepend); if (merged) { - env.PATH = merged; + env[pathKey] = merged; } }