mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-18 03:30:39 +09:00
Strip inherited host shell env from SSH remote execution (#5142)
## Thinking Path
> - Paperclip orchestrates AI agents for zero-human companies
> - Agents executing on remote SSH hosts receive an env map built from
the host
> process's env plus per-run additions like `PAPERCLIP_API_KEY`,
> `PAPERCLIP_RUN_ID`, etc.
> - The env map currently includes inherited host vars by default,
including
> identity-bound ones like `PATH`, `HOME`, `USER`, `NVM_DIR`, `XDG_*` —
> variables whose values are meaningful only on the host they came from
> - Sending the host's `PATH` (containing host-only directories like a
local
> nvm install path) to a remote SSH box overrides the remote's actual
`PATH`
> and breaks command resolution. Same hazard for `HOME` (commands
looking for
> config files end up in a non-existent dir), `USER` (writes go to the
wrong
> path), etc.
> - This PR adds `sanitizeSshRemoteEnv()` that drops inherited
identity-bound
> vars when their value matches the host process's value. Explicitly-set
> values pass through untouched, so callers that genuinely want to
override
> remote `PATH` etc. still can — but accidental leakage from
> `process.env` is filtered.
> - The benefit is that SSH remote execution stops corrupting the remote
> shell's environment with host-shaped paths, so commands resolve
correctly
> against the remote PATH and config files land in the remote `HOME`
## What Changed
- New `sanitizeSshRemoteEnv(env, inheritedEnv = process.env)` in
`packages/adapter-utils/src/server-utils.ts`. The identity-bound key set
is:
- `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`
For any key in this set, the entry is dropped iff the env value equals
the
inherited (host process) value. Other keys pass through unchanged.
- `readEnvValueCaseInsensitive(...)` helper handles Windows-style
case-insensitive env var lookups.
- Wired into `resolveSpawnTarget(...)` for the SSH transport. Sandbox
and local
paths are unaffected.
- Tests added in `server-utils.test.ts` (~50 lines) covering: matching
keys
filtered, mismatched keys preserved, non-identity keys passed through,
case
insensitivity.
## Verification
- `pnpm --filter @paperclipai/adapter-utils test -- server-utils`
- Manual QA: run any adapter against an SSH-backed environment, confirm
remote command resolution works (e.g. `node`, `npm`, the adapter's CLI)
and
config files land in the remote user's `HOME`. Compare to the prior
behaviour
by transiently re-introducing the inherited `PATH` and watching commands
fail with `command not found`.
## Risks
- Behavioural shift: SSH remote execution previously passed inherited
host env
vars verbatim. Code that relied on that (e.g. a remote command somehow
expecting the host's `PATH`) will see different behaviour. None of the
adapter code in this repo has such a dependency.
- Edge case: if a caller explicitly sets `PATH` to the same value as the
host's
`PATH` (literally — same exact string), the sanitizer drops it as a
leak.
In practice no caller constructs the env this way.
- Windows host: case-insensitive lookup handles `Path` vs `PATH`
correctly.
Tested.
## Model Used
- OpenAI GPT-5.4 (reasoning effort: high) via Codex CLI
- Provider: OpenAI
- Used to author the code changes in this PR
## 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
- [ ] If this change affects the UI, I have included before/after
screenshots — N/A
- [ ] I have updated relevant documentation to reflect my changes — N/A
- [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:
parent
90631b09b3
commit
6c090f84a9
2 changed files with 133 additions and 2 deletions
|
|
@ -12,6 +12,7 @@ import {
|
||||||
renderPaperclipWakePrompt,
|
renderPaperclipWakePrompt,
|
||||||
runningProcesses,
|
runningProcesses,
|
||||||
runChildProcess,
|
runChildProcess,
|
||||||
|
sanitizeSshRemoteEnv,
|
||||||
shapePaperclipWorkspaceEnvForExecution,
|
shapePaperclipWorkspaceEnvForExecution,
|
||||||
stringifyPaperclipWakePayload,
|
stringifyPaperclipWakePayload,
|
||||||
} from "./server-utils.js";
|
} from "./server-utils.js";
|
||||||
|
|
@ -61,6 +62,86 @@ describe("buildInvocationEnvForLogs", () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("sanitizeSshRemoteEnv", () => {
|
||||||
|
it("drops inherited host shell identity variables for SSH remote execution", () => {
|
||||||
|
expect(
|
||||||
|
sanitizeSshRemoteEnv(
|
||||||
|
{
|
||||||
|
PATH: "/host/bin:/usr/bin",
|
||||||
|
HOME: "/Users/local",
|
||||||
|
NVM_DIR: "/Users/local/.nvm",
|
||||||
|
TMPDIR: "/var/folders/local/T",
|
||||||
|
XDG_CONFIG_HOME: "/Users/local/.config",
|
||||||
|
SAFE_VALUE: "visible",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
PATH: "/host/bin:/usr/bin",
|
||||||
|
HOME: "/Users/local",
|
||||||
|
NVM_DIR: "/Users/local/.nvm",
|
||||||
|
TMPDIR: "/var/folders/local/T",
|
||||||
|
XDG_CONFIG_HOME: "/Users/local/.config",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
).toEqual({
|
||||||
|
SAFE_VALUE: "visible",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves explicit remote overrides even for filtered key names", () => {
|
||||||
|
expect(
|
||||||
|
sanitizeSshRemoteEnv(
|
||||||
|
{
|
||||||
|
PATH: "/custom/remote/bin:/usr/bin",
|
||||||
|
HOME: "/home/agent",
|
||||||
|
TMPDIR: "/tmp",
|
||||||
|
SAFE_VALUE: "visible",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
PATH: "/host/bin:/usr/bin",
|
||||||
|
HOME: "/Users/local",
|
||||||
|
TMPDIR: "/var/folders/local/T",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
).toEqual({
|
||||||
|
PATH: "/custom/remote/bin:/usr/bin",
|
||||||
|
HOME: "/home/agent",
|
||||||
|
TMPDIR: "/tmp",
|
||||||
|
SAFE_VALUE: "visible",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("filters identity keys via case-insensitive match against the inherited env", () => {
|
||||||
|
expect(
|
||||||
|
sanitizeSshRemoteEnv(
|
||||||
|
{
|
||||||
|
// Caller passed PATH in upper case while the inherited (Windows-style)
|
||||||
|
// host env exposes it as Path. The lookup must still treat them as
|
||||||
|
// equal so the leaked host PATH gets stripped.
|
||||||
|
PATH: "/host/bin:/usr/bin",
|
||||||
|
HOME: "/host/home",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Path: "/host/bin:/usr/bin",
|
||||||
|
home: "/host/home",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
).toEqual({});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("preserves explicitly-set identity keys when the inherited env disagrees in case but not in value", () => {
|
||||||
|
expect(
|
||||||
|
sanitizeSshRemoteEnv(
|
||||||
|
{
|
||||||
|
PATH: "/explicit/remote/bin",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Path: "/host/bin:/usr/bin",
|
||||||
|
},
|
||||||
|
),
|
||||||
|
).toEqual({ PATH: "/explicit/remote/bin" });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("materializePaperclipSkillCopy", () => {
|
describe("materializePaperclipSkillCopy", () => {
|
||||||
it("refuses to materialize into an ancestor of the source", async () => {
|
it("refuses to materialize into an ancestor of the source", async () => {
|
||||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-skill-copy-"));
|
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-skill-copy-"));
|
||||||
|
|
|
||||||
|
|
@ -1039,6 +1039,56 @@ function quoteForCmd(arg: string) {
|
||||||
return /[\s"&<>|^()]/.test(escaped) ? `"${escaped}"` : escaped;
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
function resolveWindowsCmdShell(env: NodeJS.ProcessEnv): string {
|
function resolveWindowsCmdShell(env: NodeJS.ProcessEnv): string {
|
||||||
const fallbackRoot = env.SystemRoot || process.env.SystemRoot || "C:\\Windows";
|
const fallbackRoot = env.SystemRoot || process.env.SystemRoot || "C:\\Windows";
|
||||||
return path.join(fallbackRoot, "System32", "cmd.exe");
|
return path.join(fallbackRoot, "System32", "cmd.exe");
|
||||||
|
|
@ -1064,9 +1114,9 @@ async function resolveSpawnTarget(
|
||||||
spec: remote,
|
spec: remote,
|
||||||
command,
|
command,
|
||||||
args,
|
args,
|
||||||
env: Object.fromEntries(
|
env: sanitizeSshRemoteEnv(Object.fromEntries(
|
||||||
Object.entries(options.remoteEnv ?? {}).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
|
Object.entries(options.remoteEnv ?? {}).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
|
||||||
),
|
)),
|
||||||
});
|
});
|
||||||
return {
|
return {
|
||||||
command: sshResolved,
|
command: sshResolved,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue