From e372e4667bdc78bc485f441894ecf2872d21b23e Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 20 Apr 2025 09:37:07 -0700 Subject: [PATCH] Make it so CONFIG_DIR is not in the list of writable roots by default (#419) To play it safe, let's keep `CONFIG_DIR` out of the default list of writable roots. This also fixes an issue where `execWithSeatbelt()` was modifying `writableRoots` instead of creating a new array. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/419). * #423 * #420 * __->__ #419 --- .../src/utils/agent/sandbox/macos-seatbelt.ts | 15 +++++++-------- codex-cli/src/utils/agent/sandbox/raw-exec.ts | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts index 03174582..760ba63d 100644 --- a/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts +++ b/codex-cli/src/utils/agent/sandbox/macos-seatbelt.ts @@ -3,11 +3,9 @@ import type { SpawnOptions } from "child_process"; import { exec } from "./raw-exec.js"; import { log } from "../log.js"; -import { CONFIG_DIR } from "src/utils/config.js"; function getCommonRoots() { return [ - CONFIG_DIR, // Without this root, it'll cause: // pyenv: cannot rehash: $HOME/.pyenv/shims isn't writable `${process.env["HOME"]}/.pyenv`, @@ -17,16 +15,17 @@ function getCommonRoots() { export function execWithSeatbelt( cmd: Array, opts: SpawnOptions, - writableRoots: Array, + writableRoots: ReadonlyArray, abortSignal?: AbortSignal, ): Promise { let scopedWritePolicy: string; let policyTemplateParams: Array; - if (writableRoots.length > 0) { - // Add `~/.codex` to the list of writable roots - // (if there's any already, not in read-only mode) - getCommonRoots().map((root) => writableRoots.push(root)); - const { policies, params } = writableRoots + + const fullWritableRoots = [...writableRoots, ...getCommonRoots()]; + // In practice, fullWritableRoots will be non-empty, but we check just in + // case the logic to build up fullWritableRoots changes. + if (fullWritableRoots.length > 0) { + const { policies, params } = fullWritableRoots .map((root, index) => ({ policy: `(subpath (param "WRITABLE_ROOT_${index}"))`, param: `-DWRITABLE_ROOT_${index}=${root}`, diff --git a/codex-cli/src/utils/agent/sandbox/raw-exec.ts b/codex-cli/src/utils/agent/sandbox/raw-exec.ts index 6cfb3047..e7bcdb2a 100644 --- a/codex-cli/src/utils/agent/sandbox/raw-exec.ts +++ b/codex-cli/src/utils/agent/sandbox/raw-exec.ts @@ -21,7 +21,7 @@ const MAX_BUFFER = 1024 * 100; // 100 KB export function exec( command: Array, options: SpawnOptions, - _writableRoots: Array, + _writableRoots: ReadonlyArray, abortSignal?: AbortSignal, ): Promise { // Adapt command for the current platform (e.g., convert 'ls' to 'dir' on Windows)