From 15df680a64b56ac8190b5cacb71b3e99434c177c Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 27 Apr 2026 13:24:16 +0800 Subject: [PATCH] [codex] Centralize subprocess env boundaries (#585) * Centralize subprocess env boundaries * Remove env boundary static audit test * Tighten env boundary process launches * Fix Windows native process launches * Inline provider env pass-throughs --- .../cli/src/commands/daemon/local-daemon.ts | 11 +- packages/cli/src/commands/daemon/status.ts | 9 +- packages/cli/src/commands/open.ts | 6 +- packages/desktop/bin/paseo | 2 +- packages/desktop/bin/paseo.cmd | 1 + packages/desktop/src/daemon/daemon-manager.ts | 4 +- .../daemon/node-entrypoint-launcher.test.ts | 7 +- .../src/daemon/node-entrypoint-launcher.ts | 3 +- packages/desktop/src/daemon/runtime-paths.ts | 1 + packages/desktop/src/login-shell-env.ts | 7 +- packages/server/package.json | 6 +- .../agent/provider-launch-config.test.ts | 10 +- .../server/agent/provider-launch-config.ts | 49 +++++-- .../src/server/agent/providers/acp-agent.ts | 35 ++--- .../agent/providers/claude-agent.env.test.ts | 115 ++++++++------- .../providers/claude-agent.spawn.test.ts | 11 +- .../server/agent/providers/claude-agent.ts | 48 +++++-- .../agent/providers/codex-app-server-agent.ts | 22 +-- .../agent/providers/diagnostic-utils.ts | 7 +- .../server/agent/providers/opencode-agent.ts | 4 +- .../server/src/server/checkout-git-utils.ts | 7 +- packages/server/src/server/config.ts | 3 +- .../server/src/server/editor-targets.test.ts | 1 + packages/server/src/server/editor-targets.ts | 19 ++- packages/server/src/server/exports.ts | 3 +- packages/server/src/server/paseo-env.test.ts | 115 +++++++++++++++ packages/server/src/server/paseo-env.ts | 98 +++++++++++++ packages/server/src/server/session.ts | 8 +- .../local/sherpa/model-downloader.ts | 6 +- .../local/sherpa/sherpa-onnx-node-loader.ts | 5 + .../src/server/workspace-git-service.ts | 11 +- .../server/src/services/github-service.ts | 7 +- packages/server/src/terminal/terminal.ts | 7 +- packages/server/src/utils/checkout-git.ts | 71 +++++----- packages/server/src/utils/executable.ts | 50 +------ packages/server/src/utils/run-git-command.ts | 19 ++- packages/server/src/utils/spawn.test.ts | 131 +++++++++++++++++- packages/server/src/utils/spawn.ts | 68 +++++++-- packages/server/src/utils/windows-command.ts | 42 ++++++ packages/server/src/utils/worktree.ts | 35 +++-- 40 files changed, 760 insertions(+), 304 deletions(-) create mode 100644 packages/server/src/server/paseo-env.test.ts create mode 100644 packages/server/src/server/paseo-env.ts create mode 100644 packages/server/src/utils/windows-command.ts diff --git a/packages/cli/src/commands/daemon/local-daemon.ts b/packages/cli/src/commands/daemon/local-daemon.ts index 3174ceee8..ad36e066f 100644 --- a/packages/cli/src/commands/daemon/local-daemon.ts +++ b/packages/cli/src/commands/daemon/local-daemon.ts @@ -1,8 +1,8 @@ -import { spawn, spawnSync } from "node:child_process"; +import { spawnSync } from "node:child_process"; import { existsSync, readFileSync } from "node:fs"; import { createRequire } from "node:module"; import path from "node:path"; -import { loadConfig, resolvePaseoHome } from "@getpaseo/server"; +import { loadConfig, resolvePaseoHome, spawnProcess } from "@getpaseo/server"; import { tryConnectToDaemon } from "../../utils/client.js"; export interface DaemonStartOptions { @@ -373,16 +373,17 @@ export async function startLocalDaemonDetached( throw new Error("Cannot use --listen and --port together"); } + const daemonRunnerEntry = resolveDaemonRunnerEntry(); const childEnv = buildChildEnv(options); const paseoHome = resolvePaseoHome(childEnv); const logPath = path.join(paseoHome, DAEMON_LOG_FILENAME); - const daemonRunnerEntry = resolveDaemonRunnerEntry(); - const child = spawn( + const child = spawnProcess( process.execPath, [...process.execArgv, daemonRunnerEntry, ...buildRunnerArgs(options)], { detached: true, + envMode: "internal", env: childEnv, stdio: ["ignore", "ignore", "ignore"], }, @@ -438,8 +439,8 @@ export function startLocalDaemonForeground(options: DaemonStartOptions): number throw new Error("Cannot use --listen and --port together"); } - const childEnv = buildChildEnv(options); const daemonRunnerEntry = resolveDaemonRunnerEntry(); + const childEnv = buildChildEnv(options); const result = spawnSync( process.execPath, [...process.execArgv, daemonRunnerEntry, ...buildRunnerArgs(options)], diff --git a/packages/cli/src/commands/daemon/status.ts b/packages/cli/src/commands/daemon/status.ts index 141915fc7..aa0d05e36 100644 --- a/packages/cli/src/commands/daemon/status.ts +++ b/packages/cli/src/commands/daemon/status.ts @@ -1,11 +1,6 @@ import type { Command } from "commander"; import { createRequire } from "node:module"; -import { - getOrCreateServerId, - findExecutable, - applyProviderEnv, - execCommand, -} from "@getpaseo/server"; +import { getOrCreateServerId, findExecutable, execCommand } from "@getpaseo/server"; import { tryConnectToDaemon } from "../../utils/client.js"; import type { CommandOptions, ListResult, OutputSchema } from "../../output/index.js"; import { resolveLocalDaemonState, resolveTcpHostFromListen } from "./local-daemon.js"; @@ -177,11 +172,9 @@ async function checkProviderBinary( if (!binaryPath) { return { path: null, version: null }; } - const env = applyProviderEnv(process.env); try { const { stdout } = await execCommand(binaryPath, ["--version"], { timeout: 5000, - env, }); return { path: binaryPath, version: stdout.trim() || null }; } catch { diff --git a/packages/cli/src/commands/open.ts b/packages/cli/src/commands/open.ts index ae5d64c6a..87ff16c9d 100644 --- a/packages/cli/src/commands/open.ts +++ b/packages/cli/src/commands/open.ts @@ -1,7 +1,7 @@ import { existsSync } from "node:fs"; -import { spawn } from "node:child_process"; import { homedir } from "node:os"; import path from "node:path"; +import { spawnProcess } from "@getpaseo/server"; function findDesktopApp(): string | null { if (process.platform === "darwin") { @@ -54,11 +54,13 @@ function cleanEnvForDesktopLaunch(): NodeJS.ProcessEnv { // desktop process inherits the env directly, so we must strip it or the // desktop app would start as a bare Node process instead of Electron. delete env.ELECTRON_RUN_AS_NODE; + delete env.ELECTRON_NO_ATTACH_CONSOLE; + delete env.PASEO_NODE_ENV; return env; } function spawnDetached(command: string, args: string[]): void { - spawn(command, args, { + spawnProcess(command, args, { detached: true, stdio: "ignore", env: cleanEnvForDesktopLaunch(), diff --git a/packages/desktop/bin/paseo b/packages/desktop/bin/paseo index 9e3e497f4..4c3e6329c 100755 --- a/packages/desktop/bin/paseo +++ b/packages/desktop/bin/paseo @@ -38,4 +38,4 @@ fi RUNNER_PATH="${RESOURCES_DIR}/app.asar.unpacked/dist/daemon/node-entrypoint-runner.js" CLI_ENTRYPOINT="${RESOURCES_DIR}/app.asar/node_modules/@getpaseo/cli/dist/index.js" -exec env ELECTRON_RUN_AS_NODE=1 "${APP_EXECUTABLE}" --disable-warning=DEP0040 "${RUNNER_PATH}" node-script "${CLI_ENTRYPOINT}" "$@" +exec env ELECTRON_RUN_AS_NODE=1 PASEO_NODE_ENV=production "${APP_EXECUTABLE}" --disable-warning=DEP0040 "${RUNNER_PATH}" node-script "${CLI_ENTRYPOINT}" "$@" diff --git a/packages/desktop/bin/paseo.cmd b/packages/desktop/bin/paseo.cmd index 6c7cddd9a..c0c3c64c2 100644 --- a/packages/desktop/bin/paseo.cmd +++ b/packages/desktop/bin/paseo.cmd @@ -10,5 +10,6 @@ if not exist "%APP_EXECUTABLE%" ( ) set "ELECTRON_RUN_AS_NODE=1" +set "PASEO_NODE_ENV=production" "%APP_EXECUTABLE%" --disable-warning=DEP0040 "%RESOURCES_DIR%\app.asar.unpacked\dist\daemon\node-entrypoint-runner.js" node-script "%RESOURCES_DIR%\app.asar\node_modules\@getpaseo\cli\dist\index.js" %* exit /b %errorlevel% diff --git a/packages/desktop/src/daemon/daemon-manager.ts b/packages/desktop/src/daemon/daemon-manager.ts index c4da31f85..f8967aa02 100644 --- a/packages/desktop/src/daemon/daemon-manager.ts +++ b/packages/desktop/src/daemon/daemon-manager.ts @@ -359,7 +359,9 @@ async function startDaemon(): Promise { const child: ChildProcess = spawnProcess(invocation.command, invocation.args, { detached: true, - env: { ...invocation.env, PASEO_DESKTOP_MANAGED: "1" }, + envMode: "internal", + env: invocation.env, + envOverlay: { PASEO_DESKTOP_MANAGED: "1" }, stdio: ["ignore", "pipe", "pipe"], }); diff --git a/packages/desktop/src/daemon/node-entrypoint-launcher.test.ts b/packages/desktop/src/daemon/node-entrypoint-launcher.test.ts index 7c34a1f4e..30b58b924 100644 --- a/packages/desktop/src/daemon/node-entrypoint-launcher.test.ts +++ b/packages/desktop/src/daemon/node-entrypoint-launcher.test.ts @@ -109,7 +109,7 @@ describe("node-entrypoint-launcher", () => { env: { PATH: "/usr/bin", ELECTRON_RUN_AS_NODE: "1", - NODE_ENV: "production", + PASEO_NODE_ENV: "production", }, }); }); @@ -150,7 +150,8 @@ describe("node-entrypoint-launcher", () => { ).toMatchObject({ PATH: "/usr/bin", ELECTRON_RUN_AS_NODE: "1", - NODE_ENV: "production", + NODE_ENV: "development", + PASEO_NODE_ENV: "production", }); }); @@ -178,7 +179,7 @@ describe("node-entrypoint-launcher", () => { env: { PATH: "/usr/bin", ELECTRON_RUN_AS_NODE: "1", - NODE_ENV: "production", + PASEO_NODE_ENV: "production", }, }); }); diff --git a/packages/desktop/src/daemon/node-entrypoint-launcher.ts b/packages/desktop/src/daemon/node-entrypoint-launcher.ts index 1a6068101..f15947f46 100644 --- a/packages/desktop/src/daemon/node-entrypoint-launcher.ts +++ b/packages/desktop/src/daemon/node-entrypoint-launcher.ts @@ -1,6 +1,7 @@ const IGNORED_ARG_PREFIXES = ["-psn_", "--no-sandbox"]; export const DESKTOP_CLI_ENV = "PASEO_DESKTOP_CLI"; +const PASEO_NODE_ENV = "PASEO_NODE_ENV"; export interface NodeEntrypointSpec { entryPath: string; @@ -38,7 +39,7 @@ export function createElectronNodeEnv( return { ...baseEnv, ELECTRON_RUN_AS_NODE: "1", - ...(options?.isPackaged === true ? { NODE_ENV: "production" } : {}), + ...(options?.isPackaged === true ? { [PASEO_NODE_ENV]: "production" } : {}), }; } diff --git a/packages/desktop/src/daemon/runtime-paths.ts b/packages/desktop/src/daemon/runtime-paths.ts index ee3722882..3d9cdb8ec 100644 --- a/packages/desktop/src/daemon/runtime-paths.ts +++ b/packages/desktop/src/daemon/runtime-paths.ts @@ -257,6 +257,7 @@ function spawnAsync( ): Promise<{ stdout: string; stderr: string; exitCode: number | null }> { return new Promise((resolve, reject) => { const child = spawnProcess(command, args, { + envMode: "internal", env: options.env, stdio: ["ignore", "pipe", "pipe"], }); diff --git a/packages/desktop/src/login-shell-env.ts b/packages/desktop/src/login-shell-env.ts index 448ed3c24..a7f92f734 100644 --- a/packages/desktop/src/login-shell-env.ts +++ b/packages/desktop/src/login-shell-env.ts @@ -54,11 +54,16 @@ function resolveShellEnv(): Record | undefined { } } + const shellEnv = { ...process.env }; + delete shellEnv.PASEO_NODE_ENV; + delete shellEnv.PASEO_DESKTOP_MANAGED; + delete shellEnv.PASEO_SUPERVISED; + const result = spawnSync(shell, [...shellArgs, command], { encoding: "utf8", timeout: RESOLVE_TIMEOUT_MS, env: { - ...process.env, + ...shellEnv, ELECTRON_RUN_AS_NODE: "1", ELECTRON_NO_ATTACH_CONSOLE: "1", }, diff --git a/packages/server/package.json b/packages/server/package.json index f71e91944..892eefa44 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -28,13 +28,13 @@ "access": "public" }, "scripts": { - "dev": "cross-env NODE_ENV=development node --import tsx scripts/dev-runner.ts", - "dev:tsx": "cross-env NODE_ENV=development tsx watch --ignore '**/*.timestamp-*' src/server/index.ts", + "dev": "cross-env PASEO_NODE_ENV=development node --import tsx scripts/dev-runner.ts", + "dev:tsx": "cross-env PASEO_NODE_ENV=development tsx watch --ignore '**/*.timestamp-*' src/server/index.ts", "build": "node -e \"require('node:fs').rmSync('dist',{ recursive: true, force: true })\" && npm run build:lib && npm run build:scripts", "build:lib": "tsc -p tsconfig.server.json --incremental false && node -e \"const fs=require('node:fs'); fs.mkdirSync('dist/server/server/speech/providers/local/sherpa/assets',{recursive:true}); fs.copyFileSync('src/server/speech/providers/local/sherpa/assets/silero_vad.onnx','dist/server/server/speech/providers/local/sherpa/assets/silero_vad.onnx'); fs.cpSync('src/terminal/shell-integration','dist/server/terminal/shell-integration',{recursive:true}); fs.cpSync('src/terminal/shell-integration','dist/src/terminal/shell-integration',{recursive:true});\"", "build:scripts": "tsc -p tsconfig.scripts.json --incremental false && node -e \"const fs=require('node:fs'); fs.mkdirSync('dist/scripts',{recursive:true}); fs.copyFileSync('scripts/mcp-stdio-socket-bridge-cli.mjs','dist/scripts/mcp-stdio-socket-bridge-cli.mjs');\"", "prepack": "npm run build", - "start": "cross-env NODE_ENV=production node dist/server/server/index.js", + "start": "node dist/server/server/index.js", "typecheck": "tsgo -p tsconfig.server.typecheck.json --noEmit", "generate:config-schema": "tsx scripts/generate-config-schema.ts", "speech:models": "tsx scripts/list-speech-models.ts", diff --git a/packages/server/src/server/agent/provider-launch-config.test.ts b/packages/server/src/server/agent/provider-launch-config.test.ts index 1f210a44d..1ee8c3985 100644 --- a/packages/server/src/server/agent/provider-launch-config.test.ts +++ b/packages/server/src/server/agent/provider-launch-config.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test, vi } from "vitest"; import { - applyProviderEnv, + createProviderEnv, migrateProviderSettings, ProviderOverrideSchema, resolveProviderCommandPrefix, @@ -55,7 +55,7 @@ describe("resolveProviderCommandPrefix", () => { }); }); -describe("applyProviderEnv", () => { +describe("createProviderEnv", () => { test("merges provider env overrides", () => { const base = { PATH: "/usr/bin", @@ -68,7 +68,7 @@ describe("applyProviderEnv", () => { }, }; - const env = applyProviderEnv(base, runtime); + const env = createProviderEnv({ baseEnv: base, runtimeSettings: runtime }); expect(env.PATH).toBe("/usr/bin"); expect(env.HOME).toBe("/custom/home"); @@ -80,7 +80,7 @@ describe("applyProviderEnv", () => { const base = { PATH: "/usr/bin" }; const runtime: ProviderRuntimeSettings = { env: { PATH: "/custom/path" } }; - const env = applyProviderEnv(base, runtime); + const env = createProviderEnv({ baseEnv: base, runtimeSettings: runtime }); expect(env.PATH).toBe("/custom/path"); }); @@ -95,7 +95,7 @@ describe("applyProviderEnv", () => { CLAUDE_CODE_ENABLE_SDK_FILE_CHECKPOINTING: "true", }; - const env = applyProviderEnv(base); + const env = createProviderEnv({ baseEnv: base }); expect(env.PATH).toBe("/usr/bin"); expect(env.CLAUDECODE).toBeUndefined(); diff --git a/packages/server/src/server/agent/provider-launch-config.ts b/packages/server/src/server/agent/provider-launch-config.ts index d933f5184..9b7fc042a 100644 --- a/packages/server/src/server/agent/provider-launch-config.ts +++ b/packages/server/src/server/agent/provider-launch-config.ts @@ -3,6 +3,7 @@ import { z } from "zod"; import { execFileSync } from "node:child_process"; import path from "node:path"; import { isCommandAvailable } from "../../utils/executable.js"; +import { createExternalProcessEnv, type ProcessEnvRecord } from "../paseo-env.js"; import type { AgentProvider } from "./agent-sdk-types.js"; import { AgentProviderSchema } from "./provider-manifest.js"; @@ -176,18 +177,41 @@ const PARENT_SESSION_ENV_VARS = [ "CLAUDE_CODE_ENABLE_SDK_FILE_CHECKPOINTING", ]; -export function applyProviderEnv( - baseEnv: Record, - runtimeSettings?: ProviderRuntimeSettings, -): Record { - const merged: Record = { - ...baseEnv, - ...runtimeSettings?.env, - }; +export interface ProviderEnvOptions { + baseEnv?: ProcessEnvRecord; + runtimeSettings?: ProviderRuntimeSettings; + overlays?: Array; +} + +export interface ProviderEnvSpec { + baseEnv?: ProcessEnvRecord; + envOverlay: ProcessEnvRecord; +} + +function collectProviderEnvOverlays( + runtimeSettings: ProviderRuntimeSettings | undefined, + overlays: Array, +): ProcessEnvRecord[] { + return [runtimeSettings?.env, ...overlays].filter( + (overlay): overlay is ProcessEnvRecord => !!overlay, + ); +} + +export function createProviderEnvSpec(options: ProviderEnvOptions = {}): ProviderEnvSpec { + const overlays = collectProviderEnvOverlays(options.runtimeSettings, options.overlays ?? []); + const envOverlay: ProcessEnvRecord = Object.assign({}, ...overlays); for (const key of PARENT_SESSION_ENV_VARS) { - delete merged[key]; + envOverlay[key] = undefined; } - return merged; + return { + ...(options.baseEnv ? { baseEnv: options.baseEnv } : {}), + envOverlay, + }; +} + +export function createProviderEnv(options: ProviderEnvOptions = {}): NodeJS.ProcessEnv { + const spec = createProviderEnvSpec(options); + return createExternalProcessEnv(spec.baseEnv ?? process.env, spec.envOverlay); } export function findExecutable(name: string): string | null { @@ -203,7 +227,10 @@ export function findExecutable(name: string): string | null { } try { const cmd = process.platform === "win32" ? "where.exe" : "which"; - const result = execFileSync(cmd, [trimmed], { encoding: "utf8" }).trim(); + const result = execFileSync(cmd, [trimmed], { + encoding: "utf8", + env: createProviderEnv({ baseEnv: process.env }), + }).trim(); const lines = result.split(/\r?\n/).filter((l: string) => l.trim()); const candidate = lines.at(-1)?.trim() ?? null; return candidate && path.isAbsolute(candidate) ? candidate : null; diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index fe37675a6..9d9144c33 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -81,7 +81,7 @@ import type { ToolCallTimelineItem, } from "../agent-sdk-types.js"; import { - applyProviderEnv, + createProviderEnvSpec, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; @@ -568,13 +568,10 @@ export class ACPAgentClient implements AgentClient { const { command, args } = await this.resolveLaunchCommand(); const child = spawnProcess(command, args, { cwd: process.cwd(), - env: { - ...applyProviderEnv( - process.env as Record, - this.runtimeSettings, - ), - ...launchEnv, - }, + ...createProviderEnvSpec({ + runtimeSettings: this.runtimeSettings, + overlays: [launchEnv], + }), stdio: ["pipe", "pipe", "pipe"], }) as ChildProcessWithoutNullStreams; @@ -1319,13 +1316,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { ); const child = spawnProcess(params.command, params.args ?? [], { cwd: params.cwd ?? this.config.cwd, - env: { - ...applyProviderEnv( - process.env as Record, - this.runtimeSettings, - ), - ...env, - }, + ...createProviderEnvSpec({ + runtimeSettings: this.runtimeSettings, + overlays: [env], + }), stdio: ["ignore", "pipe", "pipe"], }); @@ -1410,13 +1404,10 @@ export class ACPAgentSession implements AgentSession, ACPClient { const args = [...prefix.args, ...this.defaultCommand.slice(1)]; const child = spawnProcess(command, args, { cwd: this.config.cwd, - env: { - ...applyProviderEnv( - process.env as Record, - this.runtimeSettings, - ), - ...this.launchEnv, - }, + ...createProviderEnvSpec({ + runtimeSettings: this.runtimeSettings, + overlays: [this.launchEnv], + }), stdio: ["pipe", "pipe", "pipe"], }) as ChildProcessWithoutNullStreams; diff --git a/packages/server/src/server/agent/providers/claude-agent.env.test.ts b/packages/server/src/server/agent/providers/claude-agent.env.test.ts index c6af98b2f..c3a1c9a45 100644 --- a/packages/server/src/server/agent/providers/claude-agent.env.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.env.test.ts @@ -1,10 +1,11 @@ +import { query, type Query } from "@anthropic-ai/claude-agent-sdk"; import { describe, expect, test, vi } from "vitest"; import { createTestLogger } from "../../../test-utils/test-logger.js"; import type { AgentLaunchContext } from "../agent-sdk-types.js"; import { ClaudeAgentClient } from "./claude-agent.js"; -function createQueryMock(events: unknown[]) { +function createQueryMock(events: unknown[]): Query { let index = 0; return { next: vi.fn(async () => @@ -23,10 +24,10 @@ function createQueryMock(events: unknown[]) { [Symbol.asyncIterator]() { return this; }, - }; + } as Query; } -describe("Claude agent env", () => { +describe("Claude SDK env", () => { test("forwards launch-context env through Claude process env", async () => { let capturedEnv: Record | undefined; const launchContext: AgentLaunchContext = { @@ -35,38 +36,36 @@ describe("Claude agent env", () => { PASEO_TEST_FLAG: "launch-value", }, }; - const queryFactory = vi.fn( - ({ options }: { options: { env?: Record } }) => { - capturedEnv = options.env; - return createQueryMock([ - { - type: "system", - subtype: "init", - session_id: "managed-agent-env-session", - permissionMode: "default", - model: "opus", + const queryFactory = vi.fn(({ options }: Parameters[0]) => { + capturedEnv = options.env; + return createQueryMock([ + { + type: "system", + subtype: "init", + session_id: "managed-agent-env-session", + permissionMode: "default", + model: "opus", + }, + { + type: "assistant", + message: { content: "done" }, + }, + { + type: "result", + subtype: "success", + usage: { + input_tokens: 1, + cache_read_input_tokens: 0, + output_tokens: 1, }, - { - type: "assistant", - message: { content: "done" }, - }, - { - type: "result", - subtype: "success", - usage: { - input_tokens: 1, - cache_read_input_tokens: 0, - output_tokens: 1, - }, - total_cost_usd: 0, - }, - ]); - }, - ); + total_cost_usd: 0, + }, + ]); + }); const client = new ClaudeAgentClient({ logger: createTestLogger(), - queryFactory: queryFactory as never, + queryFactory, }); const session = await client.createSession( { @@ -94,38 +93,36 @@ describe("Claude agent env", () => { PASEO_TEST_FLAG: "resume-launch-value", }, }; - const queryFactory = vi.fn( - ({ options }: { options: { env?: Record } }) => { - capturedEnv = options.env; - return createQueryMock([ - { - type: "system", - subtype: "init", - session_id: "persisted-session", - permissionMode: "default", - model: "opus", + const queryFactory = vi.fn(({ options }: Parameters[0]) => { + capturedEnv = options.env; + return createQueryMock([ + { + type: "system", + subtype: "init", + session_id: "persisted-session", + permissionMode: "default", + model: "opus", + }, + { + type: "assistant", + message: { content: "done" }, + }, + { + type: "result", + subtype: "success", + usage: { + input_tokens: 1, + cache_read_input_tokens: 0, + output_tokens: 1, }, - { - type: "assistant", - message: { content: "done" }, - }, - { - type: "result", - subtype: "success", - usage: { - input_tokens: 1, - cache_read_input_tokens: 0, - output_tokens: 1, - }, - total_cost_usd: 0, - }, - ]); - }, - ); + total_cost_usd: 0, + }, + ]); + }); const client = new ClaudeAgentClient({ logger: createTestLogger(), - queryFactory: queryFactory as never, + queryFactory, }); const session = await client.resumeSession( { diff --git a/packages/server/src/server/agent/providers/claude-agent.spawn.test.ts b/packages/server/src/server/agent/providers/claude-agent.spawn.test.ts index f227c8548..d57f07402 100644 --- a/packages/server/src/server/agent/providers/claude-agent.spawn.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.spawn.test.ts @@ -35,9 +35,9 @@ function createQueryMock(events: unknown[]): Query { } function createChildProcessStub(): ChildProcess { - return { - stderr: new EventEmitter(), - } as ChildProcess; + const child = new EventEmitter() as ChildProcess; + child.stderr = new EventEmitter() as ChildProcess["stderr"]; + return child; } describe("Claude spawn override", () => { @@ -96,8 +96,9 @@ describe("Claude spawn override", () => { await session.close(); } - expect(spawnSpy).toHaveBeenCalledTimes(1); - const spawnOptions = spawnSpy.mock.calls[0]?.[2]; + const claudeSpawnCall = spawnSpy.mock.calls.find(([, args]) => args[0] === "claude.js"); + expect(claudeSpawnCall).toBeDefined(); + const spawnOptions = claudeSpawnCall?.[2]; expect(spawnOptions?.shell).toBe(false); }); }); diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index b2418b56b..adb000054 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -72,7 +72,11 @@ import type { McpServerConfig, PersistedAgentDescriptor, } from "../agent-sdk-types.js"; -import { applyProviderEnv, type ProviderRuntimeSettings } from "../provider-launch-config.js"; +import { + createProviderEnv, + createProviderEnvSpec, + type ProviderRuntimeSettings, +} from "../provider-launch-config.js"; import { findExecutable, isCommandAvailable } from "../../../utils/executable.js"; import { withTimeout } from "../../../utils/promise-timeout.js"; import { execCommand, spawnProcess } from "../../../utils/spawn.js"; @@ -256,10 +260,11 @@ function applyRuntimeSettingsToClaudeOptions( const command = isDefaultRuntime ? process.execPath : resolved.command; const child = spawnProcess(command, resolved.args, { cwd: spawnOptions.cwd, - env: { - ...applyProviderEnv(spawnOptions.env, runtimeSettings), - ...launchEnv, - }, + ...createProviderEnvSpec({ + baseEnv: spawnOptions.env, + runtimeSettings, + overlays: [launchEnv], + }), signal: spawnOptions.signal, stdio: ["pipe", "pipe", "pipe"], // Bypass cmd.exe on Windows: the SDK passes --mcp-config with inline JSON @@ -1268,13 +1273,14 @@ async function resolveClaudeVersion( runtimeSettings?: ProviderRuntimeSettings, ): Promise { const command = runtimeSettings?.command; + const envSpec = createProviderEnvSpec({ runtimeSettings }); try { if (command?.mode === "replace") { const { stdout } = await execCommand( command.argv[0]!, [...command.argv.slice(1), "--version"], - { timeout: 5_000 }, + { ...envSpec, timeout: 5_000 }, ); return stdout.trim() || null; } @@ -1285,6 +1291,7 @@ async function resolveClaudeVersion( } const { stdout } = await execCommand(executable, ["--version"], { + ...envSpec, timeout: 5_000, }); return stdout.trim() || null; @@ -1303,7 +1310,10 @@ async function resolveClaudeAuth( args: string[], ): Promise<{ stdout: string; stderr: string }> => { try { - return await execCommand(executable, args, { timeout: 5_000 }); + return await execCommand(executable, args, { + ...createProviderEnvSpec({ runtimeSettings }), + timeout: 5_000, + }); } catch (error) { const err = error as { stdout?: string; @@ -2221,6 +2231,20 @@ class ClaudeAgentSession implements AgentSession { private async buildOptions(): Promise { const { thinking, effort } = this.resolveThinkingConfig(); const appendedSystemPrompt = this.buildAppendedSystemPrompt(); + const extraClaudeOptions = this.config.extra?.claude; + const sdkEnv = createProviderEnv({ + baseEnv: process.env, + runtimeSettings: this.runtimeSettings, + overlays: [ + extraClaudeOptions?.env, + { + // Increase MCP timeouts for long-running tool calls (10 minutes) + MCP_TIMEOUT: "600000", + MCP_TOOL_TIMEOUT: "600000", + }, + this.launchEnv, + ], + }); const claudeBinary = await findExecutable("claude"); this.logger.debug( @@ -2256,13 +2280,6 @@ class ClaudeAgentSession implements AgentSession { this.captureStderr(data); this.logger.error({ stderr: data.trim() }, "Claude Agent SDK stderr"); }, - env: { - ...process.env, - // Increase MCP timeouts for long-running tool calls (10 minutes) - MCP_TIMEOUT: "600000", - MCP_TOOL_TIMEOUT: "600000", - ...this.launchEnv, - }, // Required for provider-level /rewind support. enableFileCheckpointing: true, // If we have a session ID from a previous query (e.g., after interrupt), @@ -2270,7 +2287,8 @@ class ClaudeAgentSession implements AgentSession { ...(this.claudeSessionId ? { resume: this.claudeSessionId } : {}), ...(thinking ? { thinking } : {}), ...(effort ? { effort } : {}), - ...this.config.extra?.claude, + ...extraClaudeOptions, + env: sdkEnv, }; if (this.config.mcpServers) { diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index 7c0821521..3814785d0 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -44,7 +44,8 @@ import { mapCodexToolCallFromThreadItem, } from "./codex/tool-call-mapper.js"; import { - applyProviderEnv, + createProviderEnv, + createProviderEnvSpec, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; @@ -2450,15 +2451,11 @@ export async function codexAppServerTurnInputFromPrompt( function buildCodexAppServerEnv( runtimeSettings?: ProviderRuntimeSettings, launchEnv?: Record, -): Record { - const env = applyProviderEnv(process.env, runtimeSettings); - if (!launchEnv) { - return env; - } - return { - ...env, - ...launchEnv, - }; +): NodeJS.ProcessEnv { + return createProviderEnv({ + runtimeSettings, + overlays: [launchEnv], + }); } function buildCodexAppServerInitializeParams(): { @@ -4211,7 +4208,10 @@ export class CodexAppServerAgentClient implements AgentClient { return spawnProcess(launchPrefix.command, [...launchPrefix.args, "app-server"], { detached: process.platform !== "win32", stdio: ["pipe", "pipe", "pipe"], - env: buildCodexAppServerEnv(this.runtimeSettings, launchEnv), + ...createProviderEnvSpec({ + runtimeSettings: this.runtimeSettings, + overlays: [launchEnv], + }), }) as ChildProcessWithoutNullStreams; } diff --git a/packages/server/src/server/agent/providers/diagnostic-utils.ts b/packages/server/src/server/agent/providers/diagnostic-utils.ts index b266df525..08c7e5bd0 100644 --- a/packages/server/src/server/agent/providers/diagnostic-utils.ts +++ b/packages/server/src/server/agent/providers/diagnostic-utils.ts @@ -1,4 +1,4 @@ -import type { ProviderRuntimeSettings } from "../provider-launch-config.js"; +import { createProviderEnvSpec, type ProviderRuntimeSettings } from "../provider-launch-config.js"; import { execCommand } from "../../../utils/spawn.js"; interface DiagnosticEntry { @@ -45,7 +45,10 @@ export function toDiagnosticErrorMessage(error: unknown): string { export async function resolveBinaryVersion(binaryPath: string): Promise { try { - const { stdout } = await execCommand(binaryPath, ["--version"], { timeout: 5_000 }); + const { stdout } = await execCommand(binaryPath, ["--version"], { + ...createProviderEnvSpec(), + timeout: 5_000, + }); return stdout.trim() || "unknown"; } catch { return "unknown"; diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index 8ea3649ee..a9e080b5e 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -41,7 +41,7 @@ import type { ToolCallTimelineItem, } from "../agent-sdk-types.js"; import { - applyProviderEnv, + createProviderEnvSpec, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; @@ -868,7 +868,7 @@ export class OpenCodeServerManager { [...launchPrefix.args, "serve", "--port", String(port)], { stdio: ["ignore", "pipe", "pipe"], - env: applyProviderEnv(process.env, this.runtimeSettings), + ...createProviderEnvSpec({ runtimeSettings: this.runtimeSettings }), }, ); diff --git a/packages/server/src/server/checkout-git-utils.ts b/packages/server/src/server/checkout-git-utils.ts index c5dcd9e73..88afdd84b 100644 --- a/packages/server/src/server/checkout-git-utils.ts +++ b/packages/server/src/server/checkout-git-utils.ts @@ -5,10 +5,9 @@ import { } from "../utils/checkout-git.js"; import { runGitCommand } from "../utils/run-git-command.js"; -export const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = { - ...process.env, +export const READ_ONLY_GIT_ENV = { GIT_OPTIONAL_LOCKS: "0", -}; +} as const; export type CheckoutErrorCode = "NOT_GIT_REPO" | "NOT_ALLOWED" | "MERGE_CONFLICT" | "UNKNOWN"; @@ -21,7 +20,7 @@ export async function resolveCheckoutGitDir(cwd: string): Promise try { const { stdout } = await runGitCommand(["rev-parse", "--absolute-git-dir"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const gitDir = stdout.trim(); return gitDir.length > 0 ? gitDir : null; diff --git a/packages/server/src/server/config.ts b/packages/server/src/server/config.ts index d6b83777a..6137f037e 100644 --- a/packages/server/src/server/config.ts +++ b/packages/server/src/server/config.ts @@ -1,4 +1,5 @@ import path from "node:path"; +import { resolvePaseoNodeEnv } from "./paseo-env.js"; import { z } from "zod"; import type { PaseoDaemonConfig } from "./bootstrap.js"; @@ -240,7 +241,7 @@ export function loadConfig( mcpEnabled, mcpInjectIntoAgents, mcpDebug: env.MCP_DEBUG === "1", - isDev: env.NODE_ENV === "development", + isDev: resolvePaseoNodeEnv(env) === "development", agentStoragePath: path.join(paseoHome, "agents"), staticDir: "public", agentClients: {}, diff --git a/packages/server/src/server/editor-targets.test.ts b/packages/server/src/server/editor-targets.test.ts index e550a588a..11d8f4ead 100644 --- a/packages/server/src/server/editor-targets.test.ts +++ b/packages/server/src/server/editor-targets.test.ts @@ -63,6 +63,7 @@ describe("editor-targets", () => { expect(spawn).toHaveBeenCalledWith("/usr/local/bin/code", ["/tmp/repo"], { detached: true, + env: expect.any(Object), shell: false, stdio: "ignore", }); diff --git a/packages/server/src/server/editor-targets.ts b/packages/server/src/server/editor-targets.ts index d7d6d217f..75cbdb3d8 100644 --- a/packages/server/src/server/editor-targets.ts +++ b/packages/server/src/server/editor-targets.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import type { ChildProcess } from "node:child_process"; import { existsSync } from "node:fs"; import { posix, win32 } from "node:path"; import type { @@ -6,7 +6,9 @@ import type { EditorTargetId, KnownEditorTargetId, } from "../shared/messages.js"; -import { findExecutable, quoteWindowsArgument, quoteWindowsCommand } from "../utils/executable.js"; +import { createExternalProcessEnv } from "./paseo-env.js"; +import { findExecutable } from "../utils/executable.js"; +import { spawnProcess } from "../utils/spawn.js"; interface EditorTargetDefinition { id: KnownEditorTargetId; @@ -23,7 +25,7 @@ interface ListAvailableEditorTargetsDependencies { type OpenInEditorTargetDependencies = ListAvailableEditorTargetsDependencies & { existsSync?: typeof existsSync; - spawn?: typeof spawn; + spawn?: typeof spawnProcess; }; const EDITOR_TARGETS: readonly EditorTargetDefinition[] = [ @@ -127,7 +129,7 @@ export async function openInEditorTarget( const pathToOpen = input.path.trim(); const existsSyncFn = dependencies.existsSync ?? existsSync; const findExecutableFn = dependencies.findExecutable ?? findExecutable; - const spawnFn = dependencies.spawn ?? spawn; + const spawnFn = dependencies.spawn ?? spawnProcess; if (!pathToOpen || !isAbsolutePath(pathToOpen)) { throw new Error("Editor target path must be an absolute local path"); @@ -143,17 +145,12 @@ export async function openInEditorTarget( findExecutableFn, }); - const command = platform === "win32" ? quoteWindowsCommand(launch.command) : launch.command; - const args = - platform === "win32" - ? launch.args.map((argument) => quoteWindowsArgument(argument)) - : launch.args; - await new Promise((resolve, reject) => { let child: ChildProcess; try { - child = spawnFn(command, args, { + child = spawnFn(launch.command, launch.args, { detached: true, + env: createExternalProcessEnv(process.env), shell: platform === "win32", stdio: "ignore", }); diff --git a/packages/server/src/server/exports.ts b/packages/server/src/server/exports.ts index 99405ec87..12f5d85c6 100644 --- a/packages/server/src/server/exports.ts +++ b/packages/server/src/server/exports.ts @@ -31,11 +31,10 @@ export { // Provider binary resolution export { - applyProviderEnv, type ProviderOverride, type ProviderProfileModel, } from "./agent/provider-launch-config.js"; -export { findExecutable, quoteWindowsArgument, quoteWindowsCommand } from "../utils/executable.js"; +export { findExecutable } from "../utils/executable.js"; export { execCommand, spawnProcess } from "../utils/spawn.js"; // Provider manifest (source of truth for provider definitions) diff --git a/packages/server/src/server/paseo-env.test.ts b/packages/server/src/server/paseo-env.test.ts new file mode 100644 index 000000000..c0f13e82d --- /dev/null +++ b/packages/server/src/server/paseo-env.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, test } from "vitest"; +import { + createExternalCommandProcessEnv, + createExternalProcessEnv, + createPaseoInternalEnv, + resolvePaseoNodeEnv, +} from "./paseo-env.js"; + +describe("paseo env contract", () => { + const ELECTRON_RUN_AS_NODE = "ELECTRON_RUN_AS_NODE"; + const PASEO_NODE_ENV = "PASEO_NODE_ENV"; + const baseEnv = { + [ELECTRON_RUN_AS_NODE]: "1", + ELECTRON_NO_ATTACH_CONSOLE: "1", + NODE_ENV: "development", + PATH: "/usr/bin", + PASEO_AGENT_ID: "agent-123", + PASEO_DESKTOP_MANAGED: "1", + [PASEO_NODE_ENV]: "production", + PASEO_SUPERVISED: "1", + }; + const runtimeControlEnvKeys = [ + "ELECTRON_RUN_AS_NODE", + "PASEO_NODE_ENV", + "PASEO_DESKTOP_MANAGED", + "PASEO_SUPERVISED", + "ELECTRON_NO_ATTACH_CONSOLE", + ] as const; + + test("builds internal daemon child env by preserving pass-through and control vars", () => { + const env = createPaseoInternalEnv(baseEnv); + + expect(env).toMatchObject({ + [ELECTRON_RUN_AS_NODE]: "1", + ELECTRON_NO_ATTACH_CONSOLE: "1", + NODE_ENV: "development", + PATH: "/usr/bin", + PASEO_DESKTOP_MANAGED: "1", + [PASEO_NODE_ENV]: "production", + PASEO_SUPERVISED: "1", + PASEO_AGENT_ID: "agent-123", + }); + }); + + test("builds external process env by scrubbing runtime control vars after overlays", () => { + const env = createExternalProcessEnv(baseEnv, { + ELECTRON_NO_ATTACH_CONSOLE: "1", + ELECTRON_RUN_AS_NODE: "0", + EXTRA_VALUE: "from-overlay", + PASEO_DESKTOP_MANAGED: "1", + PASEO_NODE_ENV: "test", + PASEO_SUPERVISED: "1", + PATH: "/custom/bin", + }); + + for (const key of runtimeControlEnvKeys) { + expect(env[key]).toBeUndefined(); + } + expect(env.NODE_ENV).toBe("development"); + expect(env.PASEO_AGENT_ID).toBe("agent-123"); + expect(env.PATH).toBe("/custom/bin"); + }); + + test("applies non-control overlays to external process env", () => { + const env = createExternalProcessEnv(baseEnv, { PATH: "/custom/bin" }, { CUSTOM: "value" }); + + expect(env.CUSTOM).toBe("value"); + expect(env.NODE_ENV).toBe("development"); + expect(env.PATH).toBe("/custom/bin"); + }); + + test("builds process.execPath external command env with Electron node mode", () => { + const env = createExternalCommandProcessEnv(process.execPath, baseEnv, { + ELECTRON_RUN_AS_NODE: "0", + PASEO_NODE_ENV: "test", + }); + + expect(env[ELECTRON_RUN_AS_NODE]).toBe("1"); + expect(env.NODE_ENV).toBe("development"); + expect(env.PASEO_AGENT_ID).toBe("agent-123"); + expect(env.PATH).toBe("/usr/bin"); + expect(env.ELECTRON_NO_ATTACH_CONSOLE).toBeUndefined(); + expect(env.PASEO_DESKTOP_MANAGED).toBeUndefined(); + expect(env[PASEO_NODE_ENV]).toBeUndefined(); + expect(env.PASEO_SUPERVISED).toBeUndefined(); + }); + + test("always re-adds Electron node mode after scrubbing process.execPath overlays", () => { + const env = createExternalCommandProcessEnv(process.execPath, baseEnv, { + ELECTRON_RUN_AS_NODE: undefined, + }); + + for (const key of runtimeControlEnvKeys) { + if (key === ELECTRON_RUN_AS_NODE) continue; + expect(env[key]).toBeUndefined(); + } + expect(env[ELECTRON_RUN_AS_NODE]).toBe("1"); + }); + + test("does not add Electron node mode for non-execPath commands", () => { + const env = createExternalCommandProcessEnv("node", baseEnv, { + ELECTRON_RUN_AS_NODE: "1", + }); + + expect(env[ELECTRON_RUN_AS_NODE]).toBeUndefined(); + }); + + test("does not use user NODE_ENV as Paseo runtime mode", () => { + expect(resolvePaseoNodeEnv({ NODE_ENV: "development" })).toBeUndefined(); + expect(resolvePaseoNodeEnv({ NODE_ENV: "development", PASEO_NODE_ENV: "production" })).toBe( + "production", + ); + expect(resolvePaseoNodeEnv({ NODE_ENV: "test", PASEO_NODE_ENV: "local" })).toBeUndefined(); + }); +}); diff --git a/packages/server/src/server/paseo-env.ts b/packages/server/src/server/paseo-env.ts new file mode 100644 index 000000000..208634fb0 --- /dev/null +++ b/packages/server/src/server/paseo-env.ts @@ -0,0 +1,98 @@ +import { realpathSync } from "node:fs"; +import path from "node:path"; + +const PASEO_NODE_ENV = "PASEO_NODE_ENV"; +const ELECTRON_RUN_AS_NODE = "ELECTRON_RUN_AS_NODE"; + +const RUNTIME_CONTROL_ENV_KEYS = [ + PASEO_NODE_ENV, + "PASEO_DESKTOP_MANAGED", + "PASEO_SUPERVISED", + ELECTRON_RUN_AS_NODE, + "ELECTRON_NO_ATTACH_CONSOLE", +] as const; + +export type PaseoNodeEnv = "development" | "production" | "test"; +export type ProcessEnvRecord = Record; +type ExternalProcessEnv = NodeJS.ProcessEnv & Record; + +let resolvedProcessExecPath: string | undefined; + +function buildInternalProcessEnv(baseEnv: T): T { + return { ...baseEnv } as T; +} + +function buildExternalProcessEnv( + baseEnv: ProcessEnvRecord, + overlays: ProcessEnvRecord[], +): ExternalProcessEnv { + const sanitized = Object.assign({}, baseEnv, ...overlays); + for (const key of RUNTIME_CONTROL_ENV_KEYS) { + delete sanitized[key]; + } + for (const [key, value] of Object.entries(sanitized)) { + if (value === undefined) { + delete sanitized[key]; + } + } + return sanitized as ExternalProcessEnv; +} + +function normalizeExecutablePath(executablePath: string): string { + return process.platform === "win32" ? executablePath.toLowerCase() : executablePath; +} + +function resolveExecutablePath(executablePath: string): string | undefined { + try { + return realpathSync.native(executablePath); + } catch { + return undefined; + } +} + +function isProcessExecPathCommand(command: string): boolean { + if (command === process.execPath) { + return true; + } + if (!path.isAbsolute(command)) { + return false; + } + + resolvedProcessExecPath ??= resolveExecutablePath(process.execPath); + const resolvedCommand = resolveExecutablePath(command); + if (!resolvedCommand || !resolvedProcessExecPath) { + return false; + } + + return ( + normalizeExecutablePath(resolvedCommand) === normalizeExecutablePath(resolvedProcessExecPath) + ); +} + +export function createPaseoInternalEnv(baseEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + return buildInternalProcessEnv(baseEnv); +} + +export function createExternalProcessEnv( + baseEnv: ProcessEnvRecord, + ...overlays: ProcessEnvRecord[] +): ExternalProcessEnv { + return buildExternalProcessEnv(baseEnv, overlays); +} + +export function createExternalCommandProcessEnv( + command: string, + baseEnv: ProcessEnvRecord, + ...overlays: ProcessEnvRecord[] +): ExternalProcessEnv { + const env = buildExternalProcessEnv(baseEnv, overlays); + if (isProcessExecPathCommand(command)) { + env[ELECTRON_RUN_AS_NODE] = "1"; + } + return env; +} + +export function resolvePaseoNodeEnv(env: NodeJS.ProcessEnv): PaseoNodeEnv | undefined { + const value = env[PASEO_NODE_ENV]; + return value === "development" || value === "production" || value === "test" ? value : undefined; +} diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 539b820b6..2da7b488a 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -4900,7 +4900,9 @@ export class Session { const message = branchLabel ? `${Session.PASEO_STASH_PREFIX} ${branchLabel}` : `${Session.PASEO_STASH_PREFIX} unnamed`; - await execCommand("git", ["stash", "push", "--include-untracked", "-m", message], { cwd }); + await execCommand("git", ["stash", "push", "--include-untracked", "-m", message], { + cwd, + }); await this.notifyGitMutation(cwd, "stash-push"); this.checkoutDiffManager.scheduleRefreshForCwd(cwd); this.emit({ @@ -4920,7 +4922,9 @@ export class Session { ): Promise { const { cwd, stashIndex, requestId } = msg; try { - await execCommand("git", ["stash", "pop", `stash@{${stashIndex}}`], { cwd }); + await execCommand("git", ["stash", "pop", `stash@{${stashIndex}}`], { + cwd, + }); await this.notifyGitMutation(cwd, "stash-pop"); this.checkoutDiffManager.scheduleRefreshForCwd(cwd); this.emit({ diff --git a/packages/server/src/server/speech/providers/local/sherpa/model-downloader.ts b/packages/server/src/server/speech/providers/local/sherpa/model-downloader.ts index 21b61f6c0..1cb4838aa 100644 --- a/packages/server/src/server/speech/providers/local/sherpa/model-downloader.ts +++ b/packages/server/src/server/speech/providers/local/sherpa/model-downloader.ts @@ -3,10 +3,10 @@ import { mkdir, rename, rm, stat } from "node:fs/promises"; import path from "node:path"; import { Readable } from "node:stream"; import { pipeline } from "node:stream/promises"; -import { spawn } from "node:child_process"; import type pino from "pino"; import { getSherpaOnnxModelSpec, type SherpaOnnxModelId } from "./model-catalog.js"; +import { spawnProcess } from "../../../../../utils/spawn.js"; export interface EnsureSherpaOnnxModelOptions { modelsDir: string; @@ -70,7 +70,9 @@ async function extractTarArchive(archivePath: string, destDir: string): Promise< await mkdir(destDir, { recursive: true }); await new Promise((resolve, reject) => { - const child = spawn("tar", ["xf", archivePath, "-C", destDir], { stdio: "inherit" }); + const child = spawnProcess("tar", ["xf", archivePath, "-C", destDir], { + stdio: "inherit", + }); child.on("error", reject); child.on("exit", (code) => { if (code === 0) resolve(); diff --git a/packages/server/src/server/speech/providers/local/sherpa/sherpa-onnx-node-loader.ts b/packages/server/src/server/speech/providers/local/sherpa/sherpa-onnx-node-loader.ts index a12291821..de92b0f17 100644 --- a/packages/server/src/server/speech/providers/local/sherpa/sherpa-onnx-node-loader.ts +++ b/packages/server/src/server/speech/providers/local/sherpa/sherpa-onnx-node-loader.ts @@ -7,6 +7,7 @@ import { resolveSherpaLoaderEnv, sherpaPlatformPackageName, } from "./sherpa-runtime-env.js"; +import { createExternalCommandProcessEnv } from "../../../../paseo-env.js"; export interface SherpaOnnxNodeModule { OfflineRecognizer: new (config: unknown) => unknown; @@ -41,7 +42,9 @@ function maybePatchLinuxAddonRunpath(addonPath: string): void { if (process.platform !== "linux") { return; } + const patchelfEnv = createExternalCommandProcessEnv("patchelf", process.env); const patchelfCheck = spawnSync("patchelf", ["--version"], { + env: patchelfEnv, stdio: "ignore", }); if (patchelfCheck.status !== 0) { @@ -50,6 +53,7 @@ function maybePatchLinuxAddonRunpath(addonPath: string): void { const currentRpath = spawnSync("patchelf", ["--print-rpath", addonPath], { encoding: "utf8", + env: patchelfEnv, }); if (currentRpath.status !== 0) { return; @@ -60,6 +64,7 @@ function maybePatchLinuxAddonRunpath(addonPath: string): void { } spawnSync("patchelf", ["--set-rpath", "$ORIGIN", addonPath], { + env: patchelfEnv, stdio: "ignore", }); } diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index a9692cf7c..a23dce2f4 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -491,7 +491,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { return this.readAuxiliaryCache(this.localBranchCache, key, options, async () => { const result = await this.deps.runGitCommand(["rev-parse", "--verify", "--quiet", ref], { cwd: normalizedCwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, acceptExitCodes: [0, 1], }); return result.exitCode === 0; @@ -523,7 +523,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { return this.readAuxiliaryCache(this.stashListCache, key, readOptions, async () => { const { stdout } = await this.deps.runGitCommand(["stash", "list", "--format=%gd%x00%s"], { cwd: normalizedCwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return parseWorkspaceGitStashList(stdout, { paseoOnly }); }); @@ -895,7 +895,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { try { const { stdout } = await this.deps.runGitCommand(["rev-parse", "--show-toplevel"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return parseGitRevParsePath(stdout); } catch { @@ -1714,10 +1714,7 @@ function buildNotGitSnapshot(cwd: string): WorkspaceGitRuntimeSnapshot { async function runGitFetch(cwd: string): Promise { await runGitCommand(["fetch", "origin", "--prune"], { cwd, - env: { - ...process.env, - GIT_TERMINAL_PROMPT: "0", - }, + envOverlay: { GIT_TERMINAL_PROMPT: "0" }, timeout: 120_000, }); } diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 3265b682d..2f95a2585 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -7,10 +7,9 @@ const DEFAULT_GITHUB_CACHE_TTL_MS = 30_000; export const GITHUB_POLL_FAST_INTERVAL_MS = 20_000; export const GITHUB_POLL_SLOW_INTERVAL_MS = 120_000; export const GITHUB_POLL_ERROR_BACKOFF_CAP_MS = 300_000; -const GITHUB_ENV: NodeJS.ProcessEnv = { - ...process.env, +const GITHUB_ENV = { GIT_TERMINAL_PROMPT: "0", -}; +} as const; const LabelSchema = z.object({ name: z.string().optional(), @@ -1165,7 +1164,7 @@ async function runGhCommand( ): Promise { return execCommand("gh", args, { cwd: options.cwd, - env: GITHUB_ENV, + envOverlay: GITHUB_ENV, maxBuffer: 10 * 1024 * 1024, }); } diff --git a/packages/server/src/terminal/terminal.ts b/packages/server/src/terminal/terminal.ts index bba62a367..0276ff37b 100644 --- a/packages/server/src/terminal/terminal.ts +++ b/packages/server/src/terminal/terminal.ts @@ -7,6 +7,7 @@ import { basename, dirname, join } from "node:path"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; import stripAnsi from "strip-ansi"; +import { createExternalProcessEnv } from "../server/paseo-env.js"; import type { TerminalCell, TerminalState } from "../shared/messages.js"; const { Terminal } = xterm; @@ -214,11 +215,9 @@ function prepareZshShellIntegrationRuntimeDir(sourceDir = resolveZshShellIntegra export function buildTerminalEnvironment( input: BuildTerminalEnvironmentInput, ): Record { - const baseEnv: Record = { - ...process.env, - ...input.env, + const baseEnv: Record = createExternalProcessEnv(process.env, input.env, { TERM: "xterm-256color", - }; + }); if (basename(input.shell) !== "zsh") { return baseEnv; diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 0e983a8d2..dac1c34aa 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -18,10 +18,9 @@ import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-pa import { runGitCommand } from "./run-git-command.js"; import { isPaseoOwnedWorktreeCwd } from "./worktree.js"; import { readPaseoWorktreeMetadata } from "./worktree-metadata.js"; -const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = { - ...process.env, +const READ_ONLY_GIT_ENV = { GIT_OPTIONAL_LOCKS: "0", -}; +} as const; const DEFAULT_PULL_REQUEST_STATUS_CACHE_TTL_MS = 30_000; const PULL_REQUEST_STATUS_CACHE_MAX = 1_000; @@ -178,7 +177,7 @@ async function listGitRefs(cwd: string, refPrefix: string): Promise { "--format=%(refname)%09%(committerdate:unix)", refPrefix, ], - { cwd, env: READ_ONLY_GIT_ENV }, + { cwd, envOverlay: READ_ONLY_GIT_ENV }, ); return stdout .split("\n") @@ -326,7 +325,7 @@ export async function resolveBranchCheckout( const localRef = `refs/heads/${normalized}`; const localResult = await runGitCommand(["rev-parse", "--verify", "--quiet", localRef], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, acceptExitCodes: [0, 1], }); const hasLocal = localResult.exitCode === 0; @@ -338,7 +337,7 @@ export async function resolveBranchCheckout( const remoteRefPath = `refs/remotes/${remoteRef}`; const remoteResult = await runGitCommand(["rev-parse", "--verify", "--quiet", remoteRefPath], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, acceptExitCodes: [0, 1], }); const hasRemote = remoteResult.exitCode === 0; @@ -399,7 +398,7 @@ async function listCheckoutFileChanges( ignoreWhitespace, extra: ["--name-status", ...getCheckoutDiffRefArgs(refs)], }), - { cwd, env: READ_ONLY_GIT_ENV }, + { cwd, envOverlay: READ_ONLY_GIT_ENV }, ); for (const line of nameStatusOut .split("\n") @@ -441,7 +440,7 @@ async function listCheckoutFileChanges( ["ls-files", "--others", "--exclude-standard"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }, ); for (const file of untrackedOut @@ -481,7 +480,7 @@ async function readGitFileContentAtRef( try { const { stdout } = await runGitCommand(["show", `${ref}:${path}`], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return stdout; } catch { @@ -493,7 +492,7 @@ async function tryResolveMergeBase(cwd: string, baseRef: string): Promise 0 ? sha : null; @@ -538,7 +537,7 @@ async function getTrackedNumstatByPath( }), { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, maxOutputBytes: TRACKED_DIFF_NUMSTAT_MAX_BYTES, acceptExitCodes: [0], }, @@ -710,7 +709,7 @@ function isGitError(error: unknown): boolean { async function requireGitRepo(cwd: string): Promise { try { - await runGitCommand(["rev-parse", "--git-dir"], { cwd, env: READ_ONLY_GIT_ENV }); + await runGitCommand(["rev-parse", "--git-dir"], { cwd, envOverlay: READ_ONLY_GIT_ENV }); } catch { throw new NotGitRepoError(cwd); } @@ -720,7 +719,7 @@ export async function getCurrentBranch(cwd: string): Promise { try { const { stdout } = await runGitCommand(["rev-parse", "--abbrev-ref", "HEAD"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const branch = stdout.trim(); if (branch === "HEAD") { @@ -739,7 +738,7 @@ async function getRebaseHeadBranch(cwd: string): Promise { try { const { stdout } = await runGitCommand(["rev-parse", "--git-path", path], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const headName = (await readFile(resolve(cwd, stdout.trim()), "utf8")).trim(); if (headName.startsWith("refs/heads/")) { @@ -758,7 +757,7 @@ async function getWorktreeRoot(cwd: string): Promise { try { const { stdout } = await runGitCommand(["rev-parse", "--show-toplevel"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return parseGitRevParsePath(stdout); } catch { @@ -769,7 +768,7 @@ async function getWorktreeRoot(cwd: string): Promise { export async function getMainRepoRoot(cwd: string): Promise { const { stdout: commonDirOut } = await runGitCommand(["rev-parse", "--git-common-dir"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const commonDir = resolveGitRevParsePath(cwd, commonDirOut); if (!commonDir) { @@ -783,7 +782,7 @@ export async function getMainRepoRoot(cwd: string): Promise { const { stdout: worktreeOut } = await runGitCommand(["worktree", "list", "--porcelain"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const worktrees = parseWorktreeList(worktreeOut); const nonBareNonPaseo = worktrees.filter((wt) => !wt.isBare && !isPaseoWorktreePath(wt.path)); @@ -849,7 +848,7 @@ async function getWorktreePathForBranch(cwd: string, branchName: string): Promis try { const { stdout } = await runGitCommand(["worktree", "list", "--porcelain"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const entries = parseWorktreeList(stdout); const ref = branchName.startsWith("refs/heads/") ? branchName : `refs/heads/${branchName}`; @@ -946,7 +945,7 @@ async function resolveBaseRefForCwd( async function isWorkingTreeDirty(cwd: string): Promise { const { stdout } = await runGitCommand(["status", "--porcelain"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return stdout.trim().length > 0; } @@ -955,7 +954,7 @@ export async function getOriginRemoteUrl(cwd: string): Promise { try { const { stdout } = await runGitCommand(["config", "--get", "remote.origin.url"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const url = stdout.trim(); return url.length > 0 ? url : null; @@ -973,7 +972,7 @@ async function getGitConfigValue(cwd: string, key: string): Promise 0 ? value : null; @@ -1019,7 +1018,7 @@ export async function resolveAbsoluteGitDir(cwd: string): Promise try { const { stdout } = await runGitCommand(["rev-parse", "--absolute-git-dir"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const gitDir = stdout.trim(); return gitDir.length > 0 ? gitDir : null; @@ -1061,7 +1060,7 @@ export async function resolveRepositoryDefaultBranch(repoRoot: string): Promise< ["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], { cwd: repoRoot, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }, ); const ref = stdout.trim(); @@ -1075,7 +1074,7 @@ export async function resolveRepositoryDefaultBranch(repoRoot: string): Promise< try { await runGitCommand(["show-ref", "--verify", "--quiet", `refs/heads/${localName}`], { cwd: repoRoot, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return localName; } catch { @@ -1088,7 +1087,7 @@ export async function resolveRepositoryDefaultBranch(repoRoot: string): Promise< const { stdout } = await runGitCommand(["branch", "--format=%(refname:short)"], { cwd: repoRoot, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const branches = new Set( stdout @@ -1137,7 +1136,7 @@ function normalizeComparisonBaseRefName(input: string): ComparisonBaseRefName { async function doesGitRefExist(cwd: string, fullRef: string): Promise { const result = await runGitCommand(["show-ref", "--verify", "--quiet", fullRef], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, acceptExitCodes: [0, 1], }); return result.exitCode === 0; @@ -1182,7 +1181,7 @@ async function resolveMostAheadBaseRef(cwd: string, normalizedBaseRef: string): const { stdout } = await runGitCommand( ["rev-list", "--left-right", "--count", `${normalizedBaseRef}...origin/${normalizedBaseRef}`], - { cwd, env: READ_ONLY_GIT_ENV }, + { cwd, envOverlay: READ_ONLY_GIT_ENV }, ); const [localOnlyRaw, originOnlyRaw] = stdout.trim().split(/\s+/); const localOnly = Number.parseInt(localOnlyRaw ?? "0", 10); @@ -1209,7 +1208,7 @@ async function getAheadBehind( const comparisonBaseRef = await resolveBestComparisonBaseRef(cwd, baseRef); const { stdout } = await runGitCommand( ["rev-list", "--left-right", "--count", `${comparisonBaseRef}...${currentBranch}`], - { cwd, env: READ_ONLY_GIT_ENV }, + { cwd, envOverlay: READ_ONLY_GIT_ENV }, ); const [behindRaw, aheadRaw] = stdout.trim().split(/\s+/); const behind = Number.parseInt(behindRaw ?? "0", 10); @@ -1227,7 +1226,7 @@ async function getAheadOfOrigin(cwd: string, currentBranch: string): Promise 0) { throw new Error("Working directory has uncommitted changes."); diff --git a/packages/server/src/utils/executable.ts b/packages/server/src/utils/executable.ts index 75c5b8612..001f4e63f 100644 --- a/packages/server/src/utils/executable.ts +++ b/packages/server/src/utils/executable.ts @@ -1,7 +1,11 @@ -import { spawn, type ChildProcess } from "node:child_process"; +import type { ChildProcess } from "node:child_process"; import { createRequire } from "node:module"; import { existsSync } from "node:fs"; import { extname } from "node:path"; +import { spawnProcess } from "./spawn.js"; +import { isWindowsCommandScript } from "./windows-command.js"; + +export { quoteWindowsArgument, quoteWindowsCommand } from "./windows-command.js"; type Which = (command: string, options: { all: true }) => Promise; @@ -35,11 +39,6 @@ async function enumerateCandidates(name: string): Promise { }); } -export function isWindowsCommandScript(executablePath: string): boolean { - const extension = extname(executablePath).toLowerCase(); - return process.platform === "win32" && (extension === ".cmd" || extension === ".bat"); -} - async function probeExecutable(executablePath: string): Promise { return await new Promise((resolve) => { let pendingResolve: ((result: boolean) => void) | null = resolve; @@ -60,9 +59,8 @@ async function probeExecutable(executablePath: string): Promise { let child: ChildProcess; try { - child = spawn(executablePath, ["--version"], { + child = spawnProcess(executablePath, ["--version"], { stdio: "ignore", - windowsHide: true, // Windows batch shims (.cmd/.bat) require cmd.exe; native binaries do not. shell: isWindowsCommandScript(executablePath), }); @@ -130,39 +128,3 @@ export async function findExecutable(name: string): Promise { export async function isCommandAvailable(command: string): Promise { return (await findExecutable(command)) !== null; } - -function escapeWindowsCmdValue(value: string): string { - if (process.platform !== "win32") return value; - - const isQuoted = value.startsWith('"') && value.endsWith('"'); - const unquoted = isQuoted ? value.slice(1, -1) : value; - const escaped = unquoted.replace(/%/g, "%%").replace(/([&|^<>()!])/g, "^$1"); - - if (isQuoted || /[\s"]/u.test(unquoted)) { - const quoted = escaped - .replace(/(\\*)"/g, (_match, slashes: string) => `${slashes}${slashes}\\"`) - .replace(/\\+$/u, (slashes) => `${slashes}${slashes}`); - return `"${quoted}"`; - } - - return escaped; -} - -/** - * When spawning with `shell: true` on Windows, the command is passed to - * `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a - * command path with spaces (e.g. `C:\Program Files\...`) is split at the - * space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`. - */ -export function quoteWindowsCommand(command: string): string { - return escapeWindowsCmdValue(command); -} - -/** - * `spawn(..., { shell: true })` on Windows also passes argv through `cmd.exe`. - * Any argument containing spaces must be quoted or it will be split before the - * child process sees it. - */ -export function quoteWindowsArgument(argument: string): string { - return escapeWindowsCmdValue(argument); -} diff --git a/packages/server/src/utils/run-git-command.ts b/packages/server/src/utils/run-git-command.ts index c0f48db7b..43cf48d9c 100644 --- a/packages/server/src/utils/run-git-command.ts +++ b/packages/server/src/utils/run-git-command.ts @@ -1,4 +1,5 @@ import pLimit from "p-limit"; +import type { ProcessEnvRecord } from "../server/paseo-env.js"; import { spawnProcess } from "./spawn.js"; const DEFAULT_TIMEOUT_MS = 30_000; @@ -10,7 +11,8 @@ const gitLimit = pLimit(gitConcurrency); export interface GitCommandOptions { cwd: string; - env?: NodeJS.ProcessEnv; + env?: ProcessEnvRecord; + envOverlay?: ProcessEnvRecord; timeout?: number; maxOutputBytes?: number; acceptExitCodes?: number[]; @@ -24,6 +26,19 @@ export interface GitCommandResult { signal: NodeJS.Signals | null; } +function mergeEnvOverlays( + env: ProcessEnvRecord | undefined, + envOverlay: ProcessEnvRecord | undefined, +): ProcessEnvRecord | undefined { + if (!env) { + return envOverlay; + } + if (!envOverlay) { + return env; + } + return { ...env, ...envOverlay }; +} + export function runGitCommand( args: string[], options: GitCommandOptions, @@ -38,7 +53,7 @@ export function runGitCommand( const child = spawnProcess("git", args, { cwd: options.cwd, - env: options.env, + envOverlay: mergeEnvOverlays(options.env, options.envOverlay), stdio: ["ignore", "pipe", "pipe"], }); diff --git a/packages/server/src/utils/spawn.test.ts b/packages/server/src/utils/spawn.test.ts index 117a57edb..65cf2ea74 100644 --- a/packages/server/src/utils/spawn.test.ts +++ b/packages/server/src/utils/spawn.test.ts @@ -3,7 +3,24 @@ import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test } from "vitest"; -import { execCommand } from "./spawn.js"; +import { execCommand, spawnProcess } from "./spawn.js"; + +const printEnvScript = ` +const keys = [ + "CUSTOM", + "ELECTRON_NO_ATTACH_CONSOLE", + "ELECTRON_RUN_AS_NODE", + "PASEO_DESKTOP_MANAGED", + "PASEO_NODE_ENV", + "PASEO_SUPERVISED", +]; +const values = Object.fromEntries(keys.map((key) => [key, process.env[key] ?? null])); +console.log(JSON.stringify(values)); +`; + +function parsePrintedEnv(stdout: string): Record { + return JSON.parse(stdout.trim()) as Record; +} describe("execCommand", () => { const tempDirs: string[] = []; @@ -51,4 +68,116 @@ describe("execCommand", () => { expect(result.stdout.trim()).toBe(cwd); expect(result.stderr).toBe(""); }); + + test("treats env as the replacement base and finalizes external command env", async () => { + const result = await execCommand(process.execPath, ["-e", printEnvScript], { + baseEnv: { + ELECTRON_RUN_AS_NODE: "0", + CUSTOM: "from-base", + PATH: process.env.PATH, + PASEO_NODE_ENV: "production", + PASEO_SUPERVISED: "1", + }, + env: { + CUSTOM: "from-env", + ELECTRON_NO_ATTACH_CONSOLE: "1", + PASEO_DESKTOP_MANAGED: "1", + PASEO_NODE_ENV: "test", + }, + envOverlay: { + CUSTOM: "from-overlay", + ELECTRON_RUN_AS_NODE: undefined, + }, + }); + + expect(parsePrintedEnv(result.stdout)).toEqual({ + CUSTOM: "from-overlay", + ELECTRON_NO_ATTACH_CONSOLE: null, + ELECTRON_RUN_AS_NODE: "1", + PASEO_DESKTOP_MANAGED: null, + PASEO_NODE_ENV: null, + PASEO_SUPERVISED: null, + }); + }); + + test("does not inherit process.env when env replacement is supplied", async () => { + process.env.PASEO_TEST_SHOULD_NOT_LEAK = "leaked"; + try { + const result = await execCommand( + process.execPath, + [ + "-e", + "console.log(JSON.stringify({ leaked: process.env.PASEO_TEST_SHOULD_NOT_LEAK ?? null }))", + ], + { + env: { + PATH: process.env.PATH, + }, + }, + ); + + expect(JSON.parse(result.stdout.trim())).toEqual({ leaked: null }); + } finally { + delete process.env.PASEO_TEST_SHOULD_NOT_LEAK; + } + }); + + test("spawnProcess finalizes external command env", async () => { + const child = spawnProcess(process.execPath, ["-e", printEnvScript], { + baseEnv: { + ELECTRON_RUN_AS_NODE: "0", + PATH: process.env.PATH, + PASEO_NODE_ENV: "production", + }, + envOverlay: { + CUSTOM: "spawn-overlay", + PASEO_SUPERVISED: "1", + }, + }); + + const stdoutChunks: Buffer[] = []; + const stderrChunks: Buffer[] = []; + child.stdout?.on("data", (chunk: Buffer) => stdoutChunks.push(chunk)); + child.stderr?.on("data", (chunk: Buffer) => stderrChunks.push(chunk)); + + const exitCode = await new Promise((resolve, reject) => { + child.on("error", reject); + child.on("close", resolve); + }); + + expect(Buffer.concat(stderrChunks).toString()).toBe(""); + expect(exitCode).toBe(0); + expect(parsePrintedEnv(Buffer.concat(stdoutChunks).toString())).toEqual({ + CUSTOM: "spawn-overlay", + ELECTRON_NO_ATTACH_CONSOLE: null, + ELECTRON_RUN_AS_NODE: "1", + PASEO_DESKTOP_MANAGED: null, + PASEO_NODE_ENV: null, + PASEO_SUPERVISED: null, + }); + }); + + test("internal env mode preserves Paseo-owned launcher env", async () => { + const result = await execCommand(process.execPath, ["-e", printEnvScript], { + envMode: "internal", + baseEnv: { + ELECTRON_RUN_AS_NODE: "1", + PATH: process.env.PATH, + PASEO_NODE_ENV: "production", + }, + envOverlay: { + CUSTOM: "internal", + PASEO_SUPERVISED: "1", + }, + }); + + expect(parsePrintedEnv(result.stdout)).toEqual({ + CUSTOM: "internal", + ELECTRON_NO_ATTACH_CONSOLE: null, + ELECTRON_RUN_AS_NODE: "1", + PASEO_DESKTOP_MANAGED: null, + PASEO_NODE_ENV: "production", + PASEO_SUPERVISED: "1", + }); + }); }); diff --git a/packages/server/src/utils/spawn.ts b/packages/server/src/utils/spawn.ts index f7a0b0881..c8073c4e1 100644 --- a/packages/server/src/utils/spawn.ts +++ b/packages/server/src/utils/spawn.ts @@ -1,13 +1,27 @@ import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process"; +import { extname } from "node:path"; import { promisify } from "node:util"; -import { isWindowsCommandScript, quoteWindowsArgument, quoteWindowsCommand } from "./executable.js"; +import { createExternalCommandProcessEnv, type ProcessEnvRecord } from "../server/paseo-env.js"; +import { + isWindowsCommandScript, + quoteWindowsArgument, + quoteWindowsCommand, +} from "./windows-command.js"; const execFileAsync = promisify(execFile); -interface ExecCommandOptions { +interface ExternalEnvOptions { + baseEnv?: ProcessEnvRecord; + envMode?: "external" | "internal"; + env?: ProcessEnvRecord; + envOverlay?: ProcessEnvRecord; +} + +export type SpawnProcessOptions = Omit & ExternalEnvOptions; + +interface ExecCommandOptions extends ExternalEnvOptions { cwd?: string; - env?: NodeJS.ProcessEnv; encoding?: BufferEncoding; timeout?: number; maxBuffer?: number; @@ -18,20 +32,48 @@ interface ExecCommandResult { stderr: string; } +function hasPathSeparator(value: string): boolean { + return value.includes("/") || value.includes("\\"); +} + +function shouldUseWindowsShell( + command: string, + requestedShell?: boolean | string, +): boolean | string { + if (isWindowsCommandScript(command)) { + return true; + } + if (requestedShell !== undefined) { + return requestedShell; + } + return process.platform === "win32" && !hasPathSeparator(command) && !extname(command); +} + export function spawnProcess( command: string, args: string[], - options?: SpawnOptions, + options?: SpawnProcessOptions, ): ChildProcess { + const { baseEnv, env, envOverlay, ...spawnOptions } = options ?? {}; + const resolvedBaseEnv = env ?? baseEnv ?? process.env; const isWindows = process.platform === "win32"; - const shell = isWindowsCommandScript(command) ? true : (options?.shell ?? isWindows); + const shell = shouldUseWindowsShell(command, spawnOptions.shell); const shouldQuoteForShell = isWindows && shell !== false; const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; + const childEnv = + options?.envMode === "internal" + ? ({ ...resolvedBaseEnv, ...envOverlay } as NodeJS.ProcessEnv) + : createExternalCommandProcessEnv( + command, + resolvedBaseEnv, + ...(envOverlay ? [envOverlay] : []), + ); return spawn(resolvedCommand, resolvedArgs, { - ...options, + ...spawnOptions, + env: childEnv, shell, windowsHide: true, }); @@ -42,15 +84,25 @@ export async function execCommand( args: string[], options?: ExecCommandOptions, ): Promise { + const { baseEnv, env, envOverlay } = options ?? {}; + const resolvedBaseEnv = env ?? baseEnv ?? process.env; const isWindows = process.platform === "win32"; - const shell = isWindowsCommandScript(command) ? true : isWindows; + const shell = shouldUseWindowsShell(command); const shouldQuoteForShell = isWindows && shell !== false; const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; + const childEnv = + options?.envMode === "internal" + ? ({ ...resolvedBaseEnv, ...envOverlay } as NodeJS.ProcessEnv) + : createExternalCommandProcessEnv( + command, + resolvedBaseEnv, + ...(envOverlay ? [envOverlay] : []), + ); return execFileAsync(resolvedCommand, resolvedArgs, { cwd: options?.cwd, - env: options?.env, + env: childEnv, encoding: options?.encoding ?? "utf8", timeout: options?.timeout, maxBuffer: options?.maxBuffer, diff --git a/packages/server/src/utils/windows-command.ts b/packages/server/src/utils/windows-command.ts new file mode 100644 index 000000000..605742ce3 --- /dev/null +++ b/packages/server/src/utils/windows-command.ts @@ -0,0 +1,42 @@ +import { extname } from "node:path"; + +export function isWindowsCommandScript(executablePath: string): boolean { + const extension = extname(executablePath).toLowerCase(); + return process.platform === "win32" && (extension === ".cmd" || extension === ".bat"); +} + +function escapeWindowsCmdValue(value: string): string { + if (process.platform !== "win32") return value; + + const isQuoted = value.startsWith('"') && value.endsWith('"'); + const unquoted = isQuoted ? value.slice(1, -1) : value; + const escaped = unquoted.replace(/%/g, "%%").replace(/([&|^<>()!])/g, "^$1"); + + if (isQuoted || /[\s"]/u.test(unquoted)) { + const quoted = escaped + .replace(/(\\*)"/g, (_match, slashes: string) => `${slashes}${slashes}\\"`) + .replace(/\\+$/u, (slashes) => `${slashes}${slashes}`); + return `"${quoted}"`; + } + + return escaped; +} + +/** + * When spawning with `shell: true` on Windows, the command is passed to + * `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a + * command path with spaces (e.g. `C:\Program Files\...`) is split at the + * space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`. + */ +export function quoteWindowsCommand(command: string): string { + return escapeWindowsCmdValue(command); +} + +/** + * `spawn(..., { shell: true })` on Windows also passes argv through `cmd.exe`. + * Any argument containing spaces must be quoted or it will be split before the + * child process sees it. + */ +export function quoteWindowsArgument(argument: string): string { + return escapeWindowsCmdValue(argument); +} diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index f81c6becb..767f51be4 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -1,4 +1,4 @@ -import { execFile, spawn } from "child_process"; +import { execFile } from "child_process"; import { promisify } from "util"; import { existsSync, mkdirSync, realpathSync, rmSync, statSync } from "fs"; import { rm, stat } from "fs/promises"; @@ -27,15 +27,16 @@ import { writePaseoWorktreeRuntimeMetadata, } from "./worktree-metadata.js"; import { runGitCommand } from "./run-git-command.js"; +import { spawnProcess } from "./spawn.js"; import { resolvePaseoHome } from "../server/paseo-home.js"; +import { createExternalProcessEnv } from "../server/paseo-env.js"; import { ensureNodePtySpawnHelperExecutableForCurrentPlatform } from "../terminal/terminal.js"; import { parseGitRevParsePath, resolveGitRevParsePath } from "./git-rev-parse-path.js"; const execFileAsync = promisify(execFile); -const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = { - ...process.env, +const READ_ONLY_GIT_ENV = { GIT_OPTIONAL_LOCKS: "0", -}; +} as const; export interface WorktreeConfig { branchName: string; @@ -451,7 +452,7 @@ async function execSetupCommandStreamed(options: { const spawnWithPipes = () => { const shellInvocation = buildStringCommandShellInvocation({ command: options.command }); - const child = spawn(shellInvocation.shell, shellInvocation.args, { + const child = spawnProcess(shellInvocation.shell, shellInvocation.args, { cwd: options.cwd, env: options.env, stdio: ["ignore", "pipe", "pipe"], @@ -562,7 +563,7 @@ async function inferRepoRootPathFromWorktreePath(worktreePath: string): Promise< try { const { stdout } = await runGitCommand(["rev-parse", "--show-toplevel"], { cwd: worktreePath, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const topLevel = parseGitRevParsePath(stdout); if (topLevel) { @@ -596,10 +597,7 @@ export async function runWorktreeSetupCommands(options: { branchName: options.branchName, ...(options.repoRootPath ? { repoRootPath: options.repoRootPath } : {}), })); - const setupEnv = { - ...process.env, - ...runtimeEnv, - }; + const setupEnv = createExternalProcessEnv(process.env, runtimeEnv); const results: WorktreeSetupCommandResult[] = []; for (const [index, cmd] of setupCommands.entries()) { @@ -643,7 +641,7 @@ async function resolveBranchNameForWorktreePath(worktreePath: string): Promise 0) { @@ -707,8 +705,7 @@ export async function runWorktreeTeardownCommands(options: { options.branchName ?? (await resolveBranchNameForWorktreePath(options.worktreePath)); const worktreePort = readPaseoWorktreeRuntimePort(options.worktreePath); - const teardownEnv: NodeJS.ProcessEnv = { - ...process.env, + const teardownEnv: NodeJS.ProcessEnv = createExternalProcessEnv(process.env, { // Source checkout path is the original git repo root (shared across worktrees), not the // worktree itself. This allows lifecycle scripts to copy or clean resources using paths // from the source checkout. @@ -718,7 +715,7 @@ export async function runWorktreeTeardownCommands(options: { PASEO_WORKTREE_PATH: options.worktreePath, PASEO_BRANCH_NAME: branchName, ...(worktreePort !== null ? { PASEO_WORKTREE_PORT: String(worktreePort) } : {}), - }; + }); const results: WorktreeTeardownCommandResult[] = []; for (const cmd of teardownCommands) { @@ -746,7 +743,7 @@ export async function runWorktreeTeardownCommands(options: { export async function getGitCommonDir(cwd: string): Promise { const { stdout } = await runGitCommand(["rev-parse", "--git-common-dir"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const commonDir = resolveGitRevParsePath(cwd, stdout); if (!commonDir) { @@ -986,7 +983,7 @@ export async function listPaseoWorktrees({ const worktreesRoot = await getPaseoWorktreesRoot(cwd, paseoHome); const { stdout } = await runGitCommand(["worktree", "list", "--porcelain"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const rootPrefix = normalizePathForOwnership(worktreesRoot) + sep; @@ -1015,7 +1012,7 @@ export async function resolveExistingWorktreeForSlug({ const { stdout } = await runGitCommand(["branch", "--show-current"], { cwd: existingWorktree.path, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); const branchName = stdout.trim(); if (!branchName) { @@ -1046,7 +1043,7 @@ export async function resolvePaseoWorktreeRootForCwd( try { const { stdout } = await runGitCommand(["rev-parse", "--show-toplevel"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); worktreeRoot = parseGitRevParsePath(stdout); } catch { @@ -1424,7 +1421,7 @@ async function resolveUniqueLocalBranchName(cwd: string, candidateBranch: string async function isBranchCheckedOut(cwd: string, branchName: string): Promise { const { stdout } = await runGitCommand(["worktree", "list", "--porcelain"], { cwd, - env: READ_ONLY_GIT_ENV, + envOverlay: READ_ONLY_GIT_ENV, }); return parseWorktreeList(stdout).some((entry) => entry.branchName === branchName); }