fix: preserve sandbox tool environments and incoming file versions
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
97bf34eb60
commit
14151ff69f
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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<string, string>).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");
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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("-");
|
||||
|
||||
|
|
|
|||
|
|
@ -1206,6 +1206,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
|||
{
|
||||
resumeSessionId,
|
||||
skipGitRepoCheck: executionTargetIsSandbox,
|
||||
preserveSandboxEnvironment: executionTargetIsSandbox && Boolean(runtimeExecutionTarget.workFolderHome),
|
||||
},
|
||||
);
|
||||
const args = execArgs.args;
|
||||
|
|
|
|||
|
|
@ -34,6 +34,21 @@ afterEach(async () => {
|
|||
});
|
||||
|
||||
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"],
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"]);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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<ReturnType<typeof prepare>>;
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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(); }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue