From 14151ff69f310871e98595f54ec05e5fc60701cf Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 8 Sep 2026 18:02:00 -0500 Subject: [PATCH] fix: preserve sandbox tool environments and incoming file versions Co-Authored-By: Paperclip --- doc/sandbox-work-folders.md | 8 +++++ .../adapter-utils/src/acpx-engine/execute.ts | 14 ++++++-- .../startup-characterization.test.ts | 14 ++++++++ .../codex-local/src/server/codex-args.test.ts | 9 +++++ .../codex-local/src/server/codex-args.ts | 4 +++ .../codex-local/src/server/execute.ts | 1 + .../src/drivers/acpx/runtime-sandbox.test.ts | 15 ++++++++ .../src/drivers/acpx/runtime-sandbox.ts | 4 +++ .../codex/codex-security-config.test.ts | 2 ++ .../drivers/codex/codex-security-config.ts | 2 ++ .../__tests__/sandbox-work-folders.test.ts | 36 +++++++++++++++++++ server/src/services/sandbox-work-folders.ts | 3 ++ 12 files changed, 110 insertions(+), 2 deletions(-) diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 9d77cd09ec..c145be30a4 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -85,6 +85,10 @@ Legacy adapters use the host-bound sandbox home for both CLI launch and skill discovery; a private per-run runtime directory must not override it. CLI-specific configuration remains separately staged beneath that home. Local and SSH homes are unchanged. +Sandbox Codex tool commands preserve the environment initialized by the adapter: +login-shell execution and shell snapshots are disabled so image profiles cannot +replace the managed Git PATH. This applies to CLI and ACP execution in both +runner generations; local execution keeps its existing settings. CLI state is separate from the four shared collections. A change of task, agent, responsible user, or project cannot reuse a sandbox with another binding. @@ -105,6 +109,10 @@ Startup hydrates only the four bound collections. A warm startup first saves uncheckpointed local changes and then downloads changed incoming files. There is no background incoming refresh while an agent edits. Explicit refresh is queued until the run stops, after a successful final flush. +If another writer changes a shared file between listing and download, incoming +transfer uses the downloaded version's size, hash, and executable bit. Its sync +baseline records those same bytes, so an unchanged copy cannot overwrite a later +shared edit. Outgoing checkpoints run every **180 seconds**, with at most one in flight, and a final flush when execution stops. File signatures include content and diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index f190405b85..8b9f4b5784 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -1376,9 +1376,10 @@ function buildCodexStartupConfig(input: { requestedModel: string; requestedThinkingEffort: string; fastMode: boolean; + preserveSandboxEnvironment: boolean; }): { value: string | null; invalidExistingConfig: boolean } { const hasRuntimeConfig = Boolean( - input.requestedModel || input.requestedThinkingEffort || input.fastMode, + input.requestedModel || input.requestedThinkingEffort || input.fastMode || input.preserveSandboxEnvironment, ); if (!hasRuntimeConfig) return { value: null, invalidExistingConfig: false }; @@ -1400,12 +1401,18 @@ function buildCodexStartupConfig(input: { ...(input.requestedThinkingEffort ? { model_reasoning_effort: input.requestedThinkingEffort } : {}), + ...(input.preserveSandboxEnvironment ? { allow_login_shell: false } : {}), ...(input.fastMode ? { service_tier: "fast", + } + : {}), + ...(input.fastMode || input.preserveSandboxEnvironment + ? { features: { ...parseObject(existing.features), - fast_mode: true, + ...(input.fastMode ? { fast_mode: true } : {}), + ...(input.preserveSandboxEnvironment ? { shell_snapshot: false } : {}), }, } : {}), @@ -1930,6 +1937,9 @@ async function buildRuntime(input: { requestedModel, requestedThinkingEffort, fastMode, + // The runtime already initialized its login environment. Tool login + // shells must not replace the run's managed Git PATH with image defaults. + preserveSandboxEnvironment: Boolean(workFolderHome), }); if (codexStartupConfig.invalidExistingConfig) { await input.ctx.onLog( diff --git a/packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts b/packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts index c0a900f8c8..4fe1ed0870 100644 --- a/packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts +++ b/packages/adapter-utils/src/acpx-engine/startup-characterization.test.ts @@ -533,6 +533,20 @@ describe("ACPX engine startup characterization", () => { expect(sessionInputs[0]?.cwd).toBe(remoteCwd); }); + it("preserves the Codex tool environment for work-folder sandboxes without changing other features", async () => { + const { stateDir, executionTarget, remoteCwd } = await setupRemoteSandbox(); + const { meta } = await runExecutor({ + agent: "codex", stateDir, cwd: remoteCwd, + env: { CODEX_CONFIG: JSON.stringify({ features: { shell_snapshot: true, existing_feature: true } }) }, + }, { + authToken: "real-run-jwt", + executionTarget: { ...executionTarget, workFolderHome: remoteCwd }, + }); + expect(JSON.parse(String((meta[0]?.env as Record).CODEX_CONFIG))).toEqual({ + allow_login_shell: false, features: { shell_snapshot: false, existing_feature: true }, + }); + }); + it("keeps sandbox work folders remote while spawning the ACP proxy on the host", async () => { const { root, stateDir, executionTarget } = await setupRemoteSandbox(); const home = path.join(root, "sandbox-home"); diff --git a/packages/adapters/codex-local/src/server/codex-args.test.ts b/packages/adapters/codex-local/src/server/codex-args.test.ts index 3777f5e65e..82a65700b5 100644 --- a/packages/adapters/codex-local/src/server/codex-args.test.ts +++ b/packages/adapters/codex-local/src/server/codex-args.test.ts @@ -2,6 +2,15 @@ import { describe, expect, it } from "vitest"; import { buildCodexExecArgs } from "./codex-args.js"; describe("buildCodexExecArgs", () => { + it.each([null, "existing-session"])("preserves the sandbox tool environment when resuming %s", (resumeSessionId) => { + const result = buildCodexExecArgs({ extraArgs: ["-c", "allow_login_shell=true"] }, + { resumeSessionId, preserveSandboxEnvironment: true }); + expect(result.args).toContain("features.shell_snapshot=false"); + expect(result.args.indexOf("allow_login_shell=false")).toBeGreaterThan(result.args.indexOf("allow_login_shell=true")); + expect(result.args.slice(-1)).toEqual(["-"]); + expect(buildCodexExecArgs({}).args).toEqual(["exec", "--json", "-"]); + }); + it("forwards GPT-6 Astra, its ultra reasoning effort, and fast mode", () => { const result = buildCodexExecArgs({ model: "gpt-6-astra", diff --git a/packages/adapters/codex-local/src/server/codex-args.ts b/packages/adapters/codex-local/src/server/codex-args.ts index db9b349c0a..e8564a5388 100644 --- a/packages/adapters/codex-local/src/server/codex-args.ts +++ b/packages/adapters/codex-local/src/server/codex-args.ts @@ -36,6 +36,7 @@ export function buildCodexExecArgs( options: { resumeSessionId?: string | null; skipGitRepoCheck?: boolean; + preserveSandboxEnvironment?: boolean; } = {}, ): BuildCodexExecArgsResult { const record = asRecord(config); @@ -72,6 +73,9 @@ export function buildCodexExecArgs( args.push("-c", 'service_tier="fast"', "-c", "features.fast_mode=true"); } if (extraArgs.length > 0) args.push(...extraArgs); + if (options.preserveSandboxEnvironment) { + args.push("-c", "allow_login_shell=false", "-c", "features.shell_snapshot=false"); + } if (options.resumeSessionId) args.push("resume", options.resumeSessionId, "-"); else args.push("-"); diff --git a/packages/adapters/codex-local/src/server/execute.ts b/packages/adapters/codex-local/src/server/execute.ts index 436cd585b4..9d08477e2c 100644 --- a/packages/adapters/codex-local/src/server/execute.ts +++ b/packages/adapters/codex-local/src/server/execute.ts @@ -1206,6 +1206,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise { }); describe("ACPX runtime sandbox", () => { + it("preserves the host-prepared tool environment in an external work-folder sandbox", async () => { + const fixture = await sandboxFixture("codex"); + const home = join(fixture.root, "home"); + const sandbox = await prepareAcpxRuntimeSandbox({ + binding: fixture.binding, agent: "codex", + environment: { + HOME: home, PAPERCLIP_RUNNER_EXTERNAL_SANDBOX: "1", + ...Object.fromEntries(["task", "agent", "user", "project", "repos"].map((scope) => + [`PAPERCLIP_${scope.toUpperCase()}_DIR`, join(home, scope)])), + }, + }); + expect(await readFile(join(sandbox.agentHomeDirectory, "config.toml"), "utf8")) + .toBe("allow_login_shell = false\n\n[features]\nshell_snapshot = false\n"); + }); + it.each([ ["pi", "OPENROUTER_API_KEY", "pi-home"], ["claude", "ANTHROPIC_API_KEY", "claude-home"], diff --git a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts index 8a0a7d341d..7738627d7b 100644 --- a/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts +++ b/packages/paperclip-runner/src/drivers/acpx/runtime-sandbox.ts @@ -390,6 +390,10 @@ export async function prepareAcpxRuntimeSandbox(input: { await writePrivateFile( join(agentHomeDirectory, "config.toml"), [ + // The host initialized this environment before launching Codex. Tool + // login shells would replace its managed Git PATH with image defaults. + ...(externalWorkFolderEnvironment(input.environment ?? {}).HOME + ? ["allow_login_shell = false", ""] : []), // Codex shell snapshots serialize the provider process environment. // The ACPX sidecar receives a short-lived managed credential only so // it can authenticate the provider; that value must never become 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 c500674783..9d3162fb0b 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 @@ -96,6 +96,7 @@ describe("Codex security configuration", () => { GITHUB_TOKEN: "github-secret", OPENAI_API_KEY: "provider-secret", CODEX_HOME: "/home/daytona/.codex", DATABASE_URL: "host-secret", }); + expect(args).toContain("allow_login_shell=false"); const prefix = "shell_environment_policy.include_only="; const allowed = JSON.parse(args.find((arg) => arg.startsWith(prefix))!.slice(prefix.length)); // Codex applies this allowlist AFTER its explicit environment overrides. @@ -110,6 +111,7 @@ describe("Codex security configuration", () => { HOME: "/host/private", CODEX_HOME: "/host/codex", PATH: "/bin", GITHUB_TOKEN: "github-secret", }); + expect(localArgs).not.toContain("allow_login_shell=false"); const localAllowed = JSON.parse(localArgs.find((arg) => arg.startsWith(prefix))!.slice(prefix.length)); expect(localAllowed.sort()).toEqual(["GITHUB_TOKEN", "PATH"]); }); 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 841cc28e83..7ddedbc3f6 100644 --- a/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts +++ b/packages/paperclip-runner/src/drivers/codex/codex-security-config.ts @@ -136,6 +136,8 @@ export function createIsolatedCodexAppServerArgs( `permissions.${CODEX_SKILLLESS_PERMISSION_PROFILE}.network.enabled=${hasGitHubCredential}`, ...(externalRunnerSandbox ? [ + "-c", + "allow_login_shell=false", "-c", `permissions.${CODEX_EXTERNAL_SANDBOX_PERMISSION_PROFILE}.filesystem={":root"="write"}`, "-c", diff --git a/server/src/__tests__/sandbox-work-folders.test.ts b/server/src/__tests__/sandbox-work-folders.test.ts index 69a38b5637..15393464b8 100644 --- a/server/src/__tests__/sandbox-work-folders.test.ts +++ b/server/src/__tests__/sandbox-work-folders.test.ts @@ -13,6 +13,7 @@ import { bindWarmSandboxWorkspace } from "../services/sandbox-workspace-binding. import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "../services/work-folder-retention.js"; import * as activityLog from "../services/activity-log.js"; import { workFolderService } from "../services/work-folders.js"; +import * as workFolderServices from "../services/work-folders.js"; import { collectWorkFolderGarbage } from "../services/work-folder-garbage.js"; import { localTestWorkFolderRunner } from "./helpers/work-folder-runner.js"; const exec = promisify(execFile); @@ -106,6 +107,41 @@ describe("shared sandbox work-folder lifecycle", () => { runner: { execute: (input) => localTestWorkFolderRunner.execute({ ...input, env: { ...input.env, HOME: home } }) } } }); active.push(run); return run; } + it("uses downloaded metadata when a shared file changes after the startup listing", async () => { + const svc = workFolderService(db, storage); + const folder = await svc.ensure({ companyId, scope: "project", ownerId: projectId }); + const filePath = "startup-concurrent.txt"; + await svc.write(folder, { path: filePath, body: Buffer.from("old"), operationId: randomUUID() }); + const createService = workFolderServices.workFolderService; + let replaced = false; + const factory = vi.spyOn(workFolderServices, "workFolderService").mockImplementation((...args) => { + const service = createService(...args); + return { ...service, list: async (...listArgs) => { + const listing = await service.list(...listArgs); + if (listArgs[0].id === folder.id && !replaced) { + replaced = true; + await svc.write(folder, { path: filePath, body: Buffer.from("new content with a different size"), + executable: true, operationId: randomUUID() }); + } + return listing; + } }; + }); + let run: Awaited>; + try { run = await prepare(path.join(root, "startup-concurrent"), randomUUID()); } + finally { factory.mockRestore(); } + expect(replaced).toBe(true); + expect(await fs.readFile(path.join(run.home, "project", filePath), "utf8")).toBe("new content with a different size"); + expect((await fs.stat(path.join(run.home, "project", filePath))).mode & 0o111).not.toBe(0); + // The baseline must describe the bytes actually received. An unchanged + // sandbox must not overwrite a still newer shared edit during final flush. + await svc.write(folder, { path: filePath, body: Buffer.from("another writer"), operationId: randomUUID() }); + await run.stop(); active.splice(active.indexOf(run), 1); + const content = await svc.content(folder, filePath); + let text = ""; + for await (const chunk of content.stream) text += chunk.toString(); + expect(text).toBe("another writer"); + }, 120_000); + it("retains unaudited edits and retries without misreporting a completed checkpoint", async () => { const run = await prepare(path.join(root, "activity-failure"), randomUUID()); await run.flush(); diff --git a/server/src/services/sandbox-work-folders.ts b/server/src/services/sandbox-work-folders.ts index be1c31f96e..ea8ce040ca 100644 --- a/server/src/services/sandbox-work-folders.ts +++ b/server/src/services/sandbox-work-folders.ts @@ -197,6 +197,9 @@ export async function prepareSandboxWorkFolders(input: { if (entry.kind === "directory") await transport.mkdir(paths[scope]!, entry.path); else { const result = await svc.content(folder, entry.path); + // A shared file can change after listing. Validate and baseline the + // version opened by content(), whose metadata and stream belong together. + Object.assign(entry, { byteSize: result.file.byteSize, sha256: result.file.sha256, executable: result.file.executable }); try { await transport.write(paths[scope]!, staging, entry, result.stream); } finally { result.stream.destroy(); } } }