Merge pull request #1129 from ldx/fix/env-file-validation

fix(utils): ignore unsafe dotenv entries

packages/coding-agent/CHANGELOG.md

@@ -53,6 +53,10 @@
 - Fixed auth-gateway request cancellation for requests that are already aborted before dispatch.
 - Fixed `/login` and `/logout` provider selector overflowing tall provider lists off-screen on small terminals. The selector now scrolls a 10-item window centered on the highlighted entry, shows a `(n/total)` indicator when windowed, and accepts PageUp/PageDown for faster navigation.
 
+### Fixed
+
+- Fixed `.env` loading so malformed variable names and NUL-containing values are ignored before they can poison `Bun.env` and break bash/external process execution with `nul byte found in provided data`.
+
 ## [15.1.2] - 2026-05-15
 ### Fixed
 

packages/utils/src/env.ts

@@ -3,13 +3,50 @@ import * as os from "node:os";
 import * as path from "node:path";
 import { getAgentDir, getConfigRootDir } from "./dirs";
 
+const ENV_NAME_RE = /^[A-Za-z_][A-Za-z0-9_]*$/;
+
+/**
+ * Strict shell-identifier shape. Used for dotenv keys we accept into
+ * `Bun.env` — those should be referenceable as `$NAME` from POSIX shells,
+ * so we reject anything outside `[A-Za-z_][A-Za-z0-9_]*`.
+ */
+export function isValidEnvName(name: string): boolean {
+	return ENV_NAME_RE.test(name);
+}
+
+/**
+ * The only names that are genuinely unsafe to forward to a native `execve`
+ * spawn: empty, containing `=` (would corrupt the `KEY=VALUE` framing) or
+ * NUL (terminates the C string mid-entry). Windows ships standard variables
+ * whose names contain parentheses (e.g. `ProgramFiles(x86)`, `CommonProgramFiles(x86)`)
+ * — those MUST survive the scrub so downstream resolvers (Git Bash discovery
+ * in `procmgr.ts`, etc.) can still read them.
+ */
+export function isSafeEnvName(name: string): boolean {
+	return name.length > 0 && !name.includes("=") && !name.includes("\0");
+}
+
+export function isSafeEnvValue(value: string): boolean {
+	return !value.includes("\0");
+}
+
+export function filterProcessEnv(env: Record<string, string | undefined>): Record<string, string> {
+	const result: Record<string, string> = {};
+	for (const key in env) {
+		const value = env[key];
+		if (!isSafeEnvName(key) || value === undefined || !isSafeEnvValue(value)) continue;
+		result[key] = value;
+	}
+	return result;
+}
+
 /**
  * Parses a .env file synchronously and extracts key-value string pairs.
  * Ignores lines that are empty or start with '#'. Trims whitespace.
  * Allows values to be quoted with single or double quotes.
  * Returns an object of key-value pairs.
  */
-function parseEnvFile(filePath: string): Record<string, string> {
+export function parseEnvFile(filePath: string): Record<string, string> {
 	const result: Record<string, string> = {};
 	try {
 		const content = fs.readFileSync(filePath, "utf-8");
@@ -22,12 +59,15 @@ function parseEnvFile(filePath: string): Record<string, string> {
 			if (eqIndex === -1) continue;
 
 			const key = trimmed.slice(0, eqIndex).trim();
+			if (!isValidEnvName(key)) continue;
+
 			let value = trimmed.slice(eqIndex + 1).trim();
 
 			// Remove surrounding quotes (" or ')
 			if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
 				value = value.slice(1, -1);
 			}
+			if (!isSafeEnvValue(value)) continue;
 
 			result[key] = value;
 		}
@@ -51,10 +91,17 @@ const piEnv = parseEnvFile(path.join(getConfigRootDir(), ".env"));
 const agentEnv = parseEnvFile(path.join(getAgentDir(), ".env"));
 const projectEnv = parseEnvFile(path.join(process.cwd(), ".env"));
 
+for (const key of Object.keys(Bun.env)) {
+	const value = Bun.env[key];
+	if (!isSafeEnvName(key) || value === undefined || !isSafeEnvValue(value)) {
+		delete Bun.env[key];
+	}
+}
+
 for (const file of [projectEnv, agentEnv, piEnv, homeEnv]) {
-	for (const [key, value] of Object.entries(file)) {
+	for (const key in file) {
 		if (!Bun.env[key]) {
-			Bun.env[key] = value;
+			Bun.env[key] = file[key];
 		}
 	}
 }

packages/utils/src/procmgr.ts

@@ -2,7 +2,7 @@ import * as fs from "node:fs";
 import * as path from "node:path";
 import { Process, ProcessStatus } from "@oh-my-pi/pi-natives";
 import type { Subprocess } from "bun";
-import { $env } from "./env";
+import { $env, filterProcessEnv } from "./env";
 import { $which } from "./which";
 
 export interface ShellConfig {
@@ -45,7 +45,7 @@ function isExecutable(path: string): boolean {
 function buildSpawnEnv(shell: string): Record<string, string> {
 	const noCI = $env.PI_BASH_NO_CI || $env.CLAUDE_BASH_NO_CI;
 	return {
-		...Bun.env,
+		...filterProcessEnv(Bun.env),
 		SHELL: shell,
 		GIT_EDITOR: "true",
 		GPG_TTY: "not a tty",

packages/utils/test/env.test.ts

@@ -0,0 +1,84 @@
+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, parseEnvFile } from "../src/env";
+
+const tempDirs: string[] = [];
+
+afterEach(() => {
+	for (const dir of tempDirs.splice(0)) {
+		fs.rmSync(dir, { force: true, recursive: true });
+	}
+});
+
+function writeTempEnv(content: string): string {
+	const dir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-utils-env-"));
+	tempDirs.push(dir);
+	const filePath = path.join(dir, ".env");
+	fs.writeFileSync(filePath, content);
+	return filePath;
+}
+
+describe("parseEnvFile", () => {
+	it("ignores malformed names and nul-containing values", () => {
+		const filePath = writeTempEnv(
+			[
+				"GOOD=value",
+				"_ALSO_GOOD='quoted value'",
+				"1BAD=value",
+				"BAD-NAME=value",
+				"BAD NAME=value",
+				"BAD_VALUE=before\0after",
+				"# comment",
+				"NO_EQUALS",
+			].join("\n"),
+		);
+
+		expect(parseEnvFile(filePath)).toEqual({
+			GOOD: "value",
+			_ALSO_GOOD: "quoted value",
+		});
+	});
+
+	it("mirrors valid OMP_ variables to PI_ variables", () => {
+		const filePath = writeTempEnv("OMP_FEATURE=enabled\nOMP_BAD=before\0after\n");
+
+		expect(parseEnvFile(filePath)).toEqual({
+			OMP_FEATURE: "enabled",
+			PI_FEATURE: "enabled",
+		});
+	});
+});
+
+describe("filterProcessEnv", () => {
+	it("drops entries that cannot be passed to process spawn env", () => {
+		expect(
+			filterProcessEnv({
+				GOOD: "value",
+				EMPTY: "",
+				"BAD=NAME": "value",
+				BAD_VALUE: "before\0after",
+				MISSING: undefined,
+			}),
+		).toEqual({
+			GOOD: "value",
+			EMPTY: "",
+		});
+	});
+
+	it("preserves Windows-style variable names containing parentheses", () => {
+		// `ProgramFiles(x86)` and friends are standard on Windows and must
+		// survive the scrub so Git Bash discovery in procmgr.ts can resolve
+		// 32-bit Program Files installations.
+		expect(
+			filterProcessEnv({
+				"ProgramFiles(x86)": "C:\\Program Files (x86)",
+				"CommonProgramFiles(x86)": "C:\\Program Files (x86)\\Common Files",
+			}),
+		).toEqual({
+			"ProgramFiles(x86)": "C:\\Program Files (x86)",
+			"CommonProgramFiles(x86)": "C:\\Program Files (x86)\\Common Files",
+		});
+	});
+});