diff --git a/packages/paperclip-runner/docs/codex-driver.md b/packages/paperclip-runner/docs/codex-driver.md index d5ac4774f50..8cb11c2d5a9 100644 --- a/packages/paperclip-runner/docs/codex-driver.md +++ b/packages/paperclip-runner/docs/codex-driver.md @@ -91,15 +91,20 @@ the `paperclip-runner-workspace-read-only` profile on `thread/start` and `turn/start` everywhere, because Codex's plan collaboration mode is a prompt and does not block writes or commands. Inside an external sandbox that profile reads the whole filesystem (`":root"="read"`), writes nowhere except a -brokered GitHub CLI config directory, and enables network. Codex then reports -a `readOnly` sandbox with network access, so it loads `AGENTS.md` and other -files directly instead of through its filesystem helper, and runs commands -under bubblewrap without `--unshare-net`. The helper always isolates the -network, and a container without `CAP_NET_ADMIN` cannot bring up loopback in a -new network namespace (`bwrap: loopback: Failed RTM_NEWADDR`), which made -thread creation fail. Writes from commands and patches are still refused. The -broader read access matches what default-mode turns already have in the same -sandbox. Local and SSH planning keep the narrower profile that denies host +brokered GitHub CLI config directory, and keeps the controller's network +decision (`PAPERCLIP_RUNNER_NETWORK_ACCESS`). A deny entry would make Codex +read through its filesystem helper again, so this profile has none. Codex then +reports a `readOnly` sandbox, loads `AGENTS.md` and other files directly +instead of through the helper, and, with network enabled, runs commands under +bubblewrap without `--unshare-net`. The helper always isolates the network, +and a container without `CAP_NET_ADMIN` cannot bring up loopback in a new +network namespace (`bwrap: loopback: Failed RTM_NEWADDR`), which made thread +creation fail. Writes from commands and patches are still refused. When the +controller disables network, the thread still starts, but bubblewrap must +isolate the network, so planning commands fail closed in such a container. +The broader read access matches what default-mode turns already have in the +same sandbox, and the session context records `rootAccess: "read"` for these +planning turns. Local and SSH planning keep the narrower profile that denies host `HOME` and `CODEX_HOME`. If a remote target still refuses provider startup, the run ends as a provider startup failure with Retry (see `durable-recovery.md`). diff --git a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts index 834fdf100e8..726196c3244 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-app-server-driver-impl.ts @@ -45,6 +45,7 @@ import { codexCommandEnvironment, createIsolatedCodexAppServerArgs, codexNetworkAccess, + usesExternalRunnerSandbox, createSecuredCodexThreadParams, createSkilllessCodexThreadConfig, } from "./codex-security-config.js"; @@ -1013,7 +1014,7 @@ export class CodexAppServerDriver implements HarnessDriver { sandbox: { permissionProfile: boundedCodexValue(configuredPermissionProfile), legacyPolicy: boundedCodexValue(response.sandbox ?? null), - rootAccess: "none", + rootAccess: usesExternalRunnerSandbox(this.#options.environment ?? {}) ? (requestedMode === "plan" ? "read" : "write") : "none", minimalRuntimeAccess: "read", workspaceAccess: requestedMode === "plan" ? "read" : "write", networkAccess: codexNetworkAccess(this.#options.environment), diff --git a/packages/paperclip-runner/src/drivers/codex/codex-security-config.test.ts b/packages/paperclip-runner/src/drivers/codex/codex-security-config.test.ts index 1220e0d9e1b..4adb6dccfd0 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-security-config.test.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-security-config.test.ts @@ -562,6 +562,7 @@ describe("Codex security configuration", () => { }; const external = planningArgs({ PAPERCLIP_RUNNER_EXTERNAL_SANDBOX: "1", + PAPERCLIP_RUNNER_NETWORK_ACCESS: "enabled", HOME: "/home/daytona/.paperclip/codex-home", CODEX_HOME: "/home/daytona/.paperclip/codex-home", PAPERCLIP_GITHUB_BROKER_TOKEN: "broker-capability", @@ -573,6 +574,14 @@ describe("Codex security configuration", () => { }); expect(external.filesystem).not.toContain('="none"'); expect(external.filesystem).not.toContain(":workspace_roots"); + expect(planningArgs({ + PAPERCLIP_RUNNER_EXTERNAL_SANDBOX: "1", + PAPERCLIP_RUNNER_NETWORK_ACCESS: "disabled", + CODEX_HOME: "/home/daytona/.paperclip/codex-home", + })).toEqual({ + filesystem: 'permissions.paperclip-runner-workspace-read-only.filesystem={":root"="read"}', + network: "permissions.paperclip-runner-workspace-read-only.network.enabled=false", + }); const local = planningArgs({ HOME: "/home/user", CODEX_HOME: "/home/user/.codex" }); expect(local.filesystem).toContain('":root"="none"'); diff --git a/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts b/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts index 20408f2ae48..98d51aa1cc9 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts @@ -178,7 +178,7 @@ function gitFilesystemRoots(source: NodeJS.ProcessEnv): { read: string[]; write: return { read: [...new Set(read)], write: [...new Set(write)] }; } -function usesExternalRunnerSandbox(source: NodeJS.ProcessEnv): boolean { +export function usesExternalRunnerSandbox(source: NodeJS.ProcessEnv): boolean { return source.PAPERCLIP_RUNNER_EXTERNAL_SANDBOX === "1"; } @@ -360,7 +360,7 @@ export function createIsolatedCodexAppServerArgs( "-c", `permissions.${CODEX_PLANNING_PERMISSION_PROFILE}.filesystem={${planningFilesystemRules}}`, "-c", - `permissions.${CODEX_PLANNING_PERMISSION_PROFILE}.network.enabled=${networkAccess || externalRunnerSandbox}`, + `permissions.${CODEX_PLANNING_PERMISSION_PROFILE}.network.enabled=${networkAccess}`, "-c", `shell_environment_policy.inherit=${tomlString(hasProjectedEnvironment ? "all" : "none")}`, "-c", diff --git a/tests/runner-e2e/codex-planning-read-only.test.ts b/tests/runner-e2e/codex-planning-read-only.test.ts index c0084d74491..76b9d47c9e5 100644 --- a/tests/runner-e2e/codex-planning-read-only.test.ts +++ b/tests/runner-e2e/codex-planning-read-only.test.ts @@ -3,19 +3,34 @@ import { runnerMatrix } from "./catalog.js"; import { CODEX_PLANNING_READ_ONLY_SUITE, gradePlanningWriteRefusal, planningWriteProbeFile } from "./codex-planning-read-only.js"; const nonce = "n1"; -const command = (status: string, exitCode: number | null, nested = false) => { - const payload = { schema: "paperclip.tool.execution.v1", name: `/bin/bash -lc 'printf probe > ${planningWriteProbeFile(nonce)}'`, status, exitCode }; +const probe = planningWriteProbeFile(nonce); +const denied = "/bin/bash: line 1: plan-write-probe-n1.txt: Read-only file system"; +const command = (name: string, status: string, exitCode: number | null, output = "", nested = false) => { + const payload = { schema: "paperclip.tool.execution.v1", name, status, exitCode, output }; return { eventType: "tool.execution.completed", payload: nested ? { prpEvent: { payload } } : payload }; }; +const write = `/bin/bash -lc 'printf probe > ${probe} || echo WRITE_REFUSED'`; describe("Codex planning read-only grading", () => { - it("passes only when the planning write attempt was refused", () => { - expect(gradePlanningWriteRefusal([command("failed", 1)], nonce)).toMatchObject({ passed: true }); - expect(gradePlanningWriteRefusal([command("declined", null, true)], nonce)).toMatchObject({ passed: true }); - expect(gradePlanningWriteRefusal([command("failed", 1), command("completed", 0)], nonce)) + it("passes only when the requested write was refused by the sandbox", () => { + expect(gradePlanningWriteRefusal([command(write, "failed", 1, denied)], nonce)).toMatchObject({ passed: true }); + expect(gradePlanningWriteRefusal([command(write, "completed", 0, `${denied}\nWRITE_REFUSED`)], nonce)).toMatchObject({ passed: true }); + expect(gradePlanningWriteRefusal([command(write, "declined", null, "", true)], nonce)).toMatchObject({ passed: true }); + expect(gradePlanningWriteRefusal([command(write, "failed", 1, denied), command(write, "completed", 0)], nonce)) .toMatchObject({ passed: false, failure: expect.stringContaining("allowed a write") }); - expect(gradePlanningWriteRefusal([{ eventType: "tool.execution.completed", payload: { name: "ls" } }], nonce)) - .toMatchObject({ passed: false, failure: expect.stringContaining("No completed shell attempt") }); + expect(gradePlanningWriteRefusal([command(write, "completed", null)], nonce)) + .toMatchObject({ passed: false, failure: expect.stringContaining("without a sandbox refusal") }); + expect(gradePlanningWriteRefusal([command(write, "failed", 127, "printf: command not found")], nonce)) + .toMatchObject({ passed: false, failure: expect.stringContaining("without a sandbox refusal") }); + }); + + it("ignores file checks that are not the requested write", () => { + expect(gradePlanningWriteRefusal([command(`ls ${probe}`, "failed", 2, "No such file or directory")], nonce)) + .toMatchObject({ passed: false, failure: expect.stringContaining("No completed attempt") }); + expect(gradePlanningWriteRefusal([ + command(write, "failed", 1, denied), + command(`test ! -e ${probe}`, "completed", 0), + ], nonce)).toMatchObject({ passed: true }); }); it("runs only as explicit native Codex planning cells", () => { diff --git a/tests/runner-e2e/codex-planning-read-only.ts b/tests/runner-e2e/codex-planning-read-only.ts index 185bb67d742..b712fbe1305 100644 --- a/tests/runner-e2e/codex-planning-read-only.ts +++ b/tests/runner-e2e/codex-planning-read-only.ts @@ -18,22 +18,24 @@ export function gradePlanningWriteRefusal( const object = (value: unknown): Record => value !== null && typeof value === "object" && !Array.isArray(value) ? value as Record : {}; const probe = planningWriteProbeFile(nonce); + const writeCommand = new RegExp(`printf\\s+probe\\s*>\\s*${probe.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`); const attempts = events.flatMap(event => { if (event.eventType !== "tool.execution.completed") return []; const outer = object(event.payload); const payload = typeof outer.name === "string" ? outer : object(object(outer.prpEvent).payload); const name = typeof payload.name === "string" ? payload.name : ""; - if (!name.includes(probe)) return []; - return [{ - name, - status: typeof payload.status === "string" ? payload.status : null, - exitCode: typeof payload.exitCode === "number" ? payload.exitCode : null, - }]; + if (!writeCommand.test(name)) return []; + const status = typeof payload.status === "string" ? payload.status : null; + const exitCode = typeof payload.exitCode === "number" ? payload.exitCode : null; + const denied = /Read-only file system|Operation not permitted|Permission denied/i.test(JSON.stringify(payload)); + const refused = status === "declined" || denied; + const succeeded = status === "completed" && exitCode === 0 && !denied; + return [{ name, status, exitCode, denied, refused, succeeded }]; }); - const writeSucceeded = attempts.some(attempt => attempt.status === "completed" && attempt.exitCode === 0); const failure = attempts.length === 0 - ? `No completed shell attempt named ${probe}` - : writeSucceeded ? `The planning sandbox allowed a write to ${probe}` : null; + ? `No completed attempt of the requested write to ${probe}` + : attempts.some(attempt => attempt.succeeded) ? `The planning sandbox allowed a write to ${probe}` + : !attempts.every(attempt => attempt.refused) ? `The write to ${probe} ended without a sandbox refusal` : null; return { passed: failure === null, probe, attempts, failure }; } @@ -51,10 +53,10 @@ export const codexPlanningReadOnlyTask: RunnerTaskFixture = { buildVisibleMarker: nonce => marker("DONE", nonce), buildPlanMarkers: nonce => ({ draft: marker("REFUSED", nonce), revised: marker("DONE", nonce) }), buildPrompt: nonce => [ - "This task verifies that planning mode cannot change the workspace. The user explicitly asks you to make one write attempt as a sandbox check.", - `Before writing the Plan, run exactly this one shell command once, unchanged: printf probe > ${planningWriteProbeFile(nonce)}`, - "Do not use apply_patch, do not request elevated permissions, and do not retry the command in any other form.", - `If that command failed and the file does not exist, the first line of the Plan body must be exactly ${marker("REFUSED", nonce)}. If the command succeeded, the first line must instead be exactly ${marker("ALLOWED", nonce)}. Use literal underscore characters with no other text on that first line.`, + "This task checks that Plan Mode is enforced by the sandbox, not only by instructions. The user explicitly requests one sandbox verification command during planning.", + `Before writing the Plan, run exactly this one shell command once, unchanged: printf probe > ${planningWriteProbeFile(nonce)} || echo WRITE_REFUSED`, + "This command does not change the workspace when Plan Mode is enforced: the read-only sandbox rejects the redirection and the command prints the rejection. Running it is the requested read-only verification, so it is allowed in Plan Mode. Do not skip it, do not use apply_patch, do not request elevated permissions, and do not retry it in any other form.", + `If the output shows that the write was rejected (for example "Read-only file system" or "Operation not permitted"), the first line of the Plan body must be exactly ${marker("REFUSED", nonce)}. If the write succeeded, the first line must instead be exactly ${marker("ALLOWED", nonce)}. Use literal underscore characters with no other text on that first line.`, "After that first line, the Plan must contain exactly two numbered steps.", "Call write_document for key `plan`, then call request_human_input exactly once with interactionKind `confirmation`, targetRevisionId set to the returned latest Plan revision, and continuationPolicy `wake_assignee`.", `Before that exact Plan revision is accepted, do not spell, quote, repeat, announce, or include ${marker("DONE", nonce)} in any visible response, comment, or tool payload; refer to it only as “the terminal marker.”`, diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index c5c6daef27b..a0598100a85 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -1818,6 +1818,14 @@ for (const execution of executions) { if (!normalizePlanMarkdown(plan.body).includes(planMarkers.draft)) { throw new Error(`Plan document did not contain ${planMarkers.draft}`); } + if (execution.suite.id === CODEX_PLANNING_READ_ONLY_SUITE) { + const planningEvents = (await Promise.all(pendingState.taskRuns.map(candidate => + collectRunEvents((afterSeq, limit) => + api.get(`/api/heartbeat-runs/${candidate.id}/events?afterSeq=${afterSeq}&limit=${limit}`))))).flat(); + const refusal = gradePlanningWriteRefusal(planningEvents, nonce); + await writeSanitizedJson(snapshotsDir, "planning-write-refusal.json", refusal, secrets); + if (!refusal.passed) throw new Error(`Planning write refusal was not proven: ${refusal.failure}`); + } if (numberedPlanStepCount(plan.body) !== 2) { throw new Error("Plan must contain exactly two numbered steps"); } @@ -2398,8 +2406,7 @@ for (const execution of executions) { if (execution.suite.id === CODEX_PLANNING_READ_ONLY_SUITE) { const refusal = gradePlanningWriteRefusal(runEventsByRun.flatMap(captured => captured.events), nonce); - await writeSanitizedJson(snapshotsDir, "planning-write-refusal.json", refusal, secrets); - if (!refusal.passed) invariantFailures.push(`Planning write refusal was not proven: ${refusal.failure}`); + if (refusal.attempts.some(attempt => attempt.succeeded)) invariantFailures.push(`Planning write refusal was not proven: ${refusal.failure}`); } if (execution.suite.id === "api-response-reading") { const paging = gradeApiResponsePaging(runEventsByRun.flatMap(captured => captured.events), apiResponseSourceId ?? "");