From 531ce7626ff76414f8c5ed0eff7e18137921b5fc Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Jun 2025 17:48:51 -0700 Subject: [PATCH 1/2] fix: pretty-print the sandbox config in the TUI/exec modes (#1376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that https://github.com/openai/codex/pull/1373 simplified the sandbox config, we can print something much simpler in the TUI (and in `codex exec`) to summarize the sandbox config. Before: ![Screenshot 2025-06-24 at 5 45 52 PM](https://github.com/user-attachments/assets/b7633efb-a619-43e1-9abe-7bb0be2d0ec0) With this change: ![Screenshot 2025-06-24 at 5 46 44 PM](https://github.com/user-attachments/assets/8d099bdd-a429-4796-a08d-70931d984e4f) For reference, my `config.toml` contains: ``` [sandbox] mode = "workspace-write" writable_roots = ["/tmp", "/Users/mbolin/.pyenv/shims"] ``` Fixes https://github.com/openai/codex/issues/1248 --- codex-rs/common/Cargo.toml | 1 + codex-rs/common/src/lib.rs | 5 +++++ codex-rs/common/src/sandbox_summary.rs | 28 ++++++++++++++++++++++++++ codex-rs/core/src/protocol.rs | 22 +++++++++++--------- codex-rs/exec/Cargo.toml | 6 +++++- codex-rs/exec/src/event_processor.rs | 3 ++- codex-rs/tui/Cargo.toml | 6 +++++- codex-rs/tui/src/history_cell.rs | 3 ++- 8 files changed, 60 insertions(+), 14 deletions(-) create mode 100644 codex-rs/common/src/sandbox_summary.rs diff --git a/codex-rs/common/Cargo.toml b/codex-rs/common/Cargo.toml index b4b658dabf..eff7a6c0b4 100644 --- a/codex-rs/common/Cargo.toml +++ b/codex-rs/common/Cargo.toml @@ -16,3 +16,4 @@ serde = { version = "1", optional = true } # Separate feature so that `clap` is not a mandatory dependency. cli = ["clap", "toml", "serde"] elapsed = [] +sandbox_summary = [] diff --git a/codex-rs/common/src/lib.rs b/codex-rs/common/src/lib.rs index 074f648fe6..18ed49e5a7 100644 --- a/codex-rs/common/src/lib.rs +++ b/codex-rs/common/src/lib.rs @@ -12,3 +12,8 @@ mod config_override; #[cfg(feature = "cli")] pub use config_override::CliConfigOverrides; + +mod sandbox_summary; + +#[cfg(feature = "sandbox_summary")] +pub use sandbox_summary::summarize_sandbox_policy; diff --git a/codex-rs/common/src/sandbox_summary.rs b/codex-rs/common/src/sandbox_summary.rs new file mode 100644 index 0000000000..3d33d92836 --- /dev/null +++ b/codex-rs/common/src/sandbox_summary.rs @@ -0,0 +1,28 @@ +use codex_core::protocol::SandboxPolicy; + +pub fn summarize_sandbox_policy(sandbox_policy: &SandboxPolicy) -> String { + match sandbox_policy { + SandboxPolicy::DangerFullAccess => "danger-full-access".to_string(), + SandboxPolicy::ReadOnly => "read-only".to_string(), + SandboxPolicy::WorkspaceWrite { + writable_roots, + network_access, + } => { + let mut summary = "workspace-write".to_string(); + if !writable_roots.is_empty() { + summary.push_str(&format!( + " [{}]", + writable_roots + .iter() + .map(|p| p.to_string_lossy()) + .collect::>() + .join(", ") + )); + } + if *network_access { + summary.push_str(" (network access enabled)"); + } + summary + } + } +} diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index f3250de4fb..42cf92996f 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -183,17 +183,8 @@ impl SandboxPolicy { /// the current working directory and the per-user tmp dir on macOS. It does /// not allow network access. pub fn new_workspace_write_policy() -> Self { - let mut writable_roots = vec![]; - - // Also include the per-user tmp dir on macOS. - if cfg!(target_os = "macos") { - if let Some(tmpdir) = std::env::var_os("TMPDIR") { - writable_roots.push(PathBuf::from(tmpdir)); - } - } - SandboxPolicy::WorkspaceWrite { - writable_roots, + writable_roots: vec![], network_access: false, } } @@ -229,6 +220,17 @@ impl SandboxPolicy { SandboxPolicy::WorkspaceWrite { writable_roots, .. } => { let mut roots = writable_roots.clone(); roots.push(cwd.to_path_buf()); + + // Also include the per-user tmp dir on macOS. + // Note this is added dynamically rather than storing it in + // writable_roots because writable_roots contains only static + // values deserialized from the config file. + if cfg!(target_os = "macos") { + if let Some(tmpdir) = std::env::var_os("TMPDIR") { + roots.push(PathBuf::from(tmpdir)); + } + } + roots } } diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index c3bde69719..8c0c3737a2 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -19,7 +19,11 @@ anyhow = "1" chrono = "0.4.40" clap = { version = "4", features = ["derive"] } codex-core = { path = "../core" } -codex-common = { path = "../common", features = ["cli", "elapsed"] } +codex-common = { path = "../common", features = [ + "cli", + "elapsed", + "sandbox_summary", +] } codex-linux-sandbox = { path = "../linux-sandbox" } mcp-types = { path = "../mcp-types" } owo-colors = "4.2.0" diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index 4cbbd25f0b..e2a8bbb20a 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -1,4 +1,5 @@ use codex_common::elapsed::format_elapsed; +use codex_common::summarize_sandbox_policy; use codex_core::WireApi; use codex_core::config::Config; use codex_core::model_supports_reasoning_summaries; @@ -134,7 +135,7 @@ impl EventProcessor { ("model", config.model.clone()), ("provider", config.model_provider_id.clone()), ("approval", format!("{:?}", config.approval_policy)), - ("sandbox", format!("{:?}", config.sandbox_policy)), + ("sandbox", summarize_sandbox_policy(&config.sandbox_policy)), ]; if config.model_provider.wire_api == WireApi::Responses && model_supports_reasoning_summaries(&config.model) diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 2d7840e661..0891517d0e 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -20,7 +20,11 @@ base64 = "0.22.1" clap = { version = "4", features = ["derive"] } codex-ansi-escape = { path = "../ansi-escape" } codex-core = { path = "../core" } -codex-common = { path = "../common", features = ["cli", "elapsed"] } +codex-common = { path = "../common", features = [ + "cli", + "elapsed", + "sandbox_summary", +] } codex-linux-sandbox = { path = "../linux-sandbox" } codex-login = { path = "../login" } color-eyre = "0.6.3" diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 481576b5b3..e2a54283c1 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -6,6 +6,7 @@ use crate::text_formatting::format_and_truncate_tool_result; use base64::Engine; use codex_ansi_escape::ansi_escape_line; use codex_common::elapsed::format_duration; +use codex_common::summarize_sandbox_policy; use codex_core::WireApi; use codex_core::config::Config; use codex_core::model_supports_reasoning_summaries; @@ -152,7 +153,7 @@ impl HistoryCell { ("model", config.model.clone()), ("provider", config.model_provider_id.clone()), ("approval", format!("{:?}", config.approval_policy)), - ("sandbox", format!("{:?}", config.sandbox_policy)), + ("sandbox", summarize_sandbox_policy(&config.sandbox_policy)), ]; if config.model_provider.wire_api == WireApi::Responses && model_supports_reasoning_summaries(&config.model) From 17fad32de2354a7bdb687873e6be4a62843f9fa6 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 24 Jun 2025 22:04:59 -0700 Subject: [PATCH 2/2] chore: rename unless-allow-listed to untrusted --- codex-rs/common/src/approval_mode_cli_arg.rs | 10 +++++----- codex-rs/config.md | 9 +++++++-- codex-rs/core/src/protocol.rs | 12 ++++-------- codex-rs/core/src/safety.rs | 2 +- codex-rs/mcp-server/src/codex_tool_config.rs | 2 -- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/codex-rs/common/src/approval_mode_cli_arg.rs b/codex-rs/common/src/approval_mode_cli_arg.rs index 94bd8e8927..91049ec032 100644 --- a/codex-rs/common/src/approval_mode_cli_arg.rs +++ b/codex-rs/common/src/approval_mode_cli_arg.rs @@ -13,10 +13,10 @@ pub enum ApprovalModeCliArg { /// will escalate to the user to ask for un-sandboxed execution. OnFailure, - /// Only run "known safe" commands (e.g. ls, cat, sed) without - /// asking for user approval. Will escalate to the user if the model - /// proposes a command that is not allow-listed. - UnlessAllowListed, + /// Only run "trusted" commands (e.g. ls, cat, sed) without asking for user + /// approval. Will escalate to the user if the model proposes a command that + /// is not in the "trusted" set. + Untrusted, /// Never ask for user approval /// Execution failures are immediately returned to the model. @@ -27,7 +27,7 @@ impl From for AskForApproval { fn from(value: ApprovalModeCliArg) -> Self { match value { ApprovalModeCliArg::OnFailure => AskForApproval::OnFailure, - ApprovalModeCliArg::UnlessAllowListed => AskForApproval::UnlessAllowListed, + ApprovalModeCliArg::Untrusted => AskForApproval::UnlessAllowListed, ApprovalModeCliArg::Never => AskForApproval::Never, } } diff --git a/codex-rs/config.md b/codex-rs/config.md index 0da42b9af2..14d5fd2252 100644 --- a/codex-rs/config.md +++ b/codex-rs/config.md @@ -80,8 +80,13 @@ wire_api = "chat" Determines when the user should be prompted to approve whether Codex can execute a command: ```toml -# This is analogous to --suggest in the TypeScript Codex CLI -approval_policy = "unless-allow-listed" +# Codex has hardcoded logic that defines a set of "trusted" commands. +# Setting the approval_policy to `untrusted` means that Codex will prompt the +# user before running a command not in the "trusted" set. +# +# See https://github.com/openai/codex/issues/1260 for the plan to enable +# end-users to define their own trusted commands. +approval_policy = "untrusted" ``` ```toml diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 42cf92996f..7533ddf879 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -110,22 +110,18 @@ pub enum Op { GetHistoryEntryRequest { offset: usize, log_id: u64 }, } -/// Determines how liberally commands are auto‑approved by the system. +/// Determines the conditions under which the user is consulted to approve +/// running the command proposed by Codex. #[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 + /// 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] + #[serde(rename = "untrusted")] UnlessAllowListed, - /// In addition to everything allowed by **`Suggest`**, commands that - /// *write* to files **within the user’s approved list of writable paths** - /// are also auto‑approved. - /// TODO(ragona): fix - AutoEdit, - /// *All* commands are auto‑approved, but they are expected to run inside a /// sandbox where network access is disabled and writes are confined to a /// specific set of paths. If the command fails, it will be escalated to diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index 8417bf0c5d..a93316e3ba 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -31,7 +31,7 @@ pub fn assess_patch_safety( } match policy { - AskForApproval::OnFailure | AskForApproval::AutoEdit | AskForApproval::Never => { + AskForApproval::OnFailure | AskForApproval::Never => { // Continue to see if this can be auto-approved. } // TODO(ragona): I'm not sure this is actually correct? I believe in this case diff --git a/codex-rs/mcp-server/src/codex_tool_config.rs b/codex-rs/mcp-server/src/codex_tool_config.rs index 0afefc15ca..330ee65f73 100644 --- a/codex-rs/mcp-server/src/codex_tool_config.rs +++ b/codex-rs/mcp-server/src/codex_tool_config.rs @@ -47,7 +47,6 @@ pub(crate) struct CodexToolCallParam { #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(rename_all = "kebab-case")] pub(crate) enum CodexToolCallApprovalPolicy { - AutoEdit, UnlessAllowListed, OnFailure, Never, @@ -56,7 +55,6 @@ pub(crate) enum CodexToolCallApprovalPolicy { impl From for AskForApproval { fn from(value: CodexToolCallApprovalPolicy) -> Self { match value { - CodexToolCallApprovalPolicy::AutoEdit => AskForApproval::AutoEdit, CodexToolCallApprovalPolicy::UnlessAllowListed => AskForApproval::UnlessAllowListed, CodexToolCallApprovalPolicy::OnFailure => AskForApproval::OnFailure, CodexToolCallApprovalPolicy::Never => AskForApproval::Never,