fix(exec): resolve PATH key case-insensitively for Windows pathPrepend (#25399) (#31879)

Co-authored-by: Glucksberg <markuscontasul@gmail.com>
This commit is contained in:
Tak Hoffman
2026-03-02 10:14:38 -06:00
committed by GitHub
parent 1ea42ebe98
commit 21708f58ce
3 changed files with 84 additions and 7 deletions

View File

@@ -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<string, string>, 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;
}
}

View File

@@ -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<string, string> = { 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<string, string> = { 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<string, string> = { 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);
});
});

View File

@@ -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, string>): 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;
}
}