fix(adapter-utils): preserve legacy sandbox PATH with managed GitHub (#13051)
## Thinking Path > - Paperclip lets people manage AI agents and their work. > - Remote agents need both their installed tools and managed GitHub credentials. > - The GitHub launcher replaces a missing remote PATH with a small system path. > - Legacy images install agent CLIs outside that path, so those agents cannot start. > - This pull request preserves the remote toolchain and puts managed GitHub commands first. > - Command checks now use the same environment as execution. ## Linked Issues or Issue Description Refs #13005. Related: #10239 fixes a separate Cursor environment path issue. Searched open issues and PRs for sandbox PATH and GitHub launcher changes. No duplicate of this launcher fix was found. **What happened?** A remote Claude or Codex run passes command discovery, then fails with `command not found` and exit code 127. Managed GitHub launchers use only their own directory and `/usr/local/bin:/usr/bin:/bin`. This drops NVM and other toolchain directories from the sandbox path. **Expected behavior** Agent CLIs remain available on legacy and current sandbox images. Managed `git` and `gh` still resolve first and use the responsible person's credentials. **Steps to reproduce** 1. Use a sandbox whose agent CLI is installed in an NVM or other non-system bin directory. 2. Start an agent run with managed GitHub launchers and no explicit PATH override. 3. Observe that command discovery succeeds but the agent command exits with code 127. **Paperclip version or commit** Observed on `b97101893f0926f57ed0ce9ef1f8d3e4780c62c2`. The same launcher behavior remains on the base commit `be6bb768b`. **Deployment mode** Hosted server with remote sandbox execution. The shared launcher also supports SSH targets. ## What Changed - Read the remote target's effective PATH when no remote override is set. Do not copy an inherited controller PATH. - Prepend the managed launcher directory and retain the combined path in shell startup files. - Stop startup if path discovery fails. Frame the response so login banners cannot contaminate PATH. - Pass the sanitized launch environment to sandbox command checks, installation, and the second check. - Add real shell tests for legacy and current CLI layouts, quoted paths, managed GitHub command execution, explicit overrides, SSH, and failure cases. - Document the remote path contract. ## Verification - Four focused adapter utility suites passed: 153 tests. - The regression suite passed: 13 tests, including the Linux stdin handling fix. - `pnpm --filter @paperclipai/adapter-utils typecheck` passed. - Full workspace `pnpm -r typecheck` and `pnpm build` passed. - The regression suite fails on the unchanged base revision (12 failures, 1 pass) and passes with this change (13 passes). The baseline ran in an isolated scratch copy. - Full local test coverage was attempted using the official CI shards. The run was stopped after macOS Postgres shared-memory exhaustion and CLI timeouts under load. The affected server database suite passed in isolation (31 tests), as did the five affected DB/CLI suites (89 tests). - [Full Linux CI](https://github.com/paperclipai/paperclip/actions/runs/34258333833) passed on `4e1426f5b`: all general and serialized test shards, all browser shards, typecheck and release registry checks, native runner verification, application build, and release canary dry run. - Greptile scored the latest commit 5/5 with no unresolved findings. - Shell tests use isolated local fixtures and make no provider or model requests. No live sandbox qualification is claimed. ## Risks - Remote startup adds one bounded path query when no explicit override exists. A failed query stops startup. - Explicit remote path overrides still control which tools are available. Invalid overrides now fail the command check earlier. - Credential selection and GitHub broker policy are unchanged. Tests verify managed wrappers stay first and can invoke underlying commands. - No database migration or sandbox image replacement is required. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository editing, and terminal tools. The exact serving model ID and context-window size are not exposed in this session. ## 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 searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Dotta
GitHub
parent
db85bf4b7a
commit
3ad494aacd
|
|
@ -20,6 +20,16 @@ Server-side Git operations and GitHub gateway calls follow the same selection ru
|
|||
|
||||
Managed commands disable ambient Git credential helpers, Git global/system configuration, host GitHub CLI configuration, and host SSH identity access. Per-operation GitHub CLI configuration is isolated in a writable configuration directory beneath the managed launcher directory. Missing credentials clear previous author and token values; no teammate, standing delegation, host token, or company-default user's account is substituted. Anonymous/local operations remain available where supported.
|
||||
|
||||
Remote launchers prepend their directory to the execution target's effective
|
||||
`PATH`. An explicit remote `PATH` override is preserved; otherwise Paperclip
|
||||
reads the provider's environment before staging the launcher shell files.
|
||||
This keeps legacy NVM and user-local agent installations available alongside
|
||||
newer images with system-wide CLIs. The generated shell files retain that
|
||||
combined path with managed `git` and `gh` first. Sandbox command checks use
|
||||
the same sanitized environment as execution, so a CLI visible only in the
|
||||
provider's default environment cannot pass the launch check. Failed path
|
||||
discovery stops startup instead of silently falling back to a minimal path.
|
||||
|
||||
Scripts that previously read a persistent `GH_TOKEN` must use managed `git`, `gh`, or GitHub gateway tools. Managed execution skips legacy GitHub token bindings in agent, environment, project, and routine configuration before secret preflight. Configure personal or dedicated access through the GitHub connection instead. Directly invoking an unmanaged executable or retaining a token obtained during an earlier invocation is outside the managed invocation contract.
|
||||
|
||||
## Dedicated accounts and diagnostics
|
||||
|
|
|
|||
|
|
@ -678,6 +678,9 @@ export async function ensureAdapterExecutionTargetCommandResolvable(
|
|||
await ensureSandboxCommandResolvable(
|
||||
command,
|
||||
target,
|
||||
sanitizeRemoteExecutionEnv(Object.fromEntries(
|
||||
Object.entries(env).filter((entry): entry is [string, string] => typeof entry[1] === "string"),
|
||||
)),
|
||||
options.installCommand?.trim() || null,
|
||||
options.timeoutSec,
|
||||
);
|
||||
|
|
@ -691,6 +694,7 @@ export async function ensureAdapterExecutionTargetCommandResolvable(
|
|||
async function probeSandboxCommandResolvable(
|
||||
command: string,
|
||||
target: AdapterSandboxExecutionTarget,
|
||||
env: Record<string, string>,
|
||||
): Promise<{ resolved: boolean; timedOut: boolean; stderr: string }> {
|
||||
const runner = requireSandboxRunner(target);
|
||||
const probeScript = `command -v ${shellQuote(command)}`;
|
||||
|
|
@ -698,6 +702,7 @@ async function probeSandboxCommandResolvable(
|
|||
command: "sh",
|
||||
args: ["-c", probeScript],
|
||||
cwd: target.remoteCwd,
|
||||
env,
|
||||
timeoutMs: target.timeoutMs ?? 15_000,
|
||||
});
|
||||
return {
|
||||
|
|
@ -710,6 +715,7 @@ async function probeSandboxCommandResolvable(
|
|||
async function ensureSandboxCommandResolvable(
|
||||
command: string,
|
||||
target: AdapterSandboxExecutionTarget,
|
||||
env: Record<string, string>,
|
||||
installCommand: string | null,
|
||||
timeoutSec?: number | null,
|
||||
): Promise<void> {
|
||||
|
|
@ -720,7 +726,7 @@ async function ensureSandboxCommandResolvable(
|
|||
// the first step honestly reflects whether the binary is on PATH. The
|
||||
// sandbox provider is responsible for sourcing login profiles (e2b mirrors
|
||||
// SSH's buildSshSpawnTarget) so this and the hello probe agree on PATH.
|
||||
let probe = await probeSandboxCommandResolvable(command, target);
|
||||
let probe = await probeSandboxCommandResolvable(command, target, env);
|
||||
if (probe.resolved) return;
|
||||
if (probe.timedOut) {
|
||||
throw new Error(`Timed out checking command "${command}" on sandbox target.`);
|
||||
|
|
@ -742,6 +748,7 @@ async function ensureSandboxCommandResolvable(
|
|||
command: "sh",
|
||||
args: shellCommandArgs(installCommand),
|
||||
cwd: target.remoteCwd,
|
||||
env,
|
||||
timeoutMs: installTimeoutMs,
|
||||
});
|
||||
if (installResult.timedOut) {
|
||||
|
|
@ -755,7 +762,7 @@ async function ensureSandboxCommandResolvable(
|
|||
} catch (err) {
|
||||
installFailureDetail = `install command threw: ${err instanceof Error ? err.message : String(err)}`;
|
||||
}
|
||||
probe = await probeSandboxCommandResolvable(command, target);
|
||||
probe = await probeSandboxCommandResolvable(command, target, env);
|
||||
if (probe.resolved) return;
|
||||
if (probe.timedOut) {
|
||||
throw new Error(`Timed out checking command "${command}" on sandbox target.`);
|
||||
|
|
@ -1521,6 +1528,31 @@ export async function cleanupGitHubOperationLaunchers(input: GitHubLauncherLocat
|
|||
}
|
||||
}
|
||||
|
||||
async function githubOperationLauncherBasePath(
|
||||
target: AdapterCommandCapableExecutionTarget | null,
|
||||
env: Record<string, string>,
|
||||
): Promise<string> {
|
||||
if (!target) return env.PATH || process.env.PATH || "/usr/bin:/bin";
|
||||
const configuredPath = sanitizeRemoteExecutionEnv(env).PATH;
|
||||
if (configuredPath !== undefined) return configuredPath;
|
||||
|
||||
// The provider owns login/profile setup. Query its effective PATH before
|
||||
// staging BASH_ENV, rather than substituting the controller's toolchain or
|
||||
// a minimal PATH that hides legacy NVM/user-local agent installations.
|
||||
const result = await adapterExecutionTargetCommandRunner(target).execute({
|
||||
command: "sh",
|
||||
args: ["-c", "printf '\\000%s\\000' \"$PATH\""],
|
||||
cwd: target.remoteCwd,
|
||||
timeoutMs: 15_000,
|
||||
});
|
||||
// Frame the value so login banners cannot become executable search paths.
|
||||
const remotePath = result.stdout.match(/\0([^\0]+)\0/)?.[1];
|
||||
if (result.timedOut || result.exitCode !== 0 || !remotePath) {
|
||||
throw new Error("Could not resolve remote PATH for managed GitHub launchers");
|
||||
}
|
||||
return remotePath;
|
||||
}
|
||||
|
||||
/** Stage token-free launchers next to the execution, not in shared global Git config. */
|
||||
export async function prepareGitHubOperationLaunchers(input: {
|
||||
runId: string; target: AdapterExecutionTarget | null | undefined; cwd: string; env: Record<string, string>;
|
||||
|
|
@ -1528,8 +1560,8 @@ export async function prepareGitHubOperationLaunchers(input: {
|
|||
const remote = input.target?.kind === "remote" ? input.target : null;
|
||||
const directory = githubOperationLauncherDirectory(input);
|
||||
const configDirectory = path.posix.join(directory, "gh-config");
|
||||
const basePath = input.env.PATH || (remote ? "/usr/local/bin:/usr/bin:/bin" : process.env.PATH) || "/usr/bin:/bin";
|
||||
const managedPath = `${directory}:${basePath}`;
|
||||
const basePath = await githubOperationLauncherBasePath(remote, input.env);
|
||||
const managedPath = basePath ? `${directory}:${basePath}` : directory;
|
||||
// Login shells may reorder PATH through /etc/profile or path_helper. Restore
|
||||
// the managed launchers after startup without loading a host user's profile.
|
||||
const profile = `export PATH=${shellQuote(managedPath)}\n`;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,180 @@
|
|||
import { execFile } from "node:child_process";
|
||||
import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import * as ssh from "./ssh.js";
|
||||
import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js";
|
||||
import {
|
||||
ensureAdapterExecutionTargetCommandResolvable,
|
||||
prepareGitHubOperationLaunchers,
|
||||
runAdapterExecutionTargetProcess,
|
||||
} from "./execution-target.js";
|
||||
|
||||
const exec = promisify(execFile);
|
||||
const roots: string[] = [];
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllEnvs();
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })));
|
||||
});
|
||||
|
||||
async function sandbox(layout: string) {
|
||||
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-launcher-env-"));
|
||||
roots.push(root);
|
||||
const bin = path.join(root, layout);
|
||||
await mkdir(bin, { recursive: true });
|
||||
for (const cli of ["claude", "codex", "git", "gh"]) {
|
||||
await writeFile(path.join(bin, cli), `#!/bin/sh\nprintf '%s\\n' '${cli} started'\n`, { mode: 0o700 });
|
||||
}
|
||||
const remotePath = `${bin}:${path.dirname(process.execPath)}:/usr/local/bin:/usr/bin:/bin`;
|
||||
// Execute real shells and staged launchers, with a provider-owned environment.
|
||||
// Do not inherit the controller's PATH, HOME, credentials, or shell hooks.
|
||||
const execute: CommandManagedRuntimeRunner["execute"] = async (input) => {
|
||||
const startedAt = new Date().toISOString();
|
||||
try {
|
||||
const execution = exec(input.command, input.args ?? [], {
|
||||
cwd: input.cwd ?? root,
|
||||
env: { HOME: root, PATH: remotePath, ...input.env },
|
||||
timeout: input.timeoutMs ?? 15_000,
|
||||
});
|
||||
const inputComplete = new Promise<void>((resolve, reject) => {
|
||||
const stdin = execution.child.stdin;
|
||||
if (!stdin) return resolve();
|
||||
// Hash-skip staging can exit before reading the supplied file body.
|
||||
// Its exit result still determines success; other input errors fail.
|
||||
stdin.on("error", (error: NodeJS.ErrnoException) => {
|
||||
if (error.code === "EPIPE") resolve();
|
||||
else reject(error);
|
||||
});
|
||||
stdin.end(input.stdin ?? "", resolve);
|
||||
});
|
||||
const [result] = await Promise.all([execution, inputComplete]);
|
||||
return { ...result, exitCode: 0, signal: null, timedOut: false, pid: null, startedAt };
|
||||
} catch (error) {
|
||||
const result = error as Error & { code?: number; killed?: boolean; stdout?: string; stderr?: string };
|
||||
return { exitCode: result.code ?? 1, signal: null, timedOut: result.killed ?? false,
|
||||
stdout: result.stdout ?? "", stderr: result.stderr ?? "", pid: null, startedAt };
|
||||
}
|
||||
};
|
||||
const runner = { execute: vi.fn(execute) };
|
||||
const target = { kind: "remote" as const, transport: "sandbox" as const,
|
||||
providerKey: "fixture", remoteCwd: root, runner };
|
||||
return { root, bin, remotePath, runner, target };
|
||||
}
|
||||
|
||||
describe("managed GitHub launcher environment", () => {
|
||||
it.each(["nvm/current/bin", "usr/local/bin", "tools with 'quotes'/bin"])(
|
||||
"preserves %s CLIs and keeps GitHub wrappers first in child shells",
|
||||
async (layout) => {
|
||||
const fixture = await sandbox(layout);
|
||||
vi.stubEnv("PATH", "/controller-only/bin");
|
||||
const env = await prepareGitHubOperationLaunchers({
|
||||
runId: "run-layout", target: fixture.target, cwd: "/controller", env: {},
|
||||
});
|
||||
expect(env.PATH).toBe(`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}:${fixture.remotePath}`);
|
||||
for (const cli of ["claude", "codex"]) {
|
||||
await ensureAdapterExecutionTargetCommandResolvable(cli, fixture.target, fixture.root, env);
|
||||
const result = await runAdapterExecutionTargetProcess("run-layout", fixture.target, "bash", [
|
||||
"--noprofile", "--norc", "-c", `command -v git; command -v gh; ${cli}`,
|
||||
], { cwd: fixture.root, env, timeoutSec: 5, graceSec: 1, onLog: async () => {} });
|
||||
expect(result.exitCode, result.stderr).toBe(0);
|
||||
expect(result.stdout.trim().split("\n")).toEqual([
|
||||
`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}/git`,
|
||||
`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}/gh`,
|
||||
`${cli} started`,
|
||||
]);
|
||||
}
|
||||
for (const profile of [".profile", ".bash_profile", ".bashrc", ".zshenv", ".zprofile", ".zshrc"]) {
|
||||
const script = await readFile(path.join(env.PAPERCLIP_GITHUB_LAUNCHER_DIR, profile), "utf8");
|
||||
const result = await fixture.runner.execute({ command: "sh", args: ["-c", `${script}\nprintf '%s' "$PATH"`] });
|
||||
expect(result.stdout).toBe(env.PATH);
|
||||
}
|
||||
// The wrappers' Node interpreter and underlying commands are still reachable.
|
||||
const github = await fixture.runner.execute({ command: "bash", args: ["-c", "git; gh"], env });
|
||||
expect(github.exitCode, github.stderr).toBe(0);
|
||||
expect(github.stdout).toBe("git started\ngh started\n");
|
||||
},
|
||||
);
|
||||
|
||||
it("preserves an explicit remote PATH without querying the remote environment", async () => {
|
||||
const fixture = await sandbox("custom/bin");
|
||||
const env = await prepareGitHubOperationLaunchers({
|
||||
runId: "run-explicit", target: fixture.target, cwd: fixture.root, env: { PATH: fixture.remotePath },
|
||||
});
|
||||
expect(env.PATH).toBe(`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}:${fixture.remotePath}`);
|
||||
expect(fixture.runner.execute.mock.calls.every(([input]) => !input.args?.join(" ").includes("$PATH"))).toBe(true);
|
||||
});
|
||||
|
||||
it("does not copy an inherited controller PATH into a remote launcher", async () => {
|
||||
const fixture = await sandbox("nvm/bin");
|
||||
vi.stubEnv("PATH", "/controller-only/bin");
|
||||
const env = await prepareGitHubOperationLaunchers({
|
||||
runId: "run-inherited", target: fixture.target, cwd: fixture.root, env: { PATH: process.env.PATH! },
|
||||
});
|
||||
expect(env.PATH).toBe(`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}:${fixture.remotePath}`);
|
||||
});
|
||||
|
||||
it("keeps an explicit empty remote PATH empty apart from the managed wrappers", async () => {
|
||||
const fixture = await sandbox("nvm/bin");
|
||||
const env = await prepareGitHubOperationLaunchers({
|
||||
runId: "run-empty", target: fixture.target, cwd: fixture.root, env: { PATH: "" },
|
||||
});
|
||||
expect(env.PATH).toBe(env.PAPERCLIP_GITHUB_LAUNCHER_DIR);
|
||||
expect(fixture.runner.execute.mock.calls.every(([input]) => !input.args?.join(" ").includes("$PATH"))).toBe(true);
|
||||
const result = await fixture.runner.execute({ command: "/bin/sh", args: ["-c", "command -v claude"], env });
|
||||
expect(result.exitCode).not.toBe(0);
|
||||
});
|
||||
|
||||
it("reads the SSH target PATH and ignores login banners", async () => {
|
||||
const fixture = await sandbox("ssh-toolchain/bin");
|
||||
fixture.runner.execute.mockResolvedValueOnce({ exitCode: 0, timedOut: false, signal: null,
|
||||
stdout: `Welcome\n\0${fixture.remotePath}\0\n`, stderr: "", pid: null, startedAt: new Date().toISOString() });
|
||||
vi.spyOn(ssh, "createSshCommandManagedRuntimeRunner").mockReturnValue(fixture.runner);
|
||||
const target = { kind: "remote" as const, transport: "ssh" as const, remoteCwd: fixture.root,
|
||||
spec: { host: "sandbox.example.test", port: 22, username: "runner", remoteCwd: fixture.root,
|
||||
remoteWorkspacePath: fixture.root, privateKey: null, knownHosts: null, strictHostKeyChecking: true } };
|
||||
const env = await prepareGitHubOperationLaunchers({ runId: "run-ssh", target, cwd: fixture.root, env: {} });
|
||||
expect(env.PATH).toBe(`${env.PAPERCLIP_GITHUB_LAUNCHER_DIR}:${fixture.remotePath}`);
|
||||
expect(fixture.runner.execute.mock.calls[0]?.[0].env).toBeUndefined();
|
||||
});
|
||||
|
||||
it("uses the launch environment for install and re-probe after a missing command", async () => {
|
||||
const fixture = await sandbox("custom/bin");
|
||||
const env = { PATH: fixture.remotePath, HOME: fixture.root };
|
||||
await ensureAdapterExecutionTargetCommandResolvable("fixture-cli", fixture.target, fixture.root, env, {
|
||||
installCommand: `cp ${ssh.shellQuote(path.join(fixture.bin, "claude"))} ${ssh.shellQuote(path.join(fixture.bin, "fixture-cli"))}`,
|
||||
});
|
||||
expect(fixture.runner.execute.mock.calls).toHaveLength(3);
|
||||
for (const [input] of fixture.runner.execute.mock.calls) expect(input.env).toEqual(env);
|
||||
});
|
||||
|
||||
it("checks command availability with the launch environment, not the provider default", async () => {
|
||||
const fixture = await sandbox("nvm/bin");
|
||||
const env = { PATH: "/usr/bin:/bin" };
|
||||
// The binary exists on the provider PATH, but the requested launch excludes it.
|
||||
await expect(ensureAdapterExecutionTargetCommandResolvable(
|
||||
"claude", fixture.target, fixture.root, env,
|
||||
)).rejects.toThrow('Command "claude" is not installed or not on PATH');
|
||||
const result = await runAdapterExecutionTargetProcess("run-missing", fixture.target, "sh", ["-c", "claude"], {
|
||||
cwd: fixture.root, env, timeoutSec: 5, graceSec: 1, onLog: async () => {},
|
||||
});
|
||||
expect(result.exitCode).toBe(127);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ exitCode: 1, timedOut: false, stdout: "" },
|
||||
{ exitCode: 0, timedOut: true, stdout: "" },
|
||||
{ exitCode: 0, timedOut: false, stdout: "login banner only" },
|
||||
{ exitCode: 0, timedOut: false, stdout: "\0\0" },
|
||||
])("fails before staging when remote PATH discovery fails: %j", async (failure) => {
|
||||
const fixture = await sandbox("nvm/bin");
|
||||
fixture.runner.execute.mockResolvedValueOnce({ ...failure, signal: null, stderr: "private diagnostic",
|
||||
pid: null, startedAt: new Date().toISOString() });
|
||||
await expect(prepareGitHubOperationLaunchers({
|
||||
runId: "run-failure", target: fixture.target, cwd: fixture.root, env: {},
|
||||
})).rejects.toThrow("Could not resolve remote PATH for managed GitHub launchers");
|
||||
expect(fixture.runner.execute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue