forked from farhoodlabs/paperclip
Sanitize remote execution envs at the boundary (#5325)
## 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
This commit is contained in:
@@ -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<string, string>,
|
||||
inheritedEnv: NodeJS.ProcessEnv = process.env,
|
||||
): Record<string, string> {
|
||||
const sanitized: Record<string, string> = {};
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user