diff --git a/doc/acp-run-lifecycle.md b/doc/acp-run-lifecycle.md index e4d0c566d0d..0e75f3e7d7f 100644 --- a/doc/acp-run-lifecycle.md +++ b/doc/acp-run-lifecycle.md @@ -49,6 +49,15 @@ exists, so the settlement always closes it, even on an early failure. ## The settlement order +The sandbox agent bridge passes small command envelopes through its launch +environment. When the encoded envelope exceeds 64 KiB, it uploads the envelope +in bounded chunks to a private session directory instead. This avoids the Linux +limit on one argument or environment string when retry context grows. The +wrapper reads and deletes the file before spawning the agent; bridge teardown +removes it if startup fails. The child's environment values remain unchanged. +If the launch shell fails before the wrapper emits a protocol event, the run +log retains the shell's stderr alongside the exit code. + The settlement sequence is the one live cleanup owner for every settled path. It claims the ledger once, makes the pure reuse decision, then runs the ordered steps: diff --git a/packages/adapter-utils/src/execution-target-sandbox.test.ts b/packages/adapter-utils/src/execution-target-sandbox.test.ts index 779596ca6c3..36695e85c1b 100644 --- a/packages/adapter-utils/src/execution-target-sandbox.test.ts +++ b/packages/adapter-utils/src/execution-target-sandbox.test.ts @@ -3,7 +3,7 @@ import http2 from "node:http2"; import net from "node:net"; import { duplexPair, type Duplex } from "node:stream"; import { execFile, spawn } from "node:child_process"; -import { mkdir, mkdtemp, readdir, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { promisify } from "node:util"; @@ -525,6 +525,142 @@ describe("sandbox adapter execution targets", () => { }, ); + it.each([false, true])("launches a large environment without oversized exec arguments (streamed=%s)", async (streamOutputViaSession) => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-large-process-env-")); + cleanupDirs.push(rootDir); + const value = "x".repeat(110_000); + const childPath = path.join(rootDir, "child.mjs"); + await writeFile(childPath, 'process.stdout.write(process.env.LARGE_CONTEXT ?? "missing");\n'); + const delegate = createLocalSandboxRunner(); + let checkedPrivatePayload = false; + const runner = { + execute: async (input: Parameters[0]) => { + // Linux limits each argument and environment string to 128 KiB on + // systems with 4 KiB pages. Enforce that boundary on macOS too. + const strings = [...(input.args ?? []), ...Object.entries(input.env ?? {}).map(([k, v]) => `${k}=${v}`)]; + if (strings.some((text) => Buffer.byteLength(text) >= 131_072)) { + return { exitCode: 127, stdout: "", stderr: "argument list too long: env\n", timedOut: false, signal: null, pid: null, startedAt: null }; + } + if (input.env?.PAPERCLIP_PROCESS_SESSION_DIR || input.args?.[1]?.includes("nohup node")) { + const sessionRoot = path.join(runtimeRootDir, "process-sessions"); + const entries = await readdir(sessionRoot, { withFileTypes: true }); + const sessionDir = path.join(sessionRoot, entries.find((entry) => entry.isDirectory())!.name); + expect((await stat(sessionDir)).mode & 0o777).toBe(0o700); + expect((await stat(path.join(sessionDir, "command.b64"))).mode & 0o777).toBe(0o600); + checkedPrivatePayload = true; + } + return delegate.execute(input); + }, + }; + const runtimeRootDir = path.join(rootDir, ".paperclip-runtime"); + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "large-process-env", adapterKey: "acpx", runtimeRootDir, + target: { kind: "remote", transport: "sandbox", providerKey: "local-test", remoteCwd: rootDir, runner }, + command: process.execPath, args: [childPath], cwd: rootDir, + env: { LARGE_CONTEXT: value }, timeoutSec: 10, streamOutputViaSession, + }); + try { + const result = await runProxyWithInput(bridge!.agentCommand, "", true); + expect(result.code).toBe(0); + expect(result.stdout).toBe(value); + expect(checkedPrivatePayload).toBe(true); + const sessionRoot = path.join(runtimeRootDir, "process-sessions"); + const sessionDirs = (await readdir(sessionRoot, { withFileTypes: true })).filter((entry) => entry.isDirectory()); + for (const entry of sessionDirs) { + await expect(readFile(path.join(sessionRoot, entry.name, "command.b64"))).rejects.toThrow(); + } + } finally { + await bridge?.stop(); + } + }); + + it("logs a streamed wrapper launch failure before forwarding its exit", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-wrapper-launch-error-")); + cleanupDirs.push(rootDir); + const delegate = createLocalSandboxRunner(); + const logs: string[] = []; + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "wrapper-launch-error", adapterKey: "acpx", runtimeRootDir: rootDir, + target: { + kind: "remote", transport: "sandbox", providerKey: "local-test", remoteCwd: rootDir, + runner: { execute: async (input) => input.useSession + ? { exitCode: 127, stdout: "", stderr: "argument list too long: env\n", timedOut: false, signal: null, pid: null, startedAt: null } + : delegate.execute(input) }, + }, + command: process.execPath, args: [], cwd: rootDir, env: {}, timeoutSec: 5, + streamOutputViaSession: true, + onLog: async (stream, chunk) => { if (stream === "stderr") logs.push(chunk); }, + }); + try { + const result = await runProxyWithInput(bridge!.agentCommand, "", true); + expect(result.code).toBe(127); + expect(logs).toContain("argument list too long: env\n"); + } finally { + await bridge?.stop(); + } + }, 10_000); + + it("removes an incomplete private command payload when upload fails", async () => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-payload-upload-error-")); + cleanupDirs.push(rootDir); + const delegate = createLocalSandboxRunner(); + await expect(startAdapterExecutionTargetProcessSessionBridge({ + runId: "payload-upload-error", adapterKey: "acpx", runtimeRootDir: rootDir, + target: { + kind: "remote", transport: "sandbox", providerKey: "local-test", remoteCwd: rootDir, + runner: { execute: async (input) => { + if (input.args?.[1]?.includes("command.b64.paperclip-upload.b64") && input.args[1].includes(">>")) { + throw new Error("Upload interrupted"); + } + return delegate.execute(input); + } }, + }, + command: process.execPath, args: [], cwd: rootDir, + env: { LARGE_CONTEXT: "x".repeat(110_000) }, timeoutSec: 5, + streamOutputViaSession: true, + })).rejects.toThrow("Upload interrupted"); + const entries = await readdir(path.join(rootDir, "process-sessions"), { withFileTypes: true }); + expect(entries.filter((entry) => entry.isDirectory())).toEqual([]); + }); + + it.each([ + { streamed: false, corrupt: false }, + { streamed: false, corrupt: true }, + { streamed: true, corrupt: false }, + { streamed: true, corrupt: true }, + ])("reports payload read failures without hanging (streamed=$streamed, corrupt=$corrupt)", async ({ streamed, corrupt }) => { + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-payload-read-error-")); + cleanupDirs.push(rootDir); + const delegate = createLocalSandboxRunner(); + const bridge = await startAdapterExecutionTargetProcessSessionBridge({ + runId: "payload-read-error", adapterKey: "acpx", runtimeRootDir: rootDir, + target: { + kind: "remote", transport: "sandbox", providerKey: "local-test", remoteCwd: rootDir, + runner: { execute: async (input) => { + if (input.env?.PAPERCLIP_PROCESS_SESSION_DIR || input.args?.[1]?.includes("nohup node")) { + const sessionRoot = path.join(rootDir, "process-sessions"); + const entries = await readdir(sessionRoot, { withFileTypes: true }); + const payloadPath = path.join(sessionRoot, entries.find((entry) => entry.isDirectory())!.name, "command.b64"); + if (corrupt) await writeFile(payloadPath, Buffer.from("private-payload-text").toString("base64")); + else await rm(payloadPath); + } + return delegate.execute(input); + } }, + }, + command: process.execPath, args: [], cwd: rootDir, + env: { LARGE_CONTEXT: "x".repeat(110_000) }, timeoutSec: 10, + streamOutputViaSession: streamed, + }); + try { + const result = await runProxyWithInput(bridge!.agentCommand, "", true); + expect(result.code).toBe(1); + expect(result.stderr).toContain("Failed to read sandbox process session command payload."); + expect(result.stderr).not.toContain("private-payload-text"); + } finally { + await bridge?.stop(); + } + }); + it("test_process_session_poll_exec_parents_to_run_context", async () => { // The poll timer runs run-time execs for the whole run. Its `sandbox.exec` // span must parent to the live run span, not to the ended startup step. The diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 14effb96412..f67d01aad42 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -2036,6 +2036,38 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // is not reclassified as ambient merely because it equals the host value. env: sanitizeRemoteExecutionEnv(launchEnv, {}), }), "utf8").toString("base64"); + // Base64 plus JSON escaping can push an otherwise valid child environment + // past Linux's per-argument/per-variable exec limit. Keep small launches on + // the existing path; upload larger envelopes in bounded chunks instead. + const commandEnv: Record = {}; + if (commandPayload.length <= 64 * 1024) { + commandEnv.PAPERCLIP_PROCESS_SESSION_COMMAND_B64 = commandPayload; + } else { + const payloadPath = path.posix.join(sessionDir, "command.b64"); + const runPayloadSetup = async (script: string) => { + const result = await runner.execute({ + command: shellCommand, + args: shellCommandArgs(script), + cwd: target.remoteCwd, + timeoutMs, + bypassSession: true, + }); + if (result.timedOut || result.exitCode !== 0) { + throw new Error("Failed to stage sandbox process session command payload."); + } + }; + try { + // The envelope can contain credentials. Its upload intermediates stay + // inside a private session directory, and the wrapper deletes it before + // spawning the child. Teardown removes it if the wrapper cannot start. + await runPayloadSetup(`umask 077 && mkdir -m 700 ${shellQuote(sessionDir)}`); + await client.writeTextFile(payloadPath, commandPayload); + await runPayloadSetup(`chmod 600 ${shellQuote(payloadPath)}`); + } catch (error) { + await client.remove(sessionDir).catch(() => undefined); + throw error; + } + } // Legacy poll path: background the wrapper with `nohup` and read its output // event files with the host poll below. The streamed path launches the wrapper @@ -2050,7 +2082,7 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // I3: no numeric process identifier anywhere. Background the // wrapper and let it go; do not capture `$!`. `PAPERCLIP_PROCESS_SESSION_DIR=${shellQuote(sessionDir)} ` + - `PAPERCLIP_PROCESS_SESSION_COMMAND_B64=${shellQuote(commandPayload)} ` + + Object.entries(commandEnv).map(([key, value]) => `${key}=${shellQuote(value)} `).join("") + `nohup node ${shellQuote(remoteScriptPath)} >/dev/null 2>&1 < /dev/null &`, ].join("\n"), ), @@ -2062,8 +2094,12 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { // The wrapper launch is bridge plumbing. Keep it off the persistent // session so it never queues behind an in-run session command. bypassSession: true, + }).catch(async (error) => { + await client.remove(sessionDir).catch(() => undefined); + throw error; }); if (startResult.timedOut || (startResult.exitCode ?? 1) !== 0) { + await client.remove(sessionDir).catch(() => undefined); throw new Error(`Failed to start sandbox ACP process session bridge: ${startResult.stderr || startResult.stdout}`); } } @@ -2329,16 +2365,6 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { for (const line of text.split(/\n/)) parseFrameLine(line); }; - const launchEnvForStream = - typeof input.env === "function" ? await input.env() : input.env; - const streamCommandPayload = Buffer.from(JSON.stringify({ - command: input.command, - args: input.args, - cwd: input.cwd || target.remoteCwd, - // Same provenance-clean contract as the polled payload above. Preserve - // every explicit identity override even when it equals the host value. - env: sanitizeRemoteExecutionEnv(launchEnvForStream, {}), - }), "utf8").toString("base64"); await onLog( "stdout", `[paperclip] Starting streamed ACP process session bridge in sandbox (${target.providerKey ?? "provider"}).\n`, @@ -2377,7 +2403,7 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { cwd: target.remoteCwd, env: { PAPERCLIP_PROCESS_SESSION_DIR: sessionDir, - PAPERCLIP_PROCESS_SESSION_COMMAND_B64: streamCommandPayload, + ...commandEnv, PAPERCLIP_SANDBOX_EXEC_CHANNEL: "bridge", }, timeoutMs, @@ -2388,6 +2414,9 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: { }); ingestFinalText(result.stdout); if (!sawTerminal && !stopping) { + // A shell can fail before the wrapper emits any frames. Preserve + // that launch diagnostic instead of reporting only its exit code. + if (result.stderr) await onLog("stderr", result.stderr); deliverRemoteEvent({ type: "exit", code: typeof result.exitCode === "number" ? result.exitCode : null, @@ -3027,6 +3056,34 @@ await captureSessionIdentity(); void pollStdin().catch((error) => void writeEvent({ type: "error", message: error instanceof Error ? error.message : String(error) })); `; +// Shared by both wrappers. Refuse a symlink and remove the private envelope +// before creating the child; no launch payload file belongs to the agent. +const PROCESS_SESSION_READ_COMMAND = ` +let config; +try { + let commandPayload = process.env.PAPERCLIP_PROCESS_SESSION_COMMAND_B64; + if (!commandPayload) { + const payloadPath = path.posix.join(sessionDir, "command.b64"); + const handle = await fs.open(payloadPath, fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW); + try { + commandPayload = await handle.readFile("utf8"); + } finally { + await handle.close(); + await fs.rm(payloadPath, { force: true }); + } + } + config = JSON.parse(Buffer.from(commandPayload, "base64").toString("utf8")); +} catch { + // No child exists yet. Emit a terminal event on both transports and attest + // shutdown, instead of letting a detached polled wrapper fail silently. + // Parse errors can quote credential bytes, so use a fixed diagnostic. + await writeEvent({ type: "error", message: "Failed to read sandbox process session command payload." }); + await writeEvent({ type: "shutdownAck" }); + await new Promise((resolve) => process.stdout.write("", resolve)); + process.exit(1); +} +`; + // Streamed variant: the wrapper writes each output frame as one newline- // delimited JSON line to its stdout. The host runs this wrapper as one // long-lived session command and reads the frames from the session log stream, @@ -3040,8 +3097,7 @@ import { promises as fs, constants as fsConstants } from "node:fs"; import path from "node:path"; const sessionDir = process.env.PAPERCLIP_PROCESS_SESSION_DIR; -const commandPayload = process.env.PAPERCLIP_PROCESS_SESSION_COMMAND_B64; -if (!sessionDir || !commandPayload) throw new Error("Missing process session bridge env."); +if (!sessionDir) throw new Error("Missing process session bridge env."); const stdinDir = path.posix.join(sessionDir, "stdin"); let seq = 0; @@ -3050,7 +3106,6 @@ let shuttingDown = false; let terminated = false; let killTimer = null; -const config = JSON.parse(Buffer.from(commandPayload, "base64").toString("utf8")); await fs.mkdir(stdinDir, { recursive: true }); // One newline-delimited JSON frame per event. Node keeps process.stdout writes @@ -3078,6 +3133,8 @@ if ((await isSymbolicLink(sessionDir)) || (await isSymbolicLink(stdinDir))) { process.exit(1); } +${PROCESS_SESSION_READ_COMMAND} + // Hardening (I3, not containment): the wrapper's own launch env carries the // session dir and the command payload. Scrub both keys before they reach the // spawned child, so the child never inherits a path to its own control files. @@ -3123,8 +3180,7 @@ import { promises as fs, constants as fsConstants } from "node:fs"; import path from "node:path"; const sessionDir = process.env.PAPERCLIP_PROCESS_SESSION_DIR; -const commandPayload = process.env.PAPERCLIP_PROCESS_SESSION_COMMAND_B64; -if (!sessionDir || !commandPayload) throw new Error("Missing process session bridge env."); +if (!sessionDir) throw new Error("Missing process session bridge env."); const stdinDir = path.posix.join(sessionDir, "stdin"); const eventsDir = path.posix.join(sessionDir, "events"); @@ -3134,7 +3190,6 @@ let shuttingDown = false; let terminated = false; let killTimer = null; -const config = JSON.parse(Buffer.from(commandPayload, "base64").toString("utf8")); await fs.mkdir(stdinDir, { recursive: true }); await fs.mkdir(eventsDir, { recursive: true }); @@ -3169,6 +3224,8 @@ if ((await isSymbolicLink(sessionDir)) || (await isSymbolicLink(stdinDir))) { process.exit(1); } +${PROCESS_SESSION_READ_COMMAND} + // Hardening (I3, not containment): the wrapper's own launch env carries the // session dir and the command payload. Scrub both keys before they reach the // spawned child, so the child never inherits a path to its own control files.