Merge PR #7362: fix(auth): guard config-value resolvers against case-insensitive env hijack (@roboomp)

2026-08-02 20:53:20 +02:00
parent bdc2072440 a6521f07ed
commit 3f61117994
6 changed files with 105 additions and 6 deletions
@@ -12,7 +12,7 @@ import { createHash } from "node:crypto";
 import * as fs from "node:fs/promises";
 import * as path from "node:path";
 import { parseAlibabaTokenPlanCredential } from "@oh-my-pi/pi-catalog/wire/alibaba-token-plan";
-import { $env, getAgentDbPath, getDbBusyTimeoutMs, logger } from "@oh-my-pi/pi-utils";
+import { $env, $envExact, getAgentDbPath, getDbBusyTimeoutMs, logger } from "@oh-my-pi/pi-utils";
 import type { ApiKeyResolver } from "./auth-retry";
 import * as AIError from "./error";
 import { isUsageLimitOutcome } from "./error/rate-limit";
@@ -643,7 +643,7 @@ export type AuthStorageOptions = {
 * Does NOT support "!command" syntax (that requires pi-natives).
 */
 async function defaultConfigValueResolver(config: string): Promise<string | undefined> {
-	const envValue = process.env[config];
+	const envValue = $envExact(config);
 	return envValue || config;
 }

@@ -50,6 +50,9 @@
 ### Fixed

 - Fixed headless `-p` / `--mode json` runs with `memory.backend: mnemopi` hanging after a completed turn and leaving `__omp_worker_mnemopi_embed` unreaped when the embed worker's fastembed/onnxruntime runtime wedged. Steady-state embed requests were unbounded, so a stuck native runtime blocked the turn's memory recall or shutdown consolidation forever; embeds are now bounded and the wedged worker is reaped on timeout so the next call respawns a fresh child, while initialization remains unbounded so first-time runtime installation and model bootstrap are not killed mid-install (regression of [#5753](https://github.com/can1357/oh-my-pi/issues/5753); [#7352](https://github.com/can1357/oh-my-pi/issues/7352)).
+### Fixed
+
+- Fixed a literal API key configured via `/login` (e.g. OpenCode Zen's free `public` key) being hijacked on Windows by a case-differing system environment variable, causing 401s. `process.env`/`Bun.env` reads are case-insensitive on Windows, so the config-value resolvers' "env var name, else literal" fallback resolved `public` to the built-in `PUBLIC=C:\Users\Public`. Resolution now requires an exact-case env entry (via the new `$envExact` helper) before treating a value as an env-var reference ([#7361](https://github.com/can1357/oh-my-pi/issues/7361)).

 ## [17.2.4] - 2026-08-01

@@ -73,7 +73,7 @@ import {
 	inheritReferenceThinking,
 	resolveModelReference,
 } from "@oh-my-pi/pi-catalog/identity";
-import { isBunTestRuntime, isRecord, logger, wrapFetchForExtraCa } from "@oh-my-pi/pi-utils";
+import { $envExact, isBunTestRuntime, isRecord, logger, wrapFetchForExtraCa } from "@oh-my-pi/pi-utils";
 import { parseModelString, resolveProviderModelReference } from "../config/model-resolver";
 import { generateCodexAttestation } from "../live/attestation";
 import type { AuthStorage, OAuthCredential } from "../session/auth-storage";
@@ -329,7 +329,7 @@ interface CommandApiKeyResolution {
 */
 function resolveConfigValue(valueConfig: string): string | undefined {
 	if (valueConfig.startsWith("!")) return resolveCommandConfig(valueConfig.slice(1).trim());
-	const envValue = Bun.env[valueConfig];
+	const envValue = $envExact(valueConfig);
 	if (envValue) return envValue;
 	return valueConfig;
 }
@@ -5,6 +5,7 @@
 */

 import { executeShell } from "@oh-my-pi/pi-natives";
+import { $envExact } from "@oh-my-pi/pi-utils";

 /** Cache for successful shell command results (persists for process lifetime). */
 const commandResultCache = new Map<string, string>();
@@ -21,7 +22,7 @@ export async function resolveConfigValue(config: string): Promise<string | undef
 	if (config.startsWith("!")) {
 		return await executeCommand(config);
 	}
-	const envValue = process.env[config];
+	const envValue = $envExact(config);
 	return envValue || config;
 }

@@ -246,6 +246,35 @@ export function $pickenv(...keys: string[]): string | undefined {
 	return undefined;
 }

+/**
+ * Read an environment variable by its EXACT, case-sensitive name.
+ *
+ * `process.env` / `Bun.env` lookups are case-insensitive on Windows (Node backs
+ * them with `uv_os_getenv`, Bun with a `CaseInsensitiveASCIIStringArrayHashMap`),
+ * so a lowercase literal like `public` silently resolves to a differently-cased
+ * system variable — Windows ships `PUBLIC=C:\Users\Public`. Enumerated keys are
+ * the only signal that preserves the real casing, so this trusts the lookup only
+ * when a key with identical casing is actually present. On POSIX (case-sensitive
+ * env) it is equivalent to a direct lookup.
+ *
+ * Use this instead of `process.env[name] ?? literal` wherever `name` may be a
+ * user-supplied literal (e.g. a stored API key) rather than a genuine env-var
+ * reference — otherwise the literal gets hijacked by a same-named system var.
+ *
+ * @param name - Environment variable name to look up.
+ * @param env - Environment source; defaults to `process.env`.
+ */
+export function $envExact(name: string, env: Record<string, string | undefined> = process.env): string | undefined {
+	const value = env[name];
+	if (value === undefined) return undefined;
+	// Enumeration preserves real key casing on Windows, unlike the getter; the
+	// value is trusted only when an exact-case entry actually exists.
+	for (const key in env) {
+		if (key === name) return value;
+	}
+	return undefined;
+}
+
 /**
 * Parses a positive decimal integer from `$env[name]`.
 * Empty, invalid, NaN, zero, or negative values return `defaultValue`.
@@ -2,7 +2,13 @@ import { afterEach, describe, expect, it } from "bun:test";
 import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
-import { filterProcessEnv, getDbBusyTimeoutMs, parseEnvFile, setInteractiveHost } from "@oh-my-pi/pi-utils/env";
+import {
+	$envExact,
+	filterProcessEnv,
+	getDbBusyTimeoutMs,
+	parseEnvFile,
+	setInteractiveHost,
+} from "@oh-my-pi/pi-utils/env";

 const tempDirs: string[] = [];
 const runtimeProbePath = path.join(import.meta.dir, "fixtures", "test-runtime-probe.ts");
@@ -188,3 +194,63 @@ describe("isBunTestRuntime", () => {
 		).toBe(true);
 	});
 });
+
+/**
+ * Faithful model of Windows `process.env`: case-insensitive reads, but
+ * enumeration (`ownKeys`) preserves the real key casing — exactly Node
+ * (`uv_os_getenv` + `uv_os_environ`) and Bun (`CaseInsensitiveASCIIStringArrayHashMap`).
+ */
+function windowsLikeEnv(backing: Record<string, string>): Record<string, string | undefined> {
+	return new Proxy(backing, {
+		get(target, prop) {
+			if (typeof prop !== "string") return Reflect.get(target, prop);
+			for (const key in target) {
+				if (key.toLowerCase() === prop.toLowerCase()) return target[key];
+			}
+			return undefined;
+		},
+		has(target, prop) {
+			if (typeof prop !== "string") return Reflect.has(target, prop);
+			for (const key in target) {
+				if (key.toLowerCase() === prop.toLowerCase()) return true;
+			}
+			return false;
+		},
+	}) as Record<string, string | undefined>;
+}
+
+describe("$envExact", () => {
+	it("returns the value for an exact-case key", () => {
+		const env = { OPENCODE_API_KEY: "sk-live", PATH: "/usr/bin" };
+		expect($envExact("OPENCODE_API_KEY", env)).toBe("sk-live");
+	});
+
+	it("returns undefined for an absent name", () => {
+		expect($envExact("MISSING_VAR", { PATH: "/usr/bin" })).toBeUndefined();
+	});
+
+	it("does not hijack a literal via a case-differing Windows system var", () => {
+		// Windows ships PUBLIC=C:\Users\Public and reads are case-insensitive, so
+		// a bare `env["public"]` returns it — the /login #7361 401 root cause.
+		const env = windowsLikeEnv({ PUBLIC: "C:\\Users\\Public" });
+		expect(env.public).toBe("C:\\Users\\Public");
+		expect($envExact("public", env)).toBeUndefined();
+	});
+
+	it("still resolves a genuine exact-case reference on a case-insensitive env", () => {
+		const env = windowsLikeEnv({ MY_KEY: "secret" });
+		expect($envExact("MY_KEY", env)).toBe("secret");
+		expect($envExact("my_key", env)).toBeUndefined();
+	});
+
+	it("reads process.env by default", () => {
+		const name = `PI_ENVEXACT_TEST_${Date.now()}`;
+		process.env[name] = "value";
+		try {
+			expect($envExact(name)).toBe("value");
+		} finally {
+			delete process.env[name];
+		}
+		expect($envExact(name)).toBeUndefined();
+	});
+});