From 7083c275c8fde73516066563ec2314529d70bc1a Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Wed, 29 Jul 2026 14:21:57 -0700 Subject: [PATCH] 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 --- docker/agent-runtime/README.md | 8 +++++ .../src/command-managed-runtime.test.ts | 29 +++---------------- .../src/command-managed-runtime.ts | 24 +++++---------- .../src/sandbox-managed-runtime.ts | 2 +- packages/plugins/sdk/src/protocol.ts | 2 -- .../src/__tests__/environment-runtime.test.ts | 2 -- .../services/environment-execution-target.ts | 1 - server/src/services/environment-runtime.ts | 7 ----- 8 files changed, 21 insertions(+), 54 deletions(-) diff --git a/docker/agent-runtime/README.md b/docker/agent-runtime/README.md index b2d35a26b2..9783495173 100644 --- a/docker/agent-runtime/README.md +++ b/docker/agent-runtime/README.md @@ -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. diff --git a/packages/adapter-utils/src/command-managed-runtime.test.ts b/packages/adapter-utils/src/command-managed-runtime.test.ts index 3e494212c3..a4b36fb49e 100644 --- a/packages/adapter-utils/src/command-managed-runtime.test.ts +++ b/packages/adapter-utils/src/command-managed-runtime.test.ts @@ -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; stdin?: string; timeoutMs?: number; - noProfile?: boolean; }> = []; const runner = { execute: async (input: { @@ -163,7 +161,6 @@ describe("command managed runtime", () => { env?: Record; stdin?: string; timeoutMs?: number; - noProfile?: boolean; }): Promise => { 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 () => { diff --git a/packages/adapter-utils/src/command-managed-runtime.ts b/packages/adapter-utils/src/command-managed-runtime.ts index 3d5a2f86f1..58a5d08510 100644 --- a/packages/adapter-utils/src/command-managed-runtime.ts +++ b/packages/adapter-utils/src/command-managed-runtime.ts @@ -50,7 +50,6 @@ export interface CommandManagedRuntimeRunner { env?: Record; stdin?: string; timeoutMs?: number; - noProfile?: boolean; onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise; onSpawn?: (meta: { pid: number; startedAt: string }) => Promise; }): Promise; @@ -206,7 +205,6 @@ export function createCommandManagedRuntimeClient(input: { opts: { stdin?: string; timeoutMs?: number; - noProfile?: boolean; onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise; } = {}, ) => { @@ -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; diff --git a/packages/adapter-utils/src/sandbox-managed-runtime.ts b/packages/adapter-utils/src/sandbox-managed-runtime.ts index 1eef71f91c..50e7d92d3c 100644 --- a/packages/adapter-utils/src/sandbox-managed-runtime.ts +++ b/packages/adapter-utils/src/sandbox-managed-runtime.ts @@ -207,7 +207,7 @@ export interface SandboxManagedRuntimeClient { ): Promise; listFiles(remotePath: string): Promise; remove(remotePath: string): Promise; - run(command: string, options: { timeoutMs: number; noProfile?: boolean }): Promise; + run(command: string, options: { timeoutMs: number }): Promise; /** * Optional native inbound transfer. Present only when the sandbox provider * advertises both `environmentSyncIn` and `environmentSyncOut`; otherwise the diff --git a/packages/plugins/sdk/src/protocol.ts b/packages/plugins/sdk/src/protocol.ts index 7335a58b62..d9b78d416b 100644 --- a/packages/plugins/sdk/src/protocol.ts +++ b/packages/plugins/sdk/src/protocol.ts @@ -651,8 +651,6 @@ export interface PluginEnvironmentExecuteParams extends PluginEnvironmentDriverB env?: Record; stdin?: string; timeoutMs?: number; - /** Skip login-shell profile sourcing when the command already resolves on the sandbox default PATH. */ - noProfile?: boolean; } export interface PluginEnvironmentExecuteResult { diff --git a/server/src/__tests__/environment-runtime.test.ts b/server/src/__tests__/environment-runtime.test.ts index c90e5e3cc9..cefe00fc8b 100644 --- a/server/src/__tests__/environment-runtime.test.ts +++ b/server/src/__tests__/environment-runtime.test.ts @@ -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", diff --git a/server/src/services/environment-execution-target.ts b/server/src/services/environment-execution-target.ts index f0ca1b68ca..ac5e848f31 100644 --- a/server/src/services/environment-execution-target.ts +++ b/server/src/services/environment-execution-target.ts @@ -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); diff --git a/server/src/services/environment-runtime.ts b/server/src/services/environment-runtime.ts index d4bea6611b..7e7ab0cd67 100644 --- a/server/src/services/environment-runtime.ts +++ b/server/src/services/environment-runtime.ts @@ -188,11 +188,6 @@ export interface EnvironmentDriverExecuteInput extends EnvironmentDriverLeaseInp env?: Record; 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, }, }); },