From 9b0ccf9aebff1396a05eded989921bf79af47292 Mon Sep 17 00:00:00 2001 From: moppywhip <48742547+moppywhip@users.noreply.github.com> Date: Sat, 26 Apr 2025 12:14:50 -0400 Subject: [PATCH 1/7] fix: duplicate messages in quiet mode (#680) Addressing #600 and #664 (partially) ## Bug Codex was staging duplicate items in output running when the same response item appeared in both the streaming events. Specifically: 1. Items would be staged once when received as a `response.output_item.done` event 2. The same items would be staged again when included in the final `response.completed` payload This duplication would result in each message being sent several times in the quiet mode output. ## Changes - Added a Set (`alreadyStagedItemIds`) to track items that have already been staged - Modified the `stageItem` function to check if an item's ID is already in this set before staging it - Added a regression test (`agent-dedupe-items.test.ts`) that verifies items with the same ID are only staged once ## Testing Like other tests, the included test creates a mock OpenAI stream that emits the same message twice (once as an incremental event and once in the final response) and verifies the item is only passed to `onItem` once. --- codex-cli/src/utils/agent/agent-loop.ts | 7 ++ codex-cli/tests/agent-dedupe-items.test.ts | 115 +++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 codex-cli/tests/agent-dedupe-items.test.ts diff --git a/codex-cli/src/utils/agent/agent-loop.ts b/codex-cli/src/utils/agent/agent-loop.ts index b56c3efc8e..5fca001617 100644 --- a/codex-cli/src/utils/agent/agent-loop.ts +++ b/codex-cli/src/utils/agent/agent-loop.ts @@ -46,6 +46,7 @@ export type CommandConfirmation = { }; const alreadyProcessedResponses = new Set(); +const alreadyStagedItemIds = new Set(); type AgentLoopParams = { model: string; @@ -562,6 +563,12 @@ export class AgentLoop { return; } + // Skip items we've already processed to avoid staging duplicates + if (item.id && alreadyStagedItemIds.has(item.id)) { + return; + } + alreadyStagedItemIds.add(item.id); + // Store the item so the final flush can still operate on a complete list. // We'll nil out entries once they're delivered. const idx = staged.push(item) - 1; diff --git a/codex-cli/tests/agent-dedupe-items.test.ts b/codex-cli/tests/agent-dedupe-items.test.ts new file mode 100644 index 0000000000..148aa7440f --- /dev/null +++ b/codex-cli/tests/agent-dedupe-items.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi } from "vitest"; + +// --------------------------------------------------------------------------- +// This regression test ensures that AgentLoop only surfaces each response item +// once even when the same item appears multiple times in the OpenAI streaming +// response (e.g. as an early `response.output_item.done` event *and* again in +// the final `response.completed` payload). +// --------------------------------------------------------------------------- + +// Fake OpenAI stream that emits the *same* message twice: first as an +// incremental output event and then again in the turn completion payload. +class FakeStream { + public controller = { abort: vi.fn() }; + + async *[Symbol.asyncIterator]() { + // 1) Early incremental item. + yield { + type: "response.output_item.done", + item: { + type: "message", + id: "call-dedupe-1", + role: "assistant", + content: [{ type: "input_text", text: "Hello!" }], + }, + } as any; + + // 2) Turn completion containing the *same* item again. + yield { + type: "response.completed", + response: { + id: "resp-dedupe-1", + status: "completed", + output: [ + { + type: "message", + id: "call-dedupe-1", + role: "assistant", + content: [{ type: "input_text", text: "Hello!" }], + }, + ], + }, + } as any; + } +} + +// Intercept the OpenAI SDK used inside AgentLoop so we can inject our fake +// streaming implementation. +vi.mock("openai", () => { + class FakeOpenAI { + public responses = { + create: async () => new FakeStream(), + }; + } + + class APIConnectionTimeoutError extends Error {} + + return { __esModule: true, default: FakeOpenAI, APIConnectionTimeoutError }; +}); + +// Stub approvals / formatting helpers – not relevant here. +vi.mock("../src/approvals.js", () => ({ + __esModule: true, + alwaysApprovedCommands: new Set(), + canAutoApprove: () => ({ type: "auto-approve", runInSandbox: false }) as any, + isSafeCommand: () => null, +})); + +vi.mock("../src/format-command.js", () => ({ + __esModule: true, + formatCommandForDisplay: (cmd: Array) => cmd.join(" "), +})); + +vi.mock("../src/utils/agent/log.js", () => ({ + __esModule: true, + log: () => {}, + isLoggingEnabled: () => false, +})); + +// After the dependency mocks we can import the module under test. +import { AgentLoop } from "../src/utils/agent/agent-loop.js"; + +describe("AgentLoop deduplicates output items", () => { + it("invokes onItem exactly once for duplicate items with the same id", async () => { + const received: Array = []; + + const agent = new AgentLoop({ + model: "any", + instructions: "", + config: { model: "any", instructions: "", notify: false }, + approvalPolicy: { mode: "auto" } as any, + additionalWritableRoots: [], + onItem: (item) => received.push(item), + onLoading: () => {}, + getCommandConfirmation: async () => ({ review: "yes" }) as any, + onLastResponseId: () => {}, + }); + + const userMsg = [ + { + type: "message", + role: "user", + content: [{ type: "input_text", text: "hi" }], + }, + ]; + + await agent.run(userMsg as any); + + // Give the setTimeout(3ms) inside AgentLoop.stageItem a chance to fire. + await new Promise((r) => setTimeout(r, 20)); + + // Count how many times the duplicate item surfaced. + const appearances = received.filter((i) => i.id === "call-dedupe-1").length; + expect(appearances).toBe(1); + }); +}); From bc500d30092b2ec5f2de2b9077b5d2f88c7146c4 Mon Sep 17 00:00:00 2001 From: Tomas Cupr Date: Sat, 26 Apr 2025 19:13:30 +0200 Subject: [PATCH 2/7] feat: user config api key (#569) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds support for reading OPENAI_API_KEY (and other variables) from a user‑wide dotenv file (~/.codex.config). Precedence order is now: 1. explicit environment variable 2. project‑local .env (loaded earlier) 3. ~/.codex.config Also adds a regression test that ensures the multiline editor correctly handles cases where printable text and the CSI‑u Shift+Enter sequence arrive in the same input chunk. House‑kept with Prettier; removed stray temp.json artifact. --- codex-cli/src/utils/config.ts | 28 ++++++++++- codex-cli/tests/user-config-env.test.ts | 62 +++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 codex-cli/tests/user-config-env.test.ts diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts index 7bd6052e52..1bf0530322 100644 --- a/codex-cli/src/utils/config.ts +++ b/codex-cli/src/utils/config.ts @@ -11,11 +11,37 @@ import type { FullAutoErrorMode } from "./auto-approval-mode.js"; import { AutoApprovalMode } from "./auto-approval-mode.js"; import { log } from "./logger/log.js"; import { providers } from "./providers.js"; +import { config as loadDotenv } from "dotenv"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { load as loadYaml, dump as dumpYaml } from "js-yaml"; import { homedir } from "os"; import { dirname, join, extname, resolve as resolvePath } from "path"; +// --------------------------------------------------------------------------- +// User‑wide environment config (~/.codex.env) +// --------------------------------------------------------------------------- + +// Load a user‑level dotenv file **after** process.env and any project‑local +// .env file (loaded via "dotenv/config" in cli.tsx) are in place. We rely on +// dotenv's default behaviour of *not* overriding existing variables so that +// the precedence order becomes: +// 1. Explicit environment variables +// 2. Project‑local .env (handled in cli.tsx) +// 3. User‑wide ~/.codex.env (loaded here) +// This guarantees that users can still override the global key on a per‑project +// basis while enjoying the convenience of a persistent default. + +// Skip when running inside Vitest to avoid interfering with the FS mocks used +// by tests that stub out `fs` *after* importing this module. +const USER_WIDE_CONFIG_PATH = join(homedir(), ".codex.env"); + +const isVitest = + typeof (globalThis as { vitest?: unknown }).vitest !== "undefined"; + +if (!isVitest) { + loadDotenv({ path: USER_WIDE_CONFIG_PATH }); +} + export const DEFAULT_AGENTIC_MODEL = "o4-mini"; export const DEFAULT_FULL_CONTEXT_MODEL = "gpt-4.1"; export const DEFAULT_APPROVAL_MODE = AutoApprovalMode.SUGGEST; @@ -117,7 +143,7 @@ export type StoredConfig = { // propagating to existing users until they explicitly set a model. export const EMPTY_STORED_CONFIG: StoredConfig = { model: "" }; -// Pre‑stringified JSON variant so we don’t stringify repeatedly. +// Pre‑stringified JSON variant so we don't stringify repeatedly. const EMPTY_CONFIG_JSON = JSON.stringify(EMPTY_STORED_CONFIG, null, 2) + "\n"; export type MemoryConfig = { diff --git a/codex-cli/tests/user-config-env.test.ts b/codex-cli/tests/user-config-env.test.ts new file mode 100644 index 0000000000..76cdcbacd7 --- /dev/null +++ b/codex-cli/tests/user-config-env.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { mkdtempSync, writeFileSync, rmSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +/** + * Verifies that ~/.codex.env is parsed (lowest‑priority) when present. + */ + +describe("user‑wide ~/.codex.env support", () => { + const ORIGINAL_HOME = process.env["HOME"]; + const ORIGINAL_API_KEY = process.env["OPENAI_API_KEY"]; + + let tempHome: string; + + beforeEach(() => { + // Create an isolated fake $HOME directory. + tempHome = mkdtempSync(join(tmpdir(), "codex-home-")); + process.env["HOME"] = tempHome; + + // Ensure the env var is unset so that the file value is picked up. + delete process.env["OPENAI_API_KEY"]; + + // Write ~/.codex.env with a dummy key. + writeFileSync( + join(tempHome, ".codex.env"), + "OPENAI_API_KEY=my-home-key\n", + { + encoding: "utf8", + }, + ); + }); + + afterEach(() => { + // Cleanup temp directory. + try { + rmSync(tempHome, { recursive: true, force: true }); + } catch { + // ignore + } + + // Restore original env. + if (ORIGINAL_HOME !== undefined) { + process.env["HOME"] = ORIGINAL_HOME; + } else { + delete process.env["HOME"]; + } + + if (ORIGINAL_API_KEY !== undefined) { + process.env["OPENAI_API_KEY"] = ORIGINAL_API_KEY; + } else { + delete process.env["OPENAI_API_KEY"]; + } + }); + + it("loads the API key from ~/.codex.env when not set elsewhere", async () => { + // Import the config module AFTER setting up the fake env. + const { getApiKey } = await import("../src/utils/config.js"); + + expect(getApiKey("openai")).toBe("my-home-key"); + }); +}); From 523996b5cb30c2cbf714e61864669f8241f8ec3c Mon Sep 17 00:00:00 2001 From: Fouad Matin <169186268+fouad-openai@users.noreply.github.com> Date: Sat, 26 Apr 2025 12:43:51 -0700 Subject: [PATCH 3/7] fix: `/diff` should include untracked files (#686) --- codex-cli/src/utils/get-diff.ts | 106 ++++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 6 deletions(-) diff --git a/codex-cli/src/utils/get-diff.ts b/codex-cli/src/utils/get-diff.ts index 348ee42c78..9ac7844d2a 100644 --- a/codex-cli/src/utils/get-diff.ts +++ b/codex-cli/src/utils/get-diff.ts @@ -1,5 +1,26 @@ import { execSync } from "node:child_process"; +// The objects thrown by `child_process.execSync()` are `Error` instances that +// include additional, undocumented properties such as `status` (exit code) and +// `stdout` (captured standard output). Declare a minimal interface that captures +// just the fields we need so that we can avoid the use of `any` while keeping +// the checks type-safe. +interface ExecSyncError extends Error { + // Exit status code. When a diff is produced, git exits with code 1 which we + // treat as a non-error signal. + status?: number; + // Captured stdout. We rely on this to obtain the diff output when git exits + // with status 1. + stdout?: string; +} + +// Type-guard that narrows an unknown value to `ExecSyncError`. +function isExecSyncError(err: unknown): err is ExecSyncError { + return ( + typeof err === "object" && err != null && "status" in err && "stdout" in err + ); +} + /** * Returns the current Git diff for the working directory. If the current * working directory is not inside a Git repository, `isGitRepo` will be @@ -15,13 +36,86 @@ export function getGitDiff(): { execSync("git rev-parse --is-inside-work-tree", { stdio: "ignore" }); // If the above call didn’t throw, we are inside a git repo. Retrieve the - // diff including color codes so that the overlay can render them. - const output = execSync("git diff --color", { - encoding: "utf8", - maxBuffer: 10 * 1024 * 1024, // 10 MB ought to be enough for now - }); + // diff for tracked files **and** include any untracked files so that the + // `/diff` overlay shows a complete picture of the working tree state. - return { isGitRepo: true, diff: output }; + // 1. Diff for tracked files (unchanged behaviour) + let trackedDiff = ""; + try { + trackedDiff = execSync("git diff --color", { + encoding: "utf8", + maxBuffer: 10 * 1024 * 1024, // 10 MB ought to be enough for now + }); + } catch (err) { + // Exit status 1 simply means that differences were found. Capture the + // diff from stdout in that case. Re-throw for any other status codes. + if ( + isExecSyncError(err) && + err.status === 1 && + typeof err.stdout === "string" + ) { + trackedDiff = err.stdout; + } else { + throw err; + } + } + + // 2. Determine untracked files. + // We use `git ls-files --others --exclude-standard` which outputs paths + // relative to the repository root, one per line. These are files that + // are not tracked *and* are not ignored by .gitignore. + const untrackedOutput = execSync( + "git ls-files --others --exclude-standard", + { + encoding: "utf8", + maxBuffer: 10 * 1024 * 1024, + }, + ); + + const untrackedFiles = untrackedOutput + .split("\n") + .map((p) => p.trim()) + .filter(Boolean); + + let untrackedDiff = ""; + + const nullDevice = process.platform === "win32" ? "NUL" : "/dev/null"; + + for (const file of untrackedFiles) { + try { + // `git diff --no-index` produces a diff even outside the index by + // comparing two paths. We compare the file against /dev/null so that + // the file is treated as "new". + // + // `git diff --color --no-index /dev/null ` exits with status 1 + // when differences are found, so we capture stdout from the thrown + // error object instead of letting it propagate. + execSync(`git diff --color --no-index -- "${nullDevice}" "${file}"`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + maxBuffer: 10 * 1024 * 1024, + }); + } catch (err) { + if ( + isExecSyncError(err) && + // Exit status 1 simply means that the two inputs differ, which is + // exactly what we expect here. Any other status code indicates a + // real error (e.g. the file disappeared between the ls-files and + // diff calls), so re-throw those. + err.status === 1 && + typeof err.stdout === "string" + ) { + untrackedDiff += err.stdout; + } else { + throw err; + } + } + } + + // Concatenate tracked and untracked diffs. + const combinedDiff = `${trackedDiff}${untrackedDiff}`; + + return { isGitRepo: true, diff: combinedDiff }; } catch { // Either git is not installed or we’re not inside a repository. return { isGitRepo: false, diff: "" }; From e9d16d3c2b4f4f76f189e7e8565e1b2614a78095 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Sun, 27 Apr 2025 17:04:47 -0700 Subject: [PATCH 4/7] fix: check if sandbox-exec is available (#696) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce `isSandboxExecAvailable()` helper and tidy import ordering in `handle-exec-command.ts`. - Add runtime check for the `sandbox-exec` binary on macOS; fall back to `SandboxType.NONE` with a warning if it’s missing, preventing crashes. --------- Signed-off-by: Thibault Sottiaux Co-authored-by: Fouad Matin --- .../src/utils/agent/handle-exec-command.ts | 56 ++++++++++++++++--- codex-cli/src/utils/config.ts | 4 ++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index 85d6869192..b4e4ff0d11 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,17 +1,19 @@ -import type { CommandConfirmation } from "./agent-loop.js"; -import type { AppConfig } from "../config.js"; -import type { ExecInput } from "./sandbox/interface.js"; import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; +import type { AppConfig } from "../config.js"; +import type { CommandConfirmation } from "./agent-loop.js"; +import type { ExecInput } from "./sandbox/interface.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; -import { exec, execApplyPatch } from "./exec.js"; -import { ReviewDecision } from "./review.js"; -import { FullAutoErrorMode } from "../auto-approval-mode.js"; -import { SandboxType } from "./sandbox/interface.js"; import { canAutoApprove } from "../../approvals.js"; import { formatCommandForDisplay } from "../../format-command.js"; +import { FullAutoErrorMode } from "../auto-approval-mode.js"; +import { CODEX_UNSAFE_ALLOW_NO_SANDBOX } from "../config.js"; +import { exec, execApplyPatch } from "./exec.js"; +import { ReviewDecision } from "./review.js"; import { isLoggingEnabled, log } from "../logger/log.js"; +import { SandboxType } from "./sandbox/interface.js"; import { access } from "fs/promises"; +import { execFile } from "node:child_process"; // --------------------------------------------------------------------------- // Session‑level cache of commands that the user has chosen to always approve. @@ -279,10 +281,48 @@ const isInLinux = async (): Promise => { } }; +/** + * Return `true` if the `sandbox-exec` binary can be located. This intentionally does **not** + * spawn the binary – we only care about its presence. + */ +export const isSandboxExecAvailable = (): Promise => + new Promise((res) => + execFile( + "command", + ["-v", "sandbox-exec"], + { signal: AbortSignal.timeout(200) }, + (err) => res(!err), // exit 0 ⇒ found + ), + ); + async function getSandbox(runInSandbox: boolean): Promise { if (runInSandbox) { if (process.platform === "darwin") { - return SandboxType.MACOS_SEATBELT; + // On macOS we rely on the system-provided `sandbox-exec` binary to + // enforce the Seatbelt profile. However, starting with macOS 14 the + // executable may be removed from the default installation or the user + // might be running the CLI on a stripped-down environment (for + // instance, inside certain CI images). Attempting to spawn a missing + // binary makes Node.js throw an *uncaught* `ENOENT` error further down + // the stack which crashes the whole CLI. + + // To provide a graceful degradation path we first check at runtime + // whether `sandbox-exec` can be found **and** is executable. If the + // check fails we fall back to the NONE sandbox while emitting a + // warning so users and maintainers are aware that the additional + // process isolation is not in effect. + + if (await isSandboxExecAvailable()) { + return SandboxType.MACOS_SEATBELT; + } + + if (CODEX_UNSAFE_ALLOW_NO_SANDBOX) { + log( + "WARNING: macOS Seatbelt sandbox requested but 'sandbox-exec' was not found in PATH. " + + "With `CODEX_UNSAFE_ALLOW_NO_SANDBOX` enabled, continuing without sandbox.", + ); + return SandboxType.NONE; + } } else if (await isInLinux()) { return SandboxType.NONE; } else if (process.platform === "win32") { diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts index 1bf0530322..f778764f61 100644 --- a/codex-cli/src/utils/config.ts +++ b/codex-cli/src/utils/config.ts @@ -65,6 +65,10 @@ export let OPENAI_API_KEY = process.env["OPENAI_API_KEY"] || ""; export const OPENAI_ORGANIZATION = process.env["OPENAI_ORGANIZATION"] || ""; export const OPENAI_PROJECT = process.env["OPENAI_PROJECT"] || ""; +export const CODEX_UNSAFE_ALLOW_NO_SANDBOX = Boolean( + process.env["CODEX_UNSAFE_ALLOW_NO_SANDBOX"] || "", +); + export function setApiKey(apiKey: string): void { OPENAI_API_KEY = apiKey; } From 4eda4dd772d0a68cdd8a6be03f8163fdd2ad0142 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sun, 27 Apr 2025 21:47:50 -0700 Subject: [PATCH 5/7] feat: load defaults into Config and introduce ConfigOverrides (#677) This changes how instantiating `Config` works and also adds `approval_policy` and `sandbox_policy` as fields. The idea is: * All fields of `Config` have appropriate default values. * `Config` is initially loaded from `~/.codex/config.toml`, so values in `config.toml` will override those defaults. * Clients must instantiate `Config` via `Config::load_with_overrides(ConfigOverrides)` where `ConfigOverrides` has optional overrides that are expected to be settable based on CLI flags. The `Config` should be defined early in the program and then passed down. Now functions like `init_codex()` take fewer individual parameters because they can just take a `Config`. Also, `Config::load()` used to fail silently if `~/.codex/config.toml` had a parse error and fell back to the default config. This seemed really bad because it wasn't clear why the values in my `config.toml` weren't getting picked up. I changed things so that `load_with_overrides()` returns `Result` and verified that the various CLIs print a reasonable error if `config.toml` is malformed. Finally, I also updated the TUI to show which **sandbox** value is being used, as we do for other key values like **model** and **approval**. This was also a reminder that the various values of `--sandbox` are honored on Linux but not macOS today, so I added some TODOs about fixing that. --- codex-rs/cli/src/main.rs | 8 +- codex-rs/cli/src/seatbelt.rs | 4 +- codex-rs/core/src/approval_mode_cli_arg.rs | 4 +- codex-rs/core/src/codex.rs | 2 - codex-rs/core/src/codex_wrapper.rs | 17 +-- codex-rs/core/src/config.rs | 103 ++++++++++++++---- codex-rs/core/src/exec.rs | 16 ++- codex-rs/core/src/protocol.rs | 10 +- codex-rs/core/tests/live_agent.rs | 7 +- codex-rs/core/tests/previous_response_id.rs | 7 +- codex-rs/core/tests/stream_no_completed.rs | 7 +- codex-rs/exec/Cargo.toml | 2 +- codex-rs/exec/src/cli.rs | 7 ++ codex-rs/exec/src/lib.rs | 26 +++-- codex-rs/interactive/src/cli.rs | 4 +- codex-rs/repl/src/cli.rs | 8 +- codex-rs/repl/src/lib.rs | 21 ++-- codex-rs/tui/src/app.rs | 11 +- codex-rs/tui/src/chatwidget.rs | 44 +++----- codex-rs/tui/src/cli.rs | 8 +- .../tui/src/conversation_history_widget.rs | 12 +- codex-rs/tui/src/history_cell.rs | 9 +- codex-rs/tui/src/lib.rs | 36 ++++-- 23 files changed, 234 insertions(+), 139 deletions(-) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 2eaaa1c8c3..d79f0f333c 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -5,6 +5,7 @@ use std::path::PathBuf; use clap::ArgAction; use clap::Parser; +use codex_core::SandboxModeCliArg; use codex_exec::Cli as ExecCli; use codex_interactive::Cli as InteractiveCli; use codex_repl::Cli as ReplCli; @@ -70,6 +71,10 @@ struct SeatbeltCommand { #[arg(long = "writable-root", short = 'w', value_name = "DIR", action = ArgAction::Append, use_value_delimiter = false)] writable_roots: Vec, + /// Configure the process restrictions for the command. + #[arg(long = "sandbox", short = 's')] + sandbox_policy: SandboxModeCliArg, + /// Full command args to run under seatbelt. #[arg(trailing_var_arg = true)] command: Vec, @@ -101,9 +106,10 @@ async fn main() -> anyhow::Result<()> { Some(Subcommand::Debug(debug_args)) => match debug_args.cmd { DebugCommand::Seatbelt(SeatbeltCommand { command, + sandbox_policy, writable_roots, }) => { - seatbelt::run_seatbelt(command, writable_roots).await?; + seatbelt::run_seatbelt(command, sandbox_policy.into(), writable_roots).await?; } }, } diff --git a/codex-rs/cli/src/seatbelt.rs b/codex-rs/cli/src/seatbelt.rs index c395d96c2b..d328f5524a 100644 --- a/codex-rs/cli/src/seatbelt.rs +++ b/codex-rs/cli/src/seatbelt.rs @@ -1,11 +1,13 @@ use codex_core::exec::create_seatbelt_command; +use codex_core::protocol::SandboxPolicy; use std::path::PathBuf; pub(crate) async fn run_seatbelt( command: Vec, + sandbox_policy: SandboxPolicy, writable_roots: Vec, ) -> anyhow::Result<()> { - let seatbelt_command = create_seatbelt_command(command, &writable_roots); + let seatbelt_command = create_seatbelt_command(command, sandbox_policy, &writable_roots); let status = tokio::process::Command::new(seatbelt_command[0].clone()) .args(&seatbelt_command[1..]) .spawn() diff --git a/codex-rs/core/src/approval_mode_cli_arg.rs b/codex-rs/core/src/approval_mode_cli_arg.rs index eb90b24d87..0da6a89efc 100644 --- a/codex-rs/core/src/approval_mode_cli_arg.rs +++ b/codex-rs/core/src/approval_mode_cli_arg.rs @@ -6,7 +6,7 @@ use clap::ValueEnum; use crate::protocol::AskForApproval; use crate::protocol::SandboxPolicy; -#[derive(Clone, Debug, ValueEnum)] +#[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] pub enum ApprovalModeCliArg { /// Run all commands without asking for user approval. @@ -24,7 +24,7 @@ pub enum ApprovalModeCliArg { Never, } -#[derive(Clone, Debug, ValueEnum)] +#[derive(Clone, Copy, Debug, ValueEnum)] #[value(rename_all = "kebab-case")] pub enum SandboxModeCliArg { /// Network syscalls will be blocked diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index cfeb7e401f..2f80e505c0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -36,7 +36,6 @@ use crate::exec::process_exec_tool_call; use crate::exec::ExecParams; use crate::exec::ExecToolCallOutput; use crate::exec::SandboxType; -use crate::flags::OPENAI_DEFAULT_MODEL; use crate::flags::OPENAI_STREAM_MAX_RETRIES; use crate::models::ContentItem; use crate::models::FunctionCallOutputPayload; @@ -486,7 +485,6 @@ async fn submission_loop( sandbox_policy, disable_response_storage, } => { - let model = model.unwrap_or_else(|| OPENAI_DEFAULT_MODEL.to_string()); info!(model, "Configuring session"); let client = ModelClient::new(model.clone()); diff --git a/codex-rs/core/src/codex_wrapper.rs b/codex-rs/core/src/codex_wrapper.rs index 8d19683ffa..3aeff67615 100644 --- a/codex-rs/core/src/codex_wrapper.rs +++ b/codex-rs/core/src/codex_wrapper.rs @@ -2,16 +2,13 @@ use std::sync::atomic::AtomicU64; use std::sync::Arc; use crate::config::Config; -use crate::protocol::AskForApproval; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::Op; -use crate::protocol::SandboxPolicy; use crate::protocol::Submission; use crate::util::notify_on_sigint; use crate::Codex; use tokio::sync::Notify; -use tracing::debug; /// Spawn a new [`Codex`] and initialise the session. /// @@ -19,21 +16,17 @@ use tracing::debug; /// is received as a response to the initial `ConfigureSession` submission so /// that callers can surface the information to the UI. pub async fn init_codex( - approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + config: Config, disable_response_storage: bool, - model_override: Option, ) -> anyhow::Result<(CodexWrapper, Event, Arc)> { let ctrl_c = notify_on_sigint(); - let config = Config::load().unwrap_or_default(); - debug!("loaded config: {config:?}"); let codex = CodexWrapper::new(Codex::spawn(ctrl_c.clone())?); let init_id = codex .submit(Op::ConfigureSession { - model: model_override.or_else(|| config.model.clone()), - instructions: config.instructions, - approval_policy, - sandbox_policy, + model: config.model.clone(), + instructions: config.instructions.clone(), + approval_policy: config.approval_policy, + sandbox_policy: config.sandbox_policy, disable_response_storage, }) .await?; diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index b5574ceade..d9ad333679 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -1,39 +1,98 @@ -use std::path::PathBuf; - +use crate::flags::OPENAI_DEFAULT_MODEL; +use crate::protocol::AskForApproval; +use crate::protocol::SandboxPolicy; use dirs::home_dir; use serde::Deserialize; +use std::path::PathBuf; -/// Embedded fallback instructions that mirror the TypeScript CLI’s default system prompt. These -/// are compiled into the binary so a clean install behaves correctly even if the user has not -/// created `~/.codex/instructions.md`. +/// Embedded fallback instructions that mirror the TypeScript CLI’s default +/// system prompt. These are compiled into the binary so a clean install behaves +/// correctly even if the user has not created `~/.codex/instructions.md`. const EMBEDDED_INSTRUCTIONS: &str = include_str!("../prompt.md"); -#[derive(Default, Deserialize, Debug, Clone)] +/// Application configuration loaded from disk and merged with overrides. +#[derive(Deserialize, Debug, Clone)] pub struct Config { - pub model: Option, + /// Optional override of model selection. + #[serde(default = "default_model")] + pub model: String, + /// Default approval policy for executing commands. + #[serde(default)] + pub approval_policy: AskForApproval, + #[serde(default)] + pub sandbox_policy: SandboxPolicy, + /// System instructions. pub instructions: Option, } +/// Optional overrides for user configuration (e.g., from CLI flags). +#[derive(Default, Debug, Clone)] +pub struct ConfigOverrides { + pub model: Option, + pub approval_policy: Option, + pub sandbox_policy: Option, +} + impl Config { - /// Load ~/.codex/config.toml and ~/.codex/instructions.md (if present). - /// Returns `None` if neither file exists. - pub fn load() -> Option { - let mut cfg: Config = Self::load_from_toml().unwrap_or_default(); - - // Highest precedence → user‑provided ~/.codex/instructions.md (if present) - // Fallback → embedded default instructions baked into the binary + /// Load configuration, optionally applying overrides (CLI flags). Merges + /// ~/.codex/config.toml, ~/.codex/instructions.md, embedded defaults, and + /// any values provided in `overrides` (highest precedence). + pub fn load_with_overrides(overrides: ConfigOverrides) -> std::io::Result { + let mut cfg: Config = Self::load_from_toml()?; + tracing::warn!("Config parsed from config.toml: {cfg:?}"); + // Instructions: user-provided instructions.md > embedded default. cfg.instructions = Self::load_instructions().or_else(|| Some(EMBEDDED_INSTRUCTIONS.to_string())); - Some(cfg) + // Destructure ConfigOverrides fully to ensure all overrides are applied. + let ConfigOverrides { + model, + approval_policy, + sandbox_policy, + } = overrides; + + if let Some(model) = model { + cfg.model = model; + } + if let Some(approval_policy) = approval_policy { + cfg.approval_policy = approval_policy; + } + if let Some(sandbox_policy) = sandbox_policy { + cfg.sandbox_policy = sandbox_policy; + } + Ok(cfg) } - fn load_from_toml() -> Option { - let mut p = codex_dir().ok()?; - p.push("config.toml"); - let contents = std::fs::read_to_string(&p).ok()?; - toml::from_str(&contents).ok() + /// Attempt to parse the file at `~/.codex/config.toml` into a Config. + fn load_from_toml() -> std::io::Result { + let config_toml_path = codex_dir()?.join("config.toml"); + match std::fs::read_to_string(&config_toml_path) { + Ok(contents) => toml::from_str::(&contents).map_err(|e| { + tracing::error!("Failed to parse config.toml: {e}"); + std::io::Error::new(std::io::ErrorKind::InvalidData, e) + }), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + tracing::info!("config.toml not found, using defaults"); + Ok(Self::load_default_config()) + } + Err(e) => { + tracing::error!("Failed to read config.toml: {e}"); + Err(e) + } + } + } + + /// Meant to be used exclusively for tests: load_with_overrides() should be + /// used in all other cases. + pub fn load_default_config_for_test() -> Self { + Self::load_default_config() + } + + fn load_default_config() -> Self { + // Load from an empty string to exercise #[serde(default)] to + // get the default values for each field. + toml::from_str::("").expect("empty string should parse as TOML") } fn load_instructions() -> Option { @@ -43,6 +102,10 @@ impl Config { } } +fn default_model() -> String { + OPENAI_DEFAULT_MODEL.to_string() +} + /// Returns the path to the Codex configuration directory, which is `~/.codex`. /// Does not verify that the directory exists. pub fn codex_dir() -> std::io::Result { diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index aa414e62d4..4ce07acf78 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -98,7 +98,7 @@ pub async fn process_exec_tool_call( workdir, timeout_ms, } = params; - let seatbelt_command = create_seatbelt_command(command, writable_roots); + let seatbelt_command = create_seatbelt_command(command, sandbox_policy, writable_roots); exec( ExecParams { command: seatbelt_command, @@ -154,7 +154,11 @@ pub async fn process_exec_tool_call( } } -pub fn create_seatbelt_command(command: Vec, writable_roots: &[PathBuf]) -> Vec { +pub fn create_seatbelt_command( + command: Vec, + sandbox_policy: SandboxPolicy, + writable_roots: &[PathBuf], +) -> Vec { let (policies, cli_args): (Vec, Vec) = writable_roots .iter() .enumerate() @@ -166,6 +170,14 @@ pub fn create_seatbelt_command(command: Vec, writable_roots: &[PathBuf]) }) .unzip(); + // TODO(ragona): The seatbelt policy should reflect the SandboxPolicy that + // is passed, but everything is currently hardcoded to use + // MACOS_SEATBELT_READONLY_POLICY. + // TODO(mbolin): apply_patch calls must also honor the SandboxPolicy. + if !matches!(sandbox_policy, SandboxPolicy::NetworkRestricted) { + tracing::error!("specified sandbox policy {sandbox_policy:?} will not be honroed"); + } + let full_policy = if policies.is_empty() { MACOS_SEATBELT_READONLY_POLICY.to_string() } else { diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 96c4ea4832..139e2f2fc2 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -26,7 +26,7 @@ pub enum Op { /// Configure the model session. ConfigureSession { /// If not specified, server will use its default model. - model: Option, + model: String, /// Model instructions instructions: Option, /// When to escalate for approval for execution @@ -66,11 +66,13 @@ pub enum Op { } /// Determines how liberally commands are auto‑approved by the system. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] pub enum AskForApproval { /// Under this policy, only “known safe” commands—as determined by /// `is_safe_command()`—that **only read files** are auto‑approved. /// Everything else will ask the user to approve. + #[default] UnlessAllowListed, /// In addition to everything allowed by **`Suggest`**, commands that @@ -91,13 +93,15 @@ pub enum AskForApproval { } /// Determines execution restrictions for model shell commands -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] pub enum SandboxPolicy { /// Network syscalls will be blocked NetworkRestricted, /// Filesystem writes will be restricted FileWriteRestricted, /// Network and filesystem writes will be restricted + #[default] NetworkAndFileWriteRestricted, /// No restrictions; full "unsandboxed" mode DangerousNoRestrictions, diff --git a/codex-rs/core/tests/live_agent.rs b/codex-rs/core/tests/live_agent.rs index 823cd73a01..2387649873 100644 --- a/codex-rs/core/tests/live_agent.rs +++ b/codex-rs/core/tests/live_agent.rs @@ -17,7 +17,7 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; @@ -47,13 +47,14 @@ async fn spawn_codex() -> Codex { let agent = Codex::spawn(std::sync::Arc::new(Notify::new())).unwrap(); + let config = Config::load_default_config_for_test(); agent .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index de1309e856..24c8691630 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -1,6 +1,6 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -87,13 +87,14 @@ async fn keeps_previous_response_id_between_tasks() { let codex = Codex::spawn(std::sync::Arc::new(tokio::sync::Notify::new())).unwrap(); // Init session + let config = Config::load_default_config_for_test(); codex .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/core/tests/stream_no_completed.rs b/codex-rs/core/tests/stream_no_completed.rs index c732a5fdbb..e696ea97ae 100644 --- a/codex-rs/core/tests/stream_no_completed.rs +++ b/codex-rs/core/tests/stream_no_completed.rs @@ -3,7 +3,7 @@ use std::time::Duration; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; use codex_core::protocol::InputItem; use codex_core::protocol::Op; use codex_core::protocol::SandboxPolicy; @@ -70,13 +70,14 @@ async fn retries_on_early_close() { let codex = Codex::spawn(std::sync::Arc::new(tokio::sync::Notify::new())).unwrap(); + let config = Config::load_default_config_for_test(); codex .submit(Submission { id: "init".into(), op: Op::ConfigureSession { - model: None, + model: config.model, instructions: None, - approval_policy: AskForApproval::OnFailure, + approval_policy: config.approval_policy, sandbox_policy: SandboxPolicy::NetworkAndFileWriteRestricted, disable_response_storage: false, }, diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index f214f90042..491dd4c12f 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -14,7 +14,7 @@ path = "src/lib.rs" [dependencies] anyhow = "1" clap = { version = "4", features = ["derive"] } -codex-core = { path = "../core" } +codex-core = { path = "../core", features = ["cli"] } tokio = { version = "1", features = [ "io-std", "macros", diff --git a/codex-rs/exec/src/cli.rs b/codex-rs/exec/src/cli.rs index 299e85879d..1613845a89 100644 --- a/codex-rs/exec/src/cli.rs +++ b/codex-rs/exec/src/cli.rs @@ -1,4 +1,5 @@ use clap::Parser; +use codex_core::SandboxModeCliArg; use std::path::PathBuf; #[derive(Parser, Debug)] @@ -12,6 +13,12 @@ pub struct Cli { #[arg(long, short = 'm')] pub model: Option, + /// Configure the process restrictions when a command is executed. + /// + /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. + #[arg(long = "sandbox", short = 's')] + pub sandbox_policy: Option, + /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] pub skip_git_repo_check: bool, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index ab7d735e0f..daa07e4629 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -3,13 +3,14 @@ use std::sync::Arc; pub use cli::Cli; use codex_core::codex_wrapper; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol::AskForApproval; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::FileChange; use codex_core::protocol::InputItem; use codex_core::protocol::Op; -use codex_core::protocol::SandboxPolicy; use codex_core::util::is_inside_git_repo; use tracing::debug; use tracing::error; @@ -33,6 +34,7 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { let Cli { images, model, + sandbox_policy, skip_git_repo_check, disable_response_storage, prompt, @@ -47,17 +49,17 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { std::process::exit(1); } - // TODO(mbolin): We are reworking the CLI args right now, so this will - // likely come from a new --execution-policy arg. - let approval_policy = AskForApproval::Never; - let sandbox_policy = SandboxPolicy::NetworkAndFileWriteRestricted; - let (codex_wrapper, event, ctrl_c) = codex_wrapper::init_codex( - approval_policy, - sandbox_policy, - disable_response_storage, - model, - ) - .await?; + // Load configuration and determine approval policy + let overrides = ConfigOverrides { + model: model.clone(), + // This CLI is intended to be headless and has no affordances for asking + // the user for approval. + approval_policy: Some(AskForApproval::Never), + sandbox_policy: sandbox_policy.map(Into::into), + }; + let config = Config::load_with_overrides(overrides)?; + let (codex_wrapper, event, ctrl_c) = + codex_wrapper::init_codex(config, disable_response_storage).await?; let codex = Arc::new(codex_wrapper); info!("Codex initialized with event: {event:?}"); diff --git a/codex-rs/interactive/src/cli.rs b/codex-rs/interactive/src/cli.rs index ffb61dfc2e..6d35a49ac6 100644 --- a/codex-rs/interactive/src/cli.rs +++ b/codex-rs/interactive/src/cli.rs @@ -21,8 +21,8 @@ pub struct Cli { /// Configure the process restrictions when a command is executed. /// /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's', value_enum, default_value_t = SandboxModeCliArg::NetworkAndFileWriteRestricted)] - pub sandbox_policy: SandboxModeCliArg, + #[arg(long = "sandbox", short = 's')] + pub sandbox_policy: Option, /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] diff --git a/codex-rs/repl/src/cli.rs b/codex-rs/repl/src/cli.rs index ec6c652519..a6b5bb73d9 100644 --- a/codex-rs/repl/src/cli.rs +++ b/codex-rs/repl/src/cli.rs @@ -34,14 +34,14 @@ pub struct Cli { pub no_ansi: bool, /// Configure when the model requires human approval before executing a command. - #[arg(long = "ask-for-approval", short = 'a', value_enum, default_value_t = ApprovalModeCliArg::OnFailure)] - pub approval_policy: ApprovalModeCliArg, + #[arg(long = "ask-for-approval", short = 'a')] + pub approval_policy: Option, /// Configure the process restrictions when a command is executed. /// /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's', value_enum, default_value_t = SandboxModeCliArg::NetworkAndFileWriteRestricted)] - pub sandbox_policy: SandboxModeCliArg, + #[arg(long = "sandbox", short = 's')] + pub sandbox_policy: Option, /// Allow running Codex outside a Git repository. By default the CLI /// aborts early when the current working directory is **not** inside a diff --git a/codex-rs/repl/src/lib.rs b/codex-rs/repl/src/lib.rs index 0f9c47e49b..74e54181c3 100644 --- a/codex-rs/repl/src/lib.rs +++ b/codex-rs/repl/src/lib.rs @@ -4,6 +4,7 @@ use std::io::Write; use std::sync::Arc; use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::protocol; use codex_core::protocol::FileChange; use codex_core::util::is_inside_git_repo; @@ -75,12 +76,18 @@ pub async fn run_main(cli: Cli) -> anyhow::Result<()> { // Initialize logging before any other work so early errors are captured. init_logger(cli.verbose, !cli.no_ansi); - let config = Config::load().unwrap_or_default(); + // Load config file and apply CLI overrides (model & approval policy) + let overrides = ConfigOverrides { + model: cli.model.clone(), + approval_policy: cli.approval_policy.map(Into::into), + sandbox_policy: cli.sandbox_policy.map(Into::into), + }; + let config = Config::load_with_overrides(overrides)?; codex_main(cli, config, ctrl_c).await } -async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::Result<()> { +async fn codex_main(cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::Result<()> { let mut builder = Codex::builder(); if let Some(path) = cli.record_submissions { builder = builder.record_submissions(path); @@ -93,10 +100,10 @@ async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::R let init = protocol::Submission { id: init_id.clone(), op: protocol::Op::ConfigureSession { - model: cli.model.or(cfg.model), + model: cfg.model, instructions: cfg.instructions, - approval_policy: cli.approval_policy.into(), - sandbox_policy: cli.sandbox_policy.into(), + approval_policy: cfg.approval_policy, + sandbox_policy: cfg.sandbox_policy, disable_response_storage: cli.disable_response_storage, }, }; @@ -133,8 +140,8 @@ async fn codex_main(mut cli: Cli, cfg: Config, ctrl_c: Arc) -> anyhow::R // run loop let mut reader = InputReader::new(ctrl_c.clone()); loop { - let text = match cli.prompt.take() { - Some(input) => input, + let text = match &cli.prompt { + Some(input) => input.clone(), None => match reader.request_input().await? { Some(input) => input, None => { diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 8f27ce6eb2..c5da0b56bc 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -4,10 +4,9 @@ use crate::git_warning_screen::GitWarningOutcome; use crate::git_warning_screen::GitWarningScreen; use crate::scroll_event_helper::ScrollEventHelper; use crate::tui; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; use codex_core::protocol::Event; use codex_core::protocol::Op; -use codex_core::protocol::SandboxPolicy; use color_eyre::eyre::Result; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -34,12 +33,10 @@ pub(crate) struct App<'a> { impl App<'_> { pub(crate) fn new( - approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + config: Config, initial_prompt: Option, show_git_warning: bool, initial_images: Vec, - model: Option, disable_response_storage: bool, ) -> Self { let (app_event_tx, app_event_rx) = channel(); @@ -80,12 +77,10 @@ impl App<'_> { } let chat_widget = ChatWidget::new( - approval_policy, - sandbox_policy, + config, app_event_tx.clone(), initial_prompt.clone(), initial_images, - model, disable_response_storage, ); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index b852638cc2..e2224f99be 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3,12 +3,11 @@ use std::sync::mpsc::Sender; use std::sync::Arc; use codex_core::codex_wrapper::init_codex; -use codex_core::protocol::AskForApproval; +use codex_core::config::Config; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::InputItem; use codex_core::protocol::Op; -use codex_core::protocol::SandboxPolicy; use crossterm::event::KeyEvent; use ratatui::buffer::Buffer; use ratatui::layout::Constraint; @@ -34,7 +33,7 @@ pub(crate) struct ChatWidget<'a> { conversation_history: ConversationHistoryWidget, bottom_pane: BottomPane<'a>, input_focus: InputFocus, - approval_policy: AskForApproval, + config: Config, cwd: std::path::PathBuf, } @@ -46,12 +45,10 @@ enum InputFocus { impl ChatWidget<'_> { pub(crate) fn new( - approval_policy: AskForApproval, - sandbox_policy: SandboxPolicy, + config: Config, app_event_tx: Sender, initial_prompt: Option, initial_images: Vec, - model: Option, disable_response_storage: bool, ) -> Self { let (codex_op_tx, mut codex_op_rx) = unbounded_channel::(); @@ -63,23 +60,17 @@ impl ChatWidget<'_> { let app_event_tx_clone = app_event_tx.clone(); // Create the Codex asynchronously so the UI loads as quickly as possible. + let config_for_agent_loop = config.clone(); tokio::spawn(async move { - // Initialize session; storage enabled by default - let (codex, session_event, _ctrl_c) = match init_codex( - approval_policy, - sandbox_policy, - disable_response_storage, - model, - ) - .await - { - Ok(vals) => vals, - Err(e) => { - // TODO(mbolin): This error needs to be surfaced to the user. - tracing::error!("failed to initialize codex: {e}"); - return; - } - }; + let (codex, session_event, _ctrl_c) = + match init_codex(config_for_agent_loop, disable_response_storage).await { + Ok(vals) => vals, + Err(e) => { + // TODO: surface this error to the user. + tracing::error!("failed to initialize codex: {e}"); + return; + } + }; // Forward the captured `SessionInitialized` event that was consumed // inside `init_codex()` so it can be rendered in the UI. @@ -115,7 +106,7 @@ impl ChatWidget<'_> { has_input_focus: true, }), input_focus: InputFocus::BottomPane, - approval_policy, + config, cwd: cwd.clone(), }; @@ -243,11 +234,8 @@ impl ChatWidget<'_> { match msg { EventMsg::SessionConfigured { model } => { // Record session information at the top of the conversation. - self.conversation_history.add_session_info( - model, - self.cwd.clone(), - self.approval_policy, - ); + self.conversation_history + .add_session_info(&self.config, model, self.cwd.clone()); self.request_redraw()?; } EventMsg::AgentMessage { message } => { diff --git a/codex-rs/tui/src/cli.rs b/codex-rs/tui/src/cli.rs index db25ad2b3c..f336b0c34c 100644 --- a/codex-rs/tui/src/cli.rs +++ b/codex-rs/tui/src/cli.rs @@ -18,14 +18,14 @@ pub struct Cli { pub model: Option, /// Configure when the model requires human approval before executing a command. - #[arg(long = "ask-for-approval", short = 'a', value_enum, default_value_t = ApprovalModeCliArg::OnFailure)] - pub approval_policy: ApprovalModeCliArg, + #[arg(long = "ask-for-approval", short = 'a')] + pub approval_policy: Option, /// Configure the process restrictions when a command is executed. /// /// Uses OS-specific sandboxing tools; Seatbelt on OSX, landlock+seccomp on Linux. - #[arg(long = "sandbox", short = 's', value_enum, default_value_t = SandboxModeCliArg::NetworkAndFileWriteRestricted)] - pub sandbox_policy: SandboxModeCliArg, + #[arg(long = "sandbox", short = 's')] + pub sandbox_policy: Option, /// Allow running Codex outside a Git repository. #[arg(long = "skip-git-repo-check", default_value_t = false)] diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index 27b5e9b3cf..de1dbba963 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -1,6 +1,7 @@ use crate::history_cell::CommandOutput; use crate::history_cell::HistoryCell; use crate::history_cell::PatchEventType; +use codex_core::config::Config; use codex_core::protocol::FileChange; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -181,13 +182,10 @@ impl ConversationHistoryWidget { self.add_to_history(HistoryCell::new_patch_event(event_type, changes)); } - pub fn add_session_info( - &mut self, - model: String, - cwd: std::path::PathBuf, - approval_policy: codex_core::protocol::AskForApproval, - ) { - self.add_to_history(HistoryCell::new_session_info(model, cwd, approval_policy)); + /// Note `model` could differ from `config.model` if the agent decided to + /// use a different model than the one requested by the user. + pub fn add_session_info(&mut self, config: &Config, model: String, cwd: PathBuf) { + self.add_to_history(HistoryCell::new_session_info(config, model, cwd)); } pub fn add_active_exec_command(&mut self, call_id: String, command: Vec) { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index d6ebc248c4..f9bb18179c 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -1,4 +1,5 @@ use codex_ansi_escape::ansi_escape_line; +use codex_core::config::Config; use codex_core::protocol::FileChange; use ratatui::prelude::*; use ratatui::style::Color; @@ -144,9 +145,9 @@ impl HistoryCell { } pub(crate) fn new_session_info( + config: &Config, model: String, cwd: std::path::PathBuf, - approval_policy: codex_core::protocol::AskForApproval, ) -> Self { let mut lines: Vec> = Vec::new(); @@ -158,7 +159,11 @@ impl HistoryCell { ])); lines.push(Line::from(vec![ "↳ approval: ".bold(), - format!("{:?}", approval_policy).into(), + format!("{:?}", config.approval_policy).into(), + ])); + lines.push(Line::from(vec![ + "↳ sandbox: ".bold(), + format!("{:?}", config.sandbox_policy).into(), ])); lines.push(Line::from("")); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index d0f5f664a6..8e987ad743 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -4,6 +4,8 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] use app::App; +use codex_core::config::Config; +use codex_core::config::ConfigOverrides; use codex_core::util::is_inside_git_repo; use log_layer::TuiLogLayer; use std::fs::OpenOptions; @@ -31,6 +33,23 @@ pub use cli::Cli; pub fn run_main(cli: Cli) -> std::io::Result<()> { assert_env_var_set(); + let config = { + // Load configuration and support CLI overrides. + let overrides = ConfigOverrides { + model: cli.model.clone(), + approval_policy: cli.approval_policy.map(Into::into), + sandbox_policy: cli.sandbox_policy.map(Into::into), + }; + #[allow(clippy::print_stderr)] + match Config::load_with_overrides(overrides) { + Ok(config) => config, + Err(err) => { + eprintln!("Error loading configuration: {err}"); + std::process::exit(1); + } + } + }; + let log_dir = codex_core::config::log_dir()?; std::fs::create_dir_all(&log_dir)?; // Open (or create) your log file, appending to it. @@ -79,7 +98,7 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> { // `--allow-no-git-exec` flag. let show_git_warning = !cli.skip_git_repo_check && !is_inside_git_repo(); - try_run_ratatui_app(cli, show_git_warning, log_rx); + try_run_ratatui_app(cli, config, show_git_warning, log_rx); Ok(()) } @@ -89,16 +108,18 @@ pub fn run_main(cli: Cli) -> std::io::Result<()> { )] fn try_run_ratatui_app( cli: Cli, + config: Config, show_git_warning: bool, log_rx: tokio::sync::mpsc::UnboundedReceiver, ) { - if let Err(report) = run_ratatui_app(cli, show_git_warning, log_rx) { + if let Err(report) = run_ratatui_app(cli, config, show_git_warning, log_rx) { eprintln!("Error: {report:?}"); } } fn run_ratatui_app( cli: Cli, + config: Config, show_git_warning: bool, mut log_rx: tokio::sync::mpsc::UnboundedReceiver, ) -> color_eyre::Result<()> { @@ -116,23 +137,14 @@ fn run_ratatui_app( let Cli { prompt, images, - approval_policy, - sandbox_policy: sandbox, - model, disable_response_storage, .. } = cli; - - let approval_policy = approval_policy.into(); - let sandbox_policy = sandbox.into(); - let mut app = App::new( - approval_policy, - sandbox_policy, + config, prompt, show_git_warning, images, - model, disable_response_storage, ); From fa5fa8effc5c21340ddefbb892d6086d2978017a Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 28 Apr 2025 07:48:38 -0700 Subject: [PATCH 6/7] fix: only allow running without sandbox if explicitly marked in safe container (#699) Signed-off-by: Thibault Sottiaux --- codex-cli/Dockerfile | 4 ++ codex-cli/src/cli.tsx | 4 ++ .../src/utils/agent/handle-exec-command.ts | 45 +++++-------------- codex-cli/src/utils/config.ts | 2 + 4 files changed, 20 insertions(+), 35 deletions(-) diff --git a/codex-cli/Dockerfile b/codex-cli/Dockerfile index 4ed3089bbb..ab41c5eaf9 100644 --- a/codex-cli/Dockerfile +++ b/codex-cli/Dockerfile @@ -46,6 +46,10 @@ RUN npm install -g codex.tgz \ && rm -rf /usr/local/share/npm-global/lib/node_modules/codex-cli/tests \ && rm -rf /usr/local/share/npm-global/lib/node_modules/codex-cli/docs +# Inside the container we consider the environment already sufficiently locked +# down, therefore instruct Codex CLI to allow running without sandboxing. +ENV CODEX_UNSAFE_ALLOW_NO_SANDBOX=1 + # Copy and set up firewall script as root. USER root COPY scripts/init_firewall.sh /usr/local/bin/ diff --git a/codex-cli/src/cli.tsx b/codex-cli/src/cli.tsx index 14b6b0fa68..4ddd7d4914 100644 --- a/codex-cli/src/cli.tsx +++ b/codex-cli/src/cli.tsx @@ -184,6 +184,10 @@ const cli = meow( }, ); +// --------------------------------------------------------------------------- +// Global flag handling +// --------------------------------------------------------------------------- + // Handle 'completion' subcommand before any prompting or API calls if (cli.input[0] === "completion") { const shell = cli.input[1] || "bash"; diff --git a/codex-cli/src/utils/agent/handle-exec-command.ts b/codex-cli/src/utils/agent/handle-exec-command.ts index b4e4ff0d11..6cb48016ad 100644 --- a/codex-cli/src/utils/agent/handle-exec-command.ts +++ b/codex-cli/src/utils/agent/handle-exec-command.ts @@ -1,13 +1,12 @@ -import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; -import type { AppConfig } from "../config.js"; import type { CommandConfirmation } from "./agent-loop.js"; +import type { ApplyPatchCommand, ApprovalPolicy } from "../../approvals.js"; import type { ExecInput } from "./sandbox/interface.js"; import type { ResponseInputItem } from "openai/resources/responses/responses.mjs"; import { canAutoApprove } from "../../approvals.js"; import { formatCommandForDisplay } from "../../format-command.js"; import { FullAutoErrorMode } from "../auto-approval-mode.js"; -import { CODEX_UNSAFE_ALLOW_NO_SANDBOX } from "../config.js"; +import { CODEX_UNSAFE_ALLOW_NO_SANDBOX, type AppConfig } from "../config.js"; import { exec, execApplyPatch } from "./exec.js"; import { ReviewDecision } from "./review.js"; import { isLoggingEnabled, log } from "../logger/log.js"; @@ -272,15 +271,6 @@ async function execCommand( }; } -const isInLinux = async (): Promise => { - try { - await access("/proc/1/cgroup"); - return true; - } catch { - return false; - } -}; - /** * Return `true` if the `sandbox-exec` binary can be located. This intentionally does **not** * spawn the binary – we only care about its presence. @@ -305,35 +295,20 @@ async function getSandbox(runInSandbox: boolean): Promise { // instance, inside certain CI images). Attempting to spawn a missing // binary makes Node.js throw an *uncaught* `ENOENT` error further down // the stack which crashes the whole CLI. - - // To provide a graceful degradation path we first check at runtime - // whether `sandbox-exec` can be found **and** is executable. If the - // check fails we fall back to the NONE sandbox while emitting a - // warning so users and maintainers are aware that the additional - // process isolation is not in effect. - if (await isSandboxExecAvailable()) { return SandboxType.MACOS_SEATBELT; - } - - if (CODEX_UNSAFE_ALLOW_NO_SANDBOX) { - log( - "WARNING: macOS Seatbelt sandbox requested but 'sandbox-exec' was not found in PATH. " + - "With `CODEX_UNSAFE_ALLOW_NO_SANDBOX` enabled, continuing without sandbox.", + } else { + throw new Error( + "Sandbox was mandated, but 'sandbox-exec' was not found in PATH!", ); - return SandboxType.NONE; } - } else if (await isInLinux()) { - return SandboxType.NONE; - } else if (process.platform === "win32") { - // On Windows, we don't have a sandbox implementation yet, so we fall back to NONE - // instead of throwing an error, which would crash the application - log( - "WARNING: Sandbox was requested but is not available on Windows. Continuing without sandbox.", - ); + } else if (CODEX_UNSAFE_ALLOW_NO_SANDBOX) { + // Allow running without a sandbox if the user has explicitly marked the + // environment as already being sufficiently locked-down. return SandboxType.NONE; } - // For other platforms, still throw an error as before + + // For all else, we hard fail if the user has requested a sandbox and none is available. throw new Error("Sandbox was mandated, but no sandbox is available!"); } else { return SandboxType.NONE; diff --git a/codex-cli/src/utils/config.ts b/codex-cli/src/utils/config.ts index f778764f61..6d7d682bb4 100644 --- a/codex-cli/src/utils/config.ts +++ b/codex-cli/src/utils/config.ts @@ -65,6 +65,8 @@ export let OPENAI_API_KEY = process.env["OPENAI_API_KEY"] || ""; export const OPENAI_ORGANIZATION = process.env["OPENAI_ORGANIZATION"] || ""; export const OPENAI_PROJECT = process.env["OPENAI_PROJECT"] || ""; +// Can be set `true` when Codex is running in an environment that is marked as already +// considered sufficiently locked-down so that we allow running wihtout an explicit sandbox. export const CODEX_UNSAFE_ALLOW_NO_SANDBOX = Boolean( process.env["CODEX_UNSAFE_ALLOW_NO_SANDBOX"] || "", ); From 07c1f65e654411136a4e171297d0c6905d2938db Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 28 Apr 2025 09:21:49 -0700 Subject: [PATCH 7/7] fix: drop d as keyboard shortcut for scrolling in the TUI --- codex-rs/tui/src/conversation_history_widget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index de1dbba963..5d374794e9 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -238,7 +238,7 @@ impl WidgetRef for ConversationHistoryWidget { fn render_ref(&self, area: Rect, buf: &mut Buffer) { let (title, border_style) = if self.has_input_focus { ( - "Messages (↑/↓ or j/k = line, b/u = PgUp, space/d = PgDn)", + "Messages (↑/↓ or j/k = line, b/space = page)", Style::default().fg(Color::LightYellow), ) } else {