refactor(sandbox): retire the dead noProfile flag from the exec path (#10461)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The sandbox exec path starts agent commands and passes runtime options to the server and plugin layers > - This path kept a noProfile flag after the exec wrappers stopped sourcing a login profile > - The flag no longer changed behavior, so it left dead API surface in the protocol and runtime helpers > - This pull request removes that dead flag from the plugin protocol, the server drivers, and the managed-runtime helpers > - It also updates the tests and points the agent runtime README at the sandbox requirements file > - The benefit is a smaller and clearer exec-path contract with no behavior change ## Linked Issues or Issue Description - No public GitHub issue exists. ### What happened? The sandbox exec path kept a `noProfile` field after the exec wrappers stopped sourcing a login profile. ### Expected behavior The plugin protocol, server drivers, and managed-runtime helpers should not expose or forward a dead field. ### Steps to reproduce 1. Run a managed-runtime command through the sandbox exec path. 2. Inspect the protocol payload and runtime helper inputs. 3. Observe that `noProfile` is present even though it no longer changes behavior. ### Paperclip version or commit `60c7da86fc7a6c1dbf37bbcd86e25ecaaff01607` ### Deployment mode Built from source (pnpm dev / pnpm build) ### Additional context This pull request removes the dead field, updates the affected tests, and updates the README note for the sandbox profile path. ## What Changed - Removed noProfile from the plugin protocol and the server exec-path call sites. - Updated the managed-runtime helpers to use the narrower exec-path contract. - Updated the affected tests and added the README pointer to SANDBOX-REQUIREMENTS.md. ## Verification - `git grep -n "noProfile" -- packages/ server/` returns zero matches. - `tsc --noEmit` passed for `@paperclipai/adapter-utils`, `@paperclipai/plugin-sdk`, and `@paperclipai/server`. - `command-managed-runtime.test.ts` passed: 22/22. - `environment-runtime.test.ts` passed: 24/24. ## Risks - Low risk. The flag was already a no-op. - A hidden external caller may still send the removed field. ## Model Used - OpenAI GPT-5, tool-enabled. ## 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:
Nicky Leach
GitHub
parent
7a5a217d60
commit
7083c275c8
|
|
@ -21,6 +21,14 @@ Container images for running coding-agent harnesses in sandboxed environments (f
|
|||
- tini (PID-1 init, ensures signal propagation)
|
||||
- Non-root user `paperclip` (uid/gid 1000)
|
||||
|
||||
The NodeSource install puts `node` on the default `PATH`. The agent shim in this
|
||||
image runs the harness directly with that `PATH`. The shim does not source a
|
||||
login profile, and the runtime never writes a profile or an rc file. Some
|
||||
sandbox providers instead wrap each command in a login shell. That shell sources
|
||||
`/etc/profile` and the user profile files to read an owner-supplied `PATH`. No
|
||||
exec path sources `nvm`. For the full exec-path contract, see
|
||||
`packages/plugins/sandbox-providers/SANDBOX-REQUIREMENTS.md`.
|
||||
|
||||
**Paperclip Binaries:**
|
||||
- `/usr/local/bin/paperclip-agent-shim`: Go binary compiled from `tools/agent-shim/`. Reads `/run/paperclip/runtime-command.json` and `syscall.Exec`s the harness CLI.
|
||||
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ const execFile = promisify(execFileCallback);
|
|||
|
||||
interface SpawnRunnerHandle {
|
||||
runner: CommandManagedRuntimeRunner;
|
||||
calls: Array<{ command: string; args?: string[]; cwd?: string; stdin?: string; noProfile?: boolean }>;
|
||||
calls: Array<{ command: string; args?: string[]; cwd?: string; stdin?: string }>;
|
||||
}
|
||||
|
||||
// A runner that actually executes the shell scripts (piping stdin through a real
|
||||
|
|
@ -27,7 +27,7 @@ function makeSpawnRunner(options: {
|
|||
supportsSingleStreamStdinProgress?: boolean;
|
||||
maxStdoutBytes?: number;
|
||||
} = {}): SpawnRunnerHandle {
|
||||
const calls: Array<{ command: string; args?: string[]; cwd?: string; stdin?: string; noProfile?: boolean }> = [];
|
||||
const calls: Array<{ command: string; args?: string[]; cwd?: string; stdin?: string }> = [];
|
||||
const runner: CommandManagedRuntimeRunner = {
|
||||
supportsSingleStreamStdinProgress: options.supportsSingleStreamStdinProgress,
|
||||
execute: async (input) =>
|
||||
|
|
@ -37,7 +37,6 @@ function makeSpawnRunner(options: {
|
|||
args: input.args,
|
||||
cwd: input.cwd,
|
||||
stdin: input.stdin,
|
||||
noProfile: input.noProfile,
|
||||
});
|
||||
const startedAt = new Date().toISOString();
|
||||
const command =
|
||||
|
|
@ -153,7 +152,6 @@ describe("command managed runtime", () => {
|
|||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
noProfile?: boolean;
|
||||
}> = [];
|
||||
const runner = {
|
||||
execute: async (input: {
|
||||
|
|
@ -163,7 +161,6 @@ describe("command managed runtime", () => {
|
|||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
noProfile?: boolean;
|
||||
}): Promise<RunProcessResult> => {
|
||||
calls.push({ ...input });
|
||||
const startedAt = new Date().toISOString();
|
||||
|
|
@ -236,7 +233,6 @@ describe("command managed runtime", () => {
|
|||
// The single-stream upload pipes the tarball through exactly one stdin-backed
|
||||
// process (the speed fix); nothing else streams stdin.
|
||||
expect(calls.filter((call) => call.stdin != null).length).toBe(1);
|
||||
expect(calls.some((call) => call.noProfile === true)).toBe(true);
|
||||
|
||||
await mkdir(path.join(remoteWorkspaceDir, ".paperclip-runtime"), { recursive: true });
|
||||
await writeFile(path.join(remoteWorkspaceDir, "README.md"), "remote workspace\n", "utf8");
|
||||
|
|
@ -251,7 +247,6 @@ describe("command managed runtime", () => {
|
|||
// Restore streams the download through `base64`/onLog (no stdin), so the only
|
||||
// stdin-backed call remains the single upload from prepare.
|
||||
expect(calls.filter((call) => call.stdin != null).length).toBe(1);
|
||||
expect(calls.some((call) => call.noProfile === true)).toBe(true);
|
||||
});
|
||||
|
||||
it("stages runtime assets without replacing or restoring an in-place workspace", async () => {
|
||||
|
|
@ -330,10 +325,8 @@ describe("command managed runtime", () => {
|
|||
detectCommand: "sh",
|
||||
});
|
||||
|
||||
// The detection probe must be the first shell invocation and stay on the
|
||||
// default profile-sourcing path (noProfile !== true) so a CLI provided by
|
||||
// the login profile is discoverable before we decide whether to install.
|
||||
expect(calls[0]?.noProfile).not.toBe(true);
|
||||
// The detection probe must be the first shell invocation, so a CLI on the
|
||||
// sandbox default PATH is discoverable before we decide whether to install.
|
||||
expect(calls[0]?.args?.join(" ")).toContain("command -v 'sh'");
|
||||
// Detection succeeds here, so the install command must be skipped entirely;
|
||||
// the remaining calls are workspace staging, never the install command.
|
||||
|
|
@ -588,15 +581,6 @@ describe("command managed runtime", () => {
|
|||
expect(untarIdx).toBeGreaterThan(uploadIdx);
|
||||
expect(cmd1Idx).toBeGreaterThan(untarIdx);
|
||||
expect(cmd2Idx).toBeGreaterThan(cmd1Idx);
|
||||
|
||||
// Fast path: the fixed internal transport helpers (tar upload + untar) ride
|
||||
// the no-profile shell — they are trusted, fixed commands that never need a
|
||||
// login-shell profile. The opaque post-upload commands stay profile-backed
|
||||
// (noProfile !== true) so any env a caller-supplied command relies on is present.
|
||||
expect(calls[uploadIdx]?.noProfile).toBe(true);
|
||||
expect(calls[untarIdx]?.noProfile).toBe(true);
|
||||
expect(calls[cmd1Idx]?.noProfile).not.toBe(true);
|
||||
expect(calls[cmd2Idx]?.noProfile).not.toBe(true);
|
||||
});
|
||||
|
||||
it("fallback syncIn runs a post-upload command under its own timeout, not the sync-client default", async () => {
|
||||
|
|
@ -663,11 +647,6 @@ describe("command managed runtime", () => {
|
|||
expect(scripts[3]).toContain(targetFile);
|
||||
expect(scripts[4]).toContain("rm -rf");
|
||||
expect(scripts[4]).toContain(targetFile + ".paperclip-syncin.");
|
||||
|
||||
// Fast path: the staged-write helpers (chmod + rename) are fixed internal
|
||||
// commands, so they ride the no-profile shell alongside the upload/staging.
|
||||
expect(calls[2]?.noProfile).toBe(true); // chmod
|
||||
expect(calls[3]?.noProfile).toBe(true); // mv (rename into place)
|
||||
});
|
||||
|
||||
it("fallback syncIn cleans up a staged file when chmod fails before rename", async () => {
|
||||
|
|
|
|||
|
|
@ -50,7 +50,6 @@ export interface CommandManagedRuntimeRunner {
|
|||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
noProfile?: boolean;
|
||||
onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise<void>;
|
||||
onSpawn?: (meta: { pid: number; startedAt: string }) => Promise<void>;
|
||||
}): Promise<RunProcessResult>;
|
||||
|
|
@ -206,7 +205,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
opts: {
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
noProfile?: boolean;
|
||||
onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise<void>;
|
||||
} = {},
|
||||
) => {
|
||||
|
|
@ -216,7 +214,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
cwd: input.commandCwd,
|
||||
stdin: opts.stdin,
|
||||
timeoutMs: opts.timeoutMs ?? input.timeoutMs,
|
||||
noProfile: opts.noProfile === true,
|
||||
onLog: opts.onLog,
|
||||
});
|
||||
requireSuccessfulResult(result, script);
|
||||
|
|
@ -225,7 +222,7 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
|
||||
const client: SandboxManagedRuntimeClient = {
|
||||
makeDir: async (remotePath) => {
|
||||
await runShell(`mkdir -p ${shellQuote(remotePath)}`, { noProfile: true });
|
||||
await runShell(`mkdir -p ${shellQuote(remotePath)}`);
|
||||
},
|
||||
writeFile: async (remotePath, bytes, options) => {
|
||||
const buffer = toBuffer(bytes);
|
||||
|
|
@ -252,7 +249,7 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
`mkdir -p ${shellQuote(remoteDir)} && ` +
|
||||
`base64 -d > ${shellQuote(remoteTempPath)} && ` +
|
||||
`mv -f ${shellQuote(remoteTempPath)} ${shellQuote(remotePath)}`,
|
||||
{ stdin: body, noProfile: true },
|
||||
{ stdin: body },
|
||||
);
|
||||
await options?.onProgress?.(total, total);
|
||||
return;
|
||||
|
|
@ -266,15 +263,14 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
await runShell(
|
||||
`mkdir -p ${shellQuote(remoteDir)} && ` +
|
||||
`rm -f ${shellQuote(remoteTempPath)} && : > ${shellQuote(remoteTempPath)}`,
|
||||
{ noProfile: true },
|
||||
);
|
||||
for (let offset = 0; offset < total; offset += REMOTE_WRITE_FALLBACK_DECODED_CHUNK_SIZE) {
|
||||
const end = Math.min(total, offset + REMOTE_WRITE_FALLBACK_DECODED_CHUNK_SIZE);
|
||||
const chunk = buffer.subarray(offset, end).toString("base64");
|
||||
await runShell(`base64 -d >> ${shellQuote(remoteTempPath)}`, { stdin: chunk, noProfile: true });
|
||||
await runShell(`base64 -d >> ${shellQuote(remoteTempPath)}`, { stdin: chunk });
|
||||
await options?.onProgress?.(end, total);
|
||||
}
|
||||
await runShell(`mv -f ${shellQuote(remoteTempPath)} ${shellQuote(remotePath)}`, { noProfile: true });
|
||||
await runShell(`mv -f ${shellQuote(remoteTempPath)} ${shellQuote(remotePath)}`);
|
||||
await options?.onProgress?.(total, total);
|
||||
} finally {
|
||||
await bestEffortRemoveRemotePath(client, remoteTempPath);
|
||||
|
|
@ -284,7 +280,7 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
// Chunked reads intentionally query the remote size first, even without
|
||||
// a progress sink, so each sandbox RPC stays bounded and truncation is
|
||||
// detected without materializing the whole file as one stdout string.
|
||||
const sizeResult = await runShell(`wc -c < ${shellQuote(remotePath)}`, { noProfile: true });
|
||||
const sizeResult = await runShell(`wc -c < ${shellQuote(remotePath)}`);
|
||||
const totalBytes = Number.parseInt(sizeResult.stdout.trim(), 10);
|
||||
if (!Number.isFinite(totalBytes) || totalBytes < 0) {
|
||||
throw new Error(`Could not determine remote file size for ${remotePath}`);
|
||||
|
|
@ -303,7 +299,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
for (let chunkIndex = 0; decodedSoFar < totalBytes; chunkIndex++) {
|
||||
const result = await runShell(
|
||||
`dd if=${shellQuote(remotePath)} bs=${REMOTE_READ_CHUNK_BYTES} skip=${chunkIndex} count=1 2>/dev/null | base64`,
|
||||
{ noProfile: true },
|
||||
);
|
||||
const chunk = Buffer.from(result.stdout.replace(/\s+/g, ""), "base64");
|
||||
if (chunk.byteLength === 0) break;
|
||||
|
|
@ -326,7 +321,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
`basename "$entry"; ` +
|
||||
`done; ` +
|
||||
`fi`,
|
||||
{ noProfile: true },
|
||||
);
|
||||
return result.stdout
|
||||
.split(/\r?\n/)
|
||||
|
|
@ -340,7 +334,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
args: shellCommandArgs(`rm -rf ${shellQuote(remotePath)}`),
|
||||
cwd: input.commandCwd,
|
||||
timeoutMs: input.timeoutMs,
|
||||
noProfile: true,
|
||||
});
|
||||
requireSuccessfulResult(result, `remove ${remotePath}`);
|
||||
},
|
||||
|
|
@ -350,7 +343,6 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
args: shellCommandArgs(command),
|
||||
cwd: input.commandCwd,
|
||||
timeoutMs: options.timeoutMs,
|
||||
noProfile: options.noProfile === true,
|
||||
});
|
||||
requireSuccessfulResult(result, command);
|
||||
},
|
||||
|
|
@ -390,7 +382,7 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
await client.writeFile(remoteTarPath, bufferToArrayBuffer(tarBytes));
|
||||
await client.run(
|
||||
buildSyncInExtractDirectoryCommand({ remoteTarPath, targetDir: mapping.targetPath }),
|
||||
{ timeoutMs: input.timeoutMs, noProfile: true },
|
||||
{ timeoutMs: input.timeoutMs },
|
||||
);
|
||||
bytesTransferred += tarBytes.byteLength;
|
||||
} else {
|
||||
|
|
@ -403,11 +395,11 @@ export function createCommandManagedRuntimeClient(input: {
|
|||
if (mapping.mode != null) {
|
||||
await client.run(
|
||||
buildSyncInChmodCommand({ mode: mapping.mode, targetPath: targetPathForWrite }),
|
||||
{ timeoutMs: input.timeoutMs, noProfile: true },
|
||||
{ timeoutMs: input.timeoutMs },
|
||||
);
|
||||
await client.run(
|
||||
buildSyncInRenameCommand({ sourcePath: targetPathForWrite, targetPath: mapping.targetPath }),
|
||||
{ timeoutMs: input.timeoutMs, noProfile: true },
|
||||
{ timeoutMs: input.timeoutMs },
|
||||
);
|
||||
}
|
||||
bytesTransferred += fileBytes.byteLength;
|
||||
|
|
|
|||
|
|
@ -207,7 +207,7 @@ export interface SandboxManagedRuntimeClient {
|
|||
): Promise<Buffer | Uint8Array | ArrayBuffer>;
|
||||
listFiles(remotePath: string): Promise<string[]>;
|
||||
remove(remotePath: string): Promise<void>;
|
||||
run(command: string, options: { timeoutMs: number; noProfile?: boolean }): Promise<void>;
|
||||
run(command: string, options: { timeoutMs: number }): Promise<void>;
|
||||
/**
|
||||
* Optional native inbound transfer. Present only when the sandbox provider
|
||||
* advertises both `environmentSyncIn` and `environmentSyncOut`; otherwise the
|
||||
|
|
|
|||
|
|
@ -651,8 +651,6 @@ export interface PluginEnvironmentExecuteParams extends PluginEnvironmentDriverB
|
|||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
/** Skip login-shell profile sourcing when the command already resolves on the sandbox default PATH. */
|
||||
noProfile?: boolean;
|
||||
}
|
||||
|
||||
export interface PluginEnvironmentExecuteResult {
|
||||
|
|
|
|||
|
|
@ -2360,7 +2360,6 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
|
|||
env: { FOO: "bar" },
|
||||
stdin: "",
|
||||
timeoutMs: 1000,
|
||||
noProfile: true,
|
||||
});
|
||||
const destroyed = await runtimeWithPlugin.destroyRunLease({
|
||||
environment,
|
||||
|
|
@ -2415,7 +2414,6 @@ describeEmbeddedPostgres("environmentRuntimeService", () => {
|
|||
args: ["ok"],
|
||||
cwd: "/workspace/project",
|
||||
env: { FOO: "bar" },
|
||||
noProfile: true,
|
||||
}), 31000);
|
||||
expect(workerManager.call).toHaveBeenCalledWith(pluginId, "environmentDestroyLease", {
|
||||
driverKey: "fake-plugin",
|
||||
|
|
|
|||
|
|
@ -115,7 +115,6 @@ export async function resolveEnvironmentExecutionTarget(input: {
|
|||
env: commandInput.env,
|
||||
stdin: commandInput.stdin,
|
||||
timeoutMs: commandInput.timeoutMs,
|
||||
noProfile: commandInput.noProfile,
|
||||
});
|
||||
accumulateProviderDurations(result.metadata);
|
||||
if (result.stdout) await commandInput.onLog?.("stdout", result.stdout);
|
||||
|
|
|
|||
|
|
@ -188,11 +188,6 @@ export interface EnvironmentDriverExecuteInput extends EnvironmentDriverLeaseInp
|
|||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
/**
|
||||
* Skip login-shell profile sourcing for commands that already
|
||||
* resolve on the sandbox default PATH.
|
||||
*/
|
||||
noProfile?: boolean;
|
||||
}
|
||||
|
||||
export interface EnvironmentDriverSyncInput extends EnvironmentDriverLeaseInput {
|
||||
|
|
@ -1283,7 +1278,6 @@ function createSandboxEnvironmentDriver(
|
|||
env: input.env,
|
||||
stdin: input.stdin,
|
||||
timeoutMs: input.timeoutMs,
|
||||
noProfile: input.noProfile === true,
|
||||
}, resolvePluginExecuteRpcTimeoutMs({
|
||||
requestedTimeoutMs: input.timeoutMs,
|
||||
config: sanitizedConfig,
|
||||
|
|
@ -1744,7 +1738,6 @@ function createPluginEnvironmentDriver(
|
|||
env: input.env,
|
||||
stdin: input.stdin,
|
||||
timeoutMs: input.timeoutMs,
|
||||
noProfile: input.noProfile === true,
|
||||
},
|
||||
});
|
||||
},
|
||||
|
|
|
|||
Loading…
Reference in New Issue