From 98405e16de73cd7326ad7c167f9397a0e037fcf7 Mon Sep 17 00:00:00 2001 From: Vilmos Nebehaj Date: Sat, 16 May 2026 18:27:16 -0700 Subject: [PATCH 1/2] fix(utils): ignore unsafe dotenv entries --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/utils/src/env.ts | 31 +++++++++++++- packages/utils/src/procmgr.ts | 4 +- packages/utils/test/env.test.ts | 69 ++++++++++++++++++++++++++++++ 4 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 packages/utils/test/env.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index cc9de6d42..775437f92 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/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 diff --git a/packages/utils/src/env.ts b/packages/utils/src/env.ts index f2efbac5f..e05cd5266 100644 --- a/packages/utils/src/env.ts +++ b/packages/utils/src/env.ts @@ -3,13 +3,32 @@ 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_]*$/; + +export function isValidEnvName(name: string): boolean { + return ENV_NAME_RE.test(name); +} + +export function isSafeEnvValue(value: string): boolean { + return !value.includes("\0"); +} + +export function filterProcessEnv(env: Record): Record { + const result: Record = {}; + for (const [key, value] of Object.entries(env)) { + if (!isValidEnvName(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 { +export function parseEnvFile(filePath: string): Record { const result: Record = {}; try { const content = fs.readFileSync(filePath, "utf-8"); @@ -22,12 +41,15 @@ function parseEnvFile(filePath: string): Record { 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,6 +73,13 @@ 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 (!isValidEnvName(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)) { if (!Bun.env[key]) { diff --git a/packages/utils/src/procmgr.ts b/packages/utils/src/procmgr.ts index 04612283b..32b4aef8f 100644 --- a/packages/utils/src/procmgr.ts +++ b/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 { 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", diff --git a/packages/utils/test/env.test.ts b/packages/utils/test/env.test.ts new file mode 100644 index 000000000..e0310d3a8 --- /dev/null +++ b/packages/utils/test/env.test.ts @@ -0,0 +1,69 @@ +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: "", + }); + }); +}); From 4b6be0b0dfc4cfa643e1ee74f8ad016e9009978a Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 17 May 2026 13:43:00 +0200 Subject: [PATCH 2/2] fix(utils): preserve Windows env names with parentheses The Bun.env scrub and filterProcessEnv used isValidEnvName (strict shell identifier shape), which deleted standard Windows variables like ProgramFiles(x86) and CommonProgramFiles(x86). procmgr.ts imports this module before resolving the shell and reads Bun.env['ProgramFiles(x86)'] to find Git Bash under 32-bit Program Files, so installations that only had Git there were no longer discovered and failed with 'No bash shell found'. The unsafe cases for native execve are '=' or NUL in names and NUL in values, not parentheses. Introduce isSafeEnvName covering exactly those cases and use it for the in-place Bun.env scrub and the spawn-env filter. Keep isValidEnvName (strict) for dotenv parsing, where strict shell-identifier shape is the right contract. --- packages/utils/src/env.ts | 28 +++++++++++++++++++++++----- packages/utils/test/env.test.ts | 17 ++++++++++++++++- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/env.ts b/packages/utils/src/env.ts index e05cd5266..ed219cd56 100644 --- a/packages/utils/src/env.ts +++ b/packages/utils/src/env.ts @@ -5,18 +5,36 @@ 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): Record { const result: Record = {}; - for (const [key, value] of Object.entries(env)) { - if (!isValidEnvName(key) || value === undefined || !isSafeEnvValue(value)) continue; + for (const key in env) { + const value = env[key]; + if (!isSafeEnvName(key) || value === undefined || !isSafeEnvValue(value)) continue; result[key] = value; } return result; @@ -75,15 +93,15 @@ const projectEnv = parseEnvFile(path.join(process.cwd(), ".env")); for (const key of Object.keys(Bun.env)) { const value = Bun.env[key]; - if (!isValidEnvName(key) || value === undefined || !isSafeEnvValue(value)) { + 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]; } } } diff --git a/packages/utils/test/env.test.ts b/packages/utils/test/env.test.ts index e0310d3a8..a4997c185 100644 --- a/packages/utils/test/env.test.ts +++ b/packages/utils/test/env.test.ts @@ -57,7 +57,7 @@ describe("filterProcessEnv", () => { filterProcessEnv({ GOOD: "value", EMPTY: "", - "BAD-NAME": "value", + "BAD=NAME": "value", BAD_VALUE: "before\0after", MISSING: undefined, }), @@ -66,4 +66,19 @@ describe("filterProcessEnv", () => { 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", + }); + }); });