From f6bad8f6bf95848e481098016433280f6508a1e2 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Tue, 5 May 2026 19:30:14 -0700 Subject: [PATCH] Sanitize remote execution envs at the boundary (#5325) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Adapters spawn CLIs against local, SSH, and sandbox targets, threading a runtime env through `runAdapterExecutionTargetProcess` and the SSH/sandbox runners > - Host identity vars (HOME, TMPDIR, XDG_*, NVM_DIR, PATH) routinely leak into the env we send to remote targets — sometimes via test probes, sometimes via runtime config — and break sandboxed/SSH'd CLIs whose own profiles set those values correctly > - The sanitization logic existed but lived alongside other helpers in `server-utils.ts` and was applied piecemeal at adapter callsites, so it was easy to bypass > - This pull request lifts the sanitization into a standalone `remote-execution-env.ts`, applies it at the SSH and sandbox runtime boundary so every remote spawn goes through it, and removes the duplicated callsite-level filtering > - The benefit is identity-bound host env stops leaking across SSH/sandbox transports regardless of which adapter calls in ## What Changed - `packages/adapter-utils/src/remote-execution-env.ts`: new module — single source of truth for which env keys are identity-bound and how to strip them when the value matches the host's value - `packages/adapter-utils/src/server-utils.ts`: remove the inline sanitization (now in `remote-execution-env.ts`) - `packages/adapter-utils/src/execution-target.ts`: apply sanitization at the sandbox runtime boundary - `packages/adapter-utils/src/ssh.ts`: apply sanitization at the SSH spawn boundary - `packages/adapters/opencode-local/src/server/test.ts`: drop now-redundant callsite filtering - `packages/adapters/pi-local/src/server/test.ts`: drop now-redundant callsite filtering - New tests `execution-target.test.ts` and `execution-target-sandbox.test.ts` cover the sanitizer flow at both transports, including positive cases (host-shaped path stripped) and explicit-override preservation ## Verification - `pnpm vitest run --no-coverage --project @paperclipai/adapter-utils --project @paperclipai/adapter-opencode-local --project @paperclipai/adapter-pi-local` - `pnpm typecheck` clean ## Risks Low–medium. The sanitization is now applied at one layer (boundary) instead of N (callsites), so behavior is more consistent. Any adapter that previously relied on a leaked host var landing on the remote shell would now see it stripped — but those reliances were what this change exists to fix. ## Model Used Claude Opus 4.7 (1M context) ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable — new tests at both transports - [x] If this change affects the UI, I have included before/after screenshots — N/A (no UI) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --- .../src/execution-target-sandbox.test.ts | 87 +++++++++++++ .../src/execution-target.test.ts | 122 +++++++++++++++++- .../adapter-utils/src/execution-target.ts | 24 +++- .../adapter-utils/src/remote-execution-env.ts | 49 +++++++ packages/adapter-utils/src/server-utils.ts | 50 +------ packages/adapter-utils/src/ssh.ts | 24 +++- .../opencode-local/src/server/test.ts | 9 +- packages/adapters/pi-local/src/server/test.ts | 13 +- 8 files changed, 306 insertions(+), 72 deletions(-) create mode 100644 packages/adapter-utils/src/remote-execution-env.ts diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index e0ebb37c..91bfd57d 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -19,6 +19,7 @@ describe("sandbox adapter execution targets", () => { const cleanupDirs: string[] = []; afterEach(async () => { + vi.unstubAllEnvs(); while (cleanupDirs.length > 0) { const dir = cleanupDirs.pop(); if (!dir) continue; @@ -141,6 +142,92 @@ describe("sandbox adapter execution targets", () => { })); }); + it("strips inherited host identity env before sandbox execution", async () => { + vi.stubEnv("PATH", "/host/bin:/usr/bin"); + vi.stubEnv("HOME", "/Users/local"); + vi.stubEnv("TMPDIR", "/var/folders/local/T"); + + const runner = { + execute: vi.fn(async () => ({ + exitCode: 0, + signal: null, + timedOut: false, + stdout: "ok\n", + stderr: "", + pid: null, + startedAt: new Date().toISOString(), + })), + }; + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + remoteCwd: "/workspace", + runner, + }; + + await runAdapterExecutionTargetProcess("run-1b", target, "agent-cli", ["--json"], { + cwd: "/local/workspace", + env: { + PATH: "/host/bin:/usr/bin", + HOME: "/Users/local", + TMPDIR: "/var/folders/local/T", + SAFE_VALUE: "visible", + }, + timeoutSec: 5, + graceSec: 1, + onLog: async () => {}, + }); + + expect(runner.execute).toHaveBeenCalledWith(expect.objectContaining({ + env: { + SAFE_VALUE: "visible", + }, + })); + }); + + it("preserves explicit remote identity env overrides for sandbox execution", async () => { + vi.stubEnv("PATH", "/host/bin:/usr/bin"); + vi.stubEnv("HOME", "/Users/local"); + + const runner = { + execute: vi.fn(async () => ({ + exitCode: 0, + signal: null, + timedOut: false, + stdout: "ok\n", + stderr: "", + pid: null, + startedAt: new Date().toISOString(), + })), + }; + const target: AdapterSandboxExecutionTarget = { + kind: "remote", + transport: "sandbox", + remoteCwd: "/workspace", + runner, + }; + + await runAdapterExecutionTargetProcess("run-1c", target, "agent-cli", ["--json"], { + cwd: "/local/workspace", + env: { + PATH: "/custom/remote/bin:/usr/bin", + HOME: "/home/sandbox", + SAFE_VALUE: "visible", + }, + timeoutSec: 5, + graceSec: 1, + onLog: async () => {}, + }); + + expect(runner.execute).toHaveBeenCalledWith(expect.objectContaining({ + env: { + PATH: "/custom/remote/bin:/usr/bin", + HOME: "/home/sandbox", + SAFE_VALUE: "visible", + }, + })); + }); + it("treats SSH targets as bridge-only", () => { const target = { kind: "remote" as const, diff --git a/packages/adapter-utils/src/execution-target.test.ts b/packages/adapter-utils/src/execution-target.test.ts index 71bcc344..22b04ab8 100644 --- a/packages/adapter-utils/src/execution-target.test.ts +++ b/packages/adapter-utils/src/execution-target.test.ts @@ -1,15 +1,18 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import * as ssh from "./ssh.js"; +import * as serverUtils from "./server-utils.js"; import { adapterExecutionTargetUsesManagedHome, ensureAdapterExecutionTargetRuntimeCommandInstalled, resolveAdapterExecutionTargetCwd, + runAdapterExecutionTargetProcess, runAdapterExecutionTargetShellCommand, } from "./execution-target.js"; describe("runAdapterExecutionTargetShellCommand", () => { afterEach(() => { vi.restoreAllMocks(); + vi.unstubAllEnvs(); }); it("quotes remote shell commands with the shared SSH quoting helper", async () => { @@ -42,16 +45,68 @@ describe("runAdapterExecutionTargetShellCommand", () => { }, ); + // runSshCommand owns profile sourcing and the outer `sh -lc` wrapper — + // the caller passes the raw command string. Wrapping it here would + // double-nest the login shell and re-source profiles after the explicit + // env override, silently undoing identity-var preservation. expect(runSshCommandSpy).toHaveBeenCalledWith( expect.objectContaining({ host: "ssh.example.test", username: "ssh-user", }), - `sh -lc ${ssh.shellQuote(`printf '%s\\n' "$HOME" && echo "it's ok"`)}`, + `printf '%s\\n' "$HOME" && echo "it's ok"`, expect.any(Object), ); }); + it("sanitizes inherited host env before SSH shell execution", async () => { + vi.stubEnv("PATH", "/host/bin:/usr/bin"); + vi.stubEnv("HOME", "/Users/local"); + + const runSshCommandSpy = vi.spyOn(ssh, "runSshCommand").mockResolvedValue({ + stdout: "", + stderr: "", + }); + + await runAdapterExecutionTargetShellCommand( + "run-1b", + { + kind: "remote", + transport: "ssh", + remoteCwd: "/srv/paperclip/workspace", + spec: { + host: "ssh.example.test", + port: 22, + username: "ssh-user", + remoteCwd: "/srv/paperclip/workspace", + remoteWorkspacePath: "/srv/paperclip/workspace", + privateKey: null, + knownHosts: null, + strictHostKeyChecking: true, + }, + }, + "env", + { + cwd: "/tmp/local", + env: { + PATH: "/host/bin:/usr/bin", + HOME: "/Users/local", + SAFE_VALUE: "visible", + }, + }, + ); + + expect(runSshCommandSpy).toHaveBeenCalledWith( + expect.any(Object), + expect.any(String), + expect.objectContaining({ + env: { + SAFE_VALUE: "visible", + }, + }), + ); + }); + it("returns a timedOut result when the SSH shell command times out", async () => { vi.spyOn(ssh, "runSshCommand").mockRejectedValue(Object.assign(new Error("timed out"), { code: "ETIMEDOUT", @@ -162,6 +217,71 @@ describe("runAdapterExecutionTargetShellCommand", () => { }); }); +describe("runAdapterExecutionTargetProcess", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + }); + + it("sanitizes inherited host env before SSH process execution", async () => { + vi.stubEnv("PATH", "/host/bin:/usr/bin"); + vi.stubEnv("HOME", "/Users/local"); + + const runChildProcessSpy = vi.spyOn(serverUtils, "runChildProcess").mockResolvedValue({ + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + pid: null, + startedAt: new Date().toISOString(), + }); + + await runAdapterExecutionTargetProcess( + "run-ssh-process", + { + kind: "remote", + transport: "ssh", + remoteCwd: "/srv/paperclip/workspace", + spec: { + host: "ssh.example.test", + port: 22, + username: "ssh-user", + remoteCwd: "/srv/paperclip/workspace", + remoteWorkspacePath: "/srv/paperclip/workspace", + privateKey: null, + knownHosts: null, + strictHostKeyChecking: true, + }, + }, + "agent-cli", + ["--json"], + { + cwd: "/tmp/local", + env: { + PATH: "/host/bin:/usr/bin", + HOME: "/Users/local", + SAFE_VALUE: "visible", + }, + timeoutSec: 5, + graceSec: 1, + onLog: async () => {}, + }, + ); + + expect(runChildProcessSpy).toHaveBeenCalledWith( + "run-ssh-process", + "agent-cli", + ["--json"], + expect.objectContaining({ + env: { + SAFE_VALUE: "visible", + }, + }), + ); + }); +}); + describe("ensureAdapterExecutionTargetRuntimeCommandInstalled", () => { afterEach(() => { vi.restoreAllMocks(); diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 563d1795..b51780d5 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -26,6 +26,7 @@ import { type RunProcessResult, type TerminalResultCleanupOptions, } from "./server-utils.js"; +import { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js"; import { preferredShellForSandbox } from "./sandbox-shell.js"; export interface AdapterLocalExecutionTarget { @@ -95,6 +96,8 @@ export interface AdapterExecutionTargetPaperclipBridgeHandle { stop(): Promise; } +export { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js"; + function parseObject(value: unknown): Record { return value && typeof value === "object" && !Array.isArray(value) ? (value as Record) @@ -340,11 +343,12 @@ export async function runAdapterExecutionTargetProcess( ): Promise { if (target?.kind === "remote" && target.transport === "sandbox") { const runner = requireSandboxRunner(target); + const env = sanitizeRemoteExecutionEnv(options.env); return await runner.execute({ command, args, cwd: target.remoteCwd, - env: options.env, + env, stdin: options.stdin, timeoutMs: options.timeoutSec > 0 ? options.timeoutSec * 1000 : target.timeoutMs ?? undefined, onLog: options.onLog, @@ -354,9 +358,14 @@ export async function runAdapterExecutionTargetProcess( }); } + const env = + target?.kind === "remote" && target.transport === "ssh" + ? sanitizeRemoteExecutionEnv(options.env) + : options.env; + return await runChildProcess(runId, command, args, { cwd: options.cwd, - env: options.env, + env, stdin: options.stdin, timeoutSec: options.timeoutSec, graceSec: options.graceSec, @@ -376,9 +385,16 @@ export async function runAdapterExecutionTargetShellCommand( const onLog = options.onLog ?? (async () => {}); if (target?.kind === "remote") { const startedAt = new Date().toISOString(); + const env = sanitizeRemoteExecutionEnv(options.env); if (target.transport === "ssh") { try { - const result = await runSshCommand(target.spec, `sh -lc ${shellQuote(command)}`, { + // Pass the raw command — `runSshCommand` owns profile sourcing and + // the outer `sh -lc` wrapper. Wrapping again here would nest a second + // `sh -lc` after the explicit `env KEY=VAL` overrides, re-sourcing + // login profiles AFTER the override and silently undoing any + // identity var (NVM_DIR / PATH / etc.) that a profile re-exports. + const result = await runSshCommand(target.spec, command, { + env, timeoutMs: (options.timeoutSec ?? 15) * 1000, }); if (result.stdout) await onLog("stdout", result.stdout); @@ -435,7 +451,7 @@ export async function runAdapterExecutionTargetShellCommand( command: shellCommand, args: ["-lc", command], cwd: target.remoteCwd, - env: options.env, + env, timeoutMs: (options.timeoutSec ?? 15) * 1000, onLog, }); diff --git a/packages/adapter-utils/src/remote-execution-env.ts b/packages/adapter-utils/src/remote-execution-env.ts new file mode 100644 index 00000000..9bc90022 --- /dev/null +++ b/packages/adapter-utils/src/remote-execution-env.ts @@ -0,0 +1,49 @@ +const REMOTE_EXECUTION_ENV_IDENTITY_KEYS = new Set([ + "PATH", + "HOME", + "PWD", + "SHELL", + "USER", + "LOGNAME", + "NVM_DIR", + "TMPDIR", + "TMP", + "TEMP", + "XDG_CONFIG_HOME", + "XDG_CACHE_HOME", + "XDG_DATA_HOME", + "XDG_STATE_HOME", + "XDG_RUNTIME_DIR", +]); + +function readEnvValueCaseInsensitive(env: NodeJS.ProcessEnv, key: string): string | undefined { + const direct = env[key]; + if (typeof direct === "string") return direct; + const upper = key.toUpperCase(); + for (const [candidateKey, candidateValue] of Object.entries(env)) { + if (candidateKey.toUpperCase() === upper && typeof candidateValue === "string") { + return candidateValue; + } + } + return undefined; +} + +export function sanitizeRemoteExecutionEnv( + env: Record, + inheritedEnv: NodeJS.ProcessEnv = process.env, +): Record { + const sanitized: Record = {}; + for (const [key, value] of Object.entries(env)) { + const normalizedKey = key.toUpperCase(); + if (!REMOTE_EXECUTION_ENV_IDENTITY_KEYS.has(normalizedKey)) { + sanitized[key] = value; + continue; + } + const inheritedValue = readEnvValueCaseInsensitive(inheritedEnv, key); + if (typeof inheritedValue === "string" && inheritedValue === value) { + continue; + } + sanitized[key] = value; + } + return sanitized; +} diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 1a07f643..83420182 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -2,6 +2,7 @@ import { spawn, type ChildProcess } from "node:child_process"; import { createHash, randomUUID } from "node:crypto"; import { constants as fsConstants, promises as fs, type Dirent } from "node:fs"; import path from "node:path"; +import { sanitizeRemoteExecutionEnv } from "./remote-execution-env.js"; import { buildSshSpawnTarget, type SshRemoteExecutionSpec } from "./ssh.js"; import { redactCommandText } from "./command-redaction.js"; import type { @@ -1039,54 +1040,11 @@ function quoteForCmd(arg: string) { return /[\s"&<>|^()]/.test(escaped) ? `"${escaped}"` : escaped; } -const SSH_REMOTE_ENV_IDENTITY_KEYS = new Set([ - "PATH", - "HOME", - "PWD", - "SHELL", - "USER", - "LOGNAME", - "NVM_DIR", - "TMPDIR", - "TMP", - "TEMP", - "XDG_CONFIG_HOME", - "XDG_CACHE_HOME", - "XDG_DATA_HOME", - "XDG_STATE_HOME", - "XDG_RUNTIME_DIR", -]); - -function readEnvValueCaseInsensitive(env: NodeJS.ProcessEnv, key: string): string | undefined { - const direct = env[key]; - if (typeof direct === "string") return direct; - const upper = key.toUpperCase(); - for (const [candidateKey, candidateValue] of Object.entries(env)) { - if (candidateKey.toUpperCase() === upper && typeof candidateValue === "string") { - return candidateValue; - } - } - return undefined; -} - export function sanitizeSshRemoteEnv( env: Record, inheritedEnv: NodeJS.ProcessEnv = process.env, ): Record { - const sanitized: Record = {}; - for (const [key, value] of Object.entries(env)) { - const normalizedKey = key.toUpperCase(); - if (!SSH_REMOTE_ENV_IDENTITY_KEYS.has(normalizedKey)) { - sanitized[key] = value; - continue; - } - const inheritedValue = readEnvValueCaseInsensitive(inheritedEnv, key); - if (typeof inheritedValue === "string" && inheritedValue === value) { - continue; - } - sanitized[key] = value; - } - return sanitized; + return sanitizeRemoteExecutionEnv(env, inheritedEnv); } function resolveWindowsCmdShell(env: NodeJS.ProcessEnv): string { @@ -1114,9 +1072,9 @@ async function resolveSpawnTarget( spec: remote, command, args, - env: sanitizeSshRemoteEnv(Object.fromEntries( + env: Object.fromEntries( Object.entries(options.remoteEnv ?? {}).filter((entry): entry is [string, string] => typeof entry[1] === "string"), - )), + ), }); return { command: sshResolved, diff --git a/packages/adapter-utils/src/ssh.ts b/packages/adapter-utils/src/ssh.ts index 39087b3e..2b5dbbad 100644 --- a/packages/adapter-utils/src/ssh.ts +++ b/packages/adapter-utils/src/ssh.ts @@ -721,6 +721,7 @@ export async function runSshCommand( config: SshConnectionConfig, remoteCommand: string, options: { + env?: Record; timeoutMs?: number; maxBuffer?: number; } = {}, @@ -730,12 +731,33 @@ export async function runSshCommand( const auth = await createSshAuthArgs(config); cleanup = auth.cleanup; const sshArgs = [...auth.args]; + const envEntries = Object.entries(options.env ?? {}) + .filter((entry): entry is [string, string] => typeof entry[1] === "string"); + for (const [key] of envEntries) { + if (!isValidShellEnvKey(key)) { + throw new Error(`Invalid SSH environment variable key: ${key}`); + } + } + + // Mirror buildSshSpawnTarget: source login profiles first, then run + // `env KEY=VAL cmd` so user-supplied identity overrides win over anything + // a profile re-exports. Without this, a remote profile that resets HOME + // / NVM_DIR / etc. would silently undo the explicit env passed in here. + const envArgs = envEntries.map(([key, value]) => `${key}=${shellQuote(value)}`); + const remoteScript = [ + 'if [ -f "$HOME/.profile" ]; then . "$HOME/.profile" >/dev/null 2>&1 || true; fi', + 'if [ -f "$HOME/.bash_profile" ]; then . "$HOME/.bash_profile" >/dev/null 2>&1 || true; fi', + 'if [ -f "$HOME/.zprofile" ]; then . "$HOME/.zprofile" >/dev/null 2>&1 || true; fi', + envArgs.length > 0 + ? `exec env ${envArgs.join(" ")} sh -c ${shellQuote(remoteCommand)}` + : `exec sh -c ${shellQuote(remoteCommand)}`, + ].join(" && "); sshArgs.push( "-p", String(config.port), `${config.username}@${config.host}`, - remoteCommand, + `sh -lc ${shellQuote(remoteScript)}`, ); return await execFileText("ssh", sshArgs, { diff --git a/packages/adapters/opencode-local/src/server/test.ts b/packages/adapters/opencode-local/src/server/test.ts index a4758084..e0959d5f 100644 --- a/packages/adapters/opencode-local/src/server/test.ts +++ b/packages/adapters/opencode-local/src/server/test.ts @@ -290,13 +290,6 @@ export async function testEnvironment( if (variant) args.push("--variant", variant); if (extraArgs.length > 0) args.push(...extraArgs); - // For remote targets, do NOT spread the host process.env into the - // probe env: it leaks macOS-only paths (HOME=/Users/..., host - // XDG_CONFIG_HOME, TMPDIR, etc.) into the remote shell, which causes - // opencode on the remote box to try to mkdir host paths like /Users. - // Match the pattern used by claude_local / codex_local / gemini_local - // probes: send only the user-configured adapter env across SSH. - const probeEnv = targetIsRemote ? preparedRuntimeConfig.env : runtimeEnv; try { const probe = await runAdapterExecutionTargetProcess( runId, @@ -305,7 +298,7 @@ export async function testEnvironment( args, { cwd, - env: probeEnv, + env: runtimeEnv, timeoutSec: 60, graceSec: 5, stdin: "Respond with hello.", diff --git a/packages/adapters/pi-local/src/server/test.ts b/packages/adapters/pi-local/src/server/test.ts index aef648ef..782699ce 100644 --- a/packages/adapters/pi-local/src/server/test.ts +++ b/packages/adapters/pi-local/src/server/test.ts @@ -259,17 +259,6 @@ export async function testEnvironment( args.push("--tools", "read"); if (extraArgs.length > 0) args.push(...extraArgs); - // For remote targets, do NOT spread the host process.env into the probe - // env: it leaks macOS-only PATH, HOME, TMPDIR, etc. into the remote shell. - // In particular the Mac PATH overrides the nvm-sourced PATH that - // buildSshSpawnTarget sets up, which on Linux SSH targets resolves `node` - // to /usr/bin/node (Node 18) instead of nvm's Node 22, causing pi-tui to - // crash with `Invalid regular expression flags` on its /v unicode regex. - // Match the pattern used by claude_local / codex_local / gemini_local / - // opencode_local probes: send only the user-configured adapter env across - // SSH. Local probes still get the full runtimeEnv. - const probeEnv = targetIsRemote ? normalizeEnv(env) : runtimeEnv; - try { const probe = await runAdapterExecutionTargetProcess( runId, @@ -278,7 +267,7 @@ export async function testEnvironment( args, { cwd, - env: probeEnv, + env: runtimeEnv, timeoutSec: 60, graceSec: 5, onLog: async () => {},