From 056c8f82792d58c0305902e9dbc27c80b4fc9e32 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 19 Nov 2025 13:55:29 -0800 Subject: [PATCH 1/9] fix: prepare ExecPolicy in exec-server for execpolicy2 cutover (#6888) This PR introduces an extra layer of abstraction to prepare us for the migration to execpolicy2: - introduces a new trait, `EscalationPolicy`, whose `determine_action()` method is responsible for producing the `EscalateAction` - the existing `ExecPolicy` typedef is changed to return an intermediate `ExecPolicyOutcome` instead of `EscalateAction` - the default implementation of `EscalationPolicy`, `McpEscalationPolicy`, composes `ExecPolicy` - the `ExecPolicyOutcome` includes `codex_execpolicy2::Decision`, which has a `Prompt` variant - when `McpEscalationPolicy` gets `Decision::Prompt` back from `ExecPolicy`, it prompts the user via an MCP elicitation and maps the result into an `ElicitationAction` - now that the end user can reply to an elicitation with `Decline` or `Cancel`, we introduce a new variant, `EscalateAction::Deny`, which the client handles by returning exit code `1` without running anything Note the way the elicitation is created is still not quite right, but I will fix that once we have things running end-to-end for real in a follow-up PR. --- codex-rs/Cargo.lock | 2 + codex-rs/exec-server/Cargo.toml | 2 + codex-rs/exec-server/src/posix.rs | 74 +++++++++-- .../exec-server/src/posix/escalate_client.rs | 7 ++ .../src/posix/escalate_protocol.rs | 2 + .../exec-server/src/posix/escalate_server.rs | 71 ++++++++--- .../src/posix/escalation_policy.rs | 14 +++ codex-rs/exec-server/src/posix/mcp.rs | 33 ++--- .../src/posix/mcp_escalation_policy.rs | 117 ++++++++++++++++++ 9 files changed, 266 insertions(+), 56 deletions(-) create mode 100644 codex-rs/exec-server/src/posix/escalation_policy.rs create mode 100644 codex-rs/exec-server/src/posix/mcp_escalation_policy.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 6ace22c766..66a1c208e4 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1188,6 +1188,7 @@ name = "codex-exec-server" version = "0.0.0" dependencies = [ "anyhow", + "async-trait", "clap", "codex-core", "libc", @@ -1196,6 +1197,7 @@ dependencies = [ "rmcp", "serde", "serde_json", + "shlex", "socket2 0.6.0", "tempfile", "tokio", diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 24ee460f50..60e0facff1 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -12,6 +12,7 @@ workspace = true [dependencies] anyhow = { workspace = true } +async-trait = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-core = { workspace = true } libc = { workspace = true } @@ -31,6 +32,7 @@ rmcp = { workspace = true, default-features = false, features = [ ] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } +shlex = { workspace = true } socket2 = { workspace = true } tokio = { workspace = true, features = [ "io-std", diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index 179fc7a6fc..bbb624df09 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -64,25 +64,17 @@ use tracing_subscriber::{self}; use crate::posix::escalate_protocol::EscalateAction; use crate::posix::escalate_server::EscalateServer; +use crate::posix::escalation_policy::EscalationPolicy; +use crate::posix::mcp_escalation_policy::ExecPolicyOutcome; mod escalate_client; mod escalate_protocol; mod escalate_server; +mod escalation_policy; mod mcp; +mod mcp_escalation_policy; mod socket; -fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> EscalateAction { - // TODO: execpolicy - if file == Path::new("/opt/homebrew/bin/gh") - && let [_, arg1, arg2, ..] = argv - && arg1 == "issue" - && arg2 == "list" - { - return EscalateAction::Escalate; - } - EscalateAction::Run -} - #[derive(Parser)] #[command(version)] pub struct Cli { @@ -135,7 +127,7 @@ pub async fn main() -> anyhow::Result<()> { } Some(Commands::ShellExec(args)) => { let bash_path = mcp::get_bash_path()?; - let escalate_server = EscalateServer::new(bash_path, dummy_exec_policy); + let escalate_server = EscalateServer::new(bash_path, DummyEscalationPolicy {}); let result = escalate_server .exec( args.command.clone(), @@ -162,3 +154,59 @@ pub async fn main() -> anyhow::Result<()> { } } } + +// TODO: replace with execpolicy2 + +struct DummyEscalationPolicy; + +#[async_trait::async_trait] +impl EscalationPolicy for DummyEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + ) -> Result { + let outcome = dummy_exec_policy(file, argv, workdir); + let action = match outcome { + ExecPolicyOutcome::Allow { + run_with_escalated_permissions, + } => { + if run_with_escalated_permissions { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ExecPolicyOutcome::Forbidden => EscalateAction::Deny { + reason: Some("Execution forbidden by policy".to_string()), + }, + ExecPolicyOutcome::Prompt { .. } => EscalateAction::Deny { + reason: Some("Could not prompt user for permission".to_string()), + }, + }; + Ok(action) + } +} + +fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome { + if file.ends_with("/rm") { + ExecPolicyOutcome::Forbidden + } else if file.ends_with("/git") { + ExecPolicyOutcome::Prompt { + run_with_escalated_permissions: false, + } + } else if file == Path::new("/opt/homebrew/bin/gh") + && let [_, arg1, arg2, ..] = argv + && arg1 == "issue" + && arg2 == "list" + { + ExecPolicyOutcome::Allow { + run_with_escalated_permissions: true, + } + } else { + ExecPolicyOutcome::Allow { + run_with_escalated_permissions: false, + } + } +} diff --git a/codex-rs/exec-server/src/posix/escalate_client.rs b/codex-rs/exec-server/src/posix/escalate_client.rs index 2add1dade2..bea4b6fa5f 100644 --- a/codex-rs/exec-server/src/posix/escalate_client.rs +++ b/codex-rs/exec-server/src/posix/escalate_client.rs @@ -98,5 +98,12 @@ pub(crate) async fn run(file: String, argv: Vec) -> anyhow::Result Err(err.into()) } + EscalateAction::Deny { reason } => { + match reason { + Some(reason) => eprintln!("Execution denied: {reason}"), + None => eprintln!("Execution denied"), + } + Ok(1) + } } } diff --git a/codex-rs/exec-server/src/posix/escalate_protocol.rs b/codex-rs/exec-server/src/posix/escalate_protocol.rs index 09d33fc982..e3fc27d071 100644 --- a/codex-rs/exec-server/src/posix/escalate_protocol.rs +++ b/codex-rs/exec-server/src/posix/escalate_protocol.rs @@ -34,6 +34,8 @@ pub(super) enum EscalateAction { Run, /// The command should be escalated to the server for execution. Escalate, + /// The command should not be executed. + Deny { reason: Option }, } /// The client sends this to the server to forward its open FDs. diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/exec-server/src/posix/escalate_server.rs index 6d058ff0f0..4f259f0506 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/exec-server/src/posix/escalate_server.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use std::os::fd::AsRawFd; -use std::path::Path; use std::path::PathBuf; use std::process::Stdio; +use std::sync::Arc; use std::time::Duration; use anyhow::Context as _; @@ -21,25 +21,24 @@ use crate::posix::escalate_protocol::EscalateRequest; use crate::posix::escalate_protocol::EscalateResponse; use crate::posix::escalate_protocol::SuperExecMessage; use crate::posix::escalate_protocol::SuperExecResult; +use crate::posix::escalation_policy::EscalationPolicy; use crate::posix::socket::AsyncDatagramSocket; use crate::posix::socket::AsyncSocket; -/// This is the policy which decides how to handle an exec() call. -/// -/// `file` is the absolute, canonical path to the executable to run, i.e. the first arg to exec. -/// `argv` is the argv, including the program name (`argv[0]`). -/// `workdir` is the absolute, canonical path to the working directory in which to execute the -/// command. -pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> EscalateAction; - pub(crate) struct EscalateServer { bash_path: PathBuf, - policy: ExecPolicy, + policy: Arc, } impl EscalateServer { - pub fn new(bash_path: PathBuf, policy: ExecPolicy) -> Self { - Self { bash_path, policy } + pub fn new

(bash_path: PathBuf, policy: P) -> Self + where + P: EscalationPolicy + Send + Sync + 'static, + { + Self { + bash_path, + policy: Arc::new(policy), + } } pub async fn exec( @@ -53,7 +52,7 @@ impl EscalateServer { let client_socket = escalate_client.into_inner(); client_socket.set_cloexec(false)?; - let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy)); + let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy.clone())); let mut env = env.clone(); env.insert( ESCALATE_SOCKET_ENV_VAR.to_string(), @@ -96,7 +95,10 @@ impl EscalateServer { } } -async fn escalate_task(socket: AsyncDatagramSocket, policy: ExecPolicy) -> anyhow::Result<()> { +async fn escalate_task( + socket: AsyncDatagramSocket, + policy: Arc, +) -> anyhow::Result<()> { loop { let (_, mut fds) = socket.receive_with_fds().await?; if fds.len() != 1 { @@ -104,6 +106,7 @@ async fn escalate_task(socket: AsyncDatagramSocket, policy: ExecPolicy) -> anyho continue; } let stream_socket = AsyncSocket::from_fd(fds.remove(0))?; + let policy = policy.clone(); tokio::spawn(async move { if let Err(err) = handle_escalate_session_with_policy(stream_socket, policy).await { tracing::error!("escalate session failed: {err:?}"); @@ -122,7 +125,7 @@ pub(crate) struct ExecResult { async fn handle_escalate_session_with_policy( socket: AsyncSocket, - policy: ExecPolicy, + policy: Arc, ) -> anyhow::Result<()> { let EscalateRequest { file, @@ -132,8 +135,12 @@ async fn handle_escalate_session_with_policy( } = socket.receive::().await?; let file = PathBuf::from(&file).absolutize()?.into_owned(); let workdir = PathBuf::from(&workdir).absolutize()?.into_owned(); - let action = policy(file.as_path(), &argv, &workdir); + let action = policy + .determine_action(file.as_path(), &argv, &workdir) + .await?; + tracing::debug!("decided {action:?} for {file:?} {argv:?} {workdir:?}"); + match action { EscalateAction::Run => { socket @@ -195,6 +202,13 @@ async fn handle_escalate_session_with_policy( }) .await?; } + EscalateAction::Deny { reason } => { + socket + .send(EscalateResponse { + action: EscalateAction::Deny { reason }, + }) + .await?; + } } Ok(()) } @@ -204,14 +218,33 @@ mod tests { use super::*; use pretty_assertions::assert_eq; use std::collections::HashMap; + use std::path::Path; use std::path::PathBuf; + struct DeterministicEscalationPolicy { + action: EscalateAction, + } + + #[async_trait::async_trait] + impl EscalationPolicy for DeterministicEscalationPolicy { + async fn determine_action( + &self, + _file: &Path, + _argv: &[String], + _workdir: &Path, + ) -> Result { + Ok(self.action.clone()) + } + } + #[tokio::test] async fn handle_escalate_session_respects_run_in_sandbox_decision() -> anyhow::Result<()> { let (server, client) = AsyncSocket::pair()?; let server_task = tokio::spawn(handle_escalate_session_with_policy( server, - |_file, _argv, _workdir| EscalateAction::Run, + Arc::new(DeterministicEscalationPolicy { + action: EscalateAction::Run, + }), )); client @@ -238,7 +271,9 @@ mod tests { let (server, client) = AsyncSocket::pair()?; let server_task = tokio::spawn(handle_escalate_session_with_policy( server, - |_file, _argv, _workdir| EscalateAction::Escalate, + Arc::new(DeterministicEscalationPolicy { + action: EscalateAction::Escalate, + }), )); client diff --git a/codex-rs/exec-server/src/posix/escalation_policy.rs b/codex-rs/exec-server/src/posix/escalation_policy.rs new file mode 100644 index 0000000000..a7fcc4f627 --- /dev/null +++ b/codex-rs/exec-server/src/posix/escalation_policy.rs @@ -0,0 +1,14 @@ +use std::path::Path; + +use crate::posix::escalate_protocol::EscalateAction; + +/// Decides what action to take in response to an execve request from a client. +#[async_trait::async_trait] +pub(crate) trait EscalationPolicy: Send + Sync { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + ) -> Result; +} diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index f4e2b19d4f..2a6e84dd9e 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -18,9 +18,10 @@ use rmcp::tool_handler; use rmcp::tool_router; use rmcp::transport::stdio; -use crate::posix::escalate_server; use crate::posix::escalate_server::EscalateServer; -use crate::posix::escalate_server::ExecPolicy; +use crate::posix::escalate_server::{self}; +use crate::posix::mcp_escalation_policy::ExecPolicy; +use crate::posix::mcp_escalation_policy::McpEscalationPolicy; /// Path to our patched bash. const CODEX_BASH_PATH_ENV_VAR: &str = "CODEX_BASH_PATH"; @@ -81,10 +82,13 @@ impl ExecTool { #[tool] async fn shell( &self, - _context: RequestContext, + context: RequestContext, Parameters(params): Parameters, ) -> Result { - let escalate_server = EscalateServer::new(self.bash_path.clone(), self.policy); + let escalate_server = EscalateServer::new( + self.bash_path.clone(), + McpEscalationPolicy::new(self.policy, context), + ); let result = escalate_server .exec( params.command, @@ -99,27 +103,6 @@ impl ExecTool { ExecResult::from(result), )?])) } - - #[allow(dead_code)] - async fn prompt( - &self, - command: String, - workdir: String, - context: RequestContext, - ) -> Result { - context - .peer - .create_elicitation(CreateElicitationRequestParam { - message: format!("Allow Codex to run `{command:?}` in `{workdir:?}`?"), - #[allow(clippy::expect_used)] - requested_schema: ElicitationSchema::builder() - .property("dummy", PrimitiveSchema::String(StringSchema::new())) - .build() - .expect("failed to build elicitation schema"), - }) - .await - .map_err(|e| McpError::internal_error(e.to_string(), None)) - } } #[tool_handler] diff --git a/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs b/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs new file mode 100644 index 0000000000..24f42fe91b --- /dev/null +++ b/codex-rs/exec-server/src/posix/mcp_escalation_policy.rs @@ -0,0 +1,117 @@ +use std::path::Path; + +use rmcp::ErrorData as McpError; +use rmcp::RoleServer; +use rmcp::model::CreateElicitationRequestParam; +use rmcp::model::CreateElicitationResult; +use rmcp::model::ElicitationAction; +use rmcp::model::ElicitationSchema; +use rmcp::model::PrimitiveSchema; +use rmcp::model::StringSchema; +use rmcp::service::RequestContext; + +use crate::posix::escalate_protocol::EscalateAction; +use crate::posix::escalation_policy::EscalationPolicy; + +/// This is the policy which decides how to handle an exec() call. +/// +/// `file` is the absolute, canonical path to the executable to run, i.e. the first arg to exec. +/// `argv` is the argv, including the program name (`argv[0]`). +/// `workdir` is the absolute, canonical path to the working directory in which to execute the +/// command. +pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> ExecPolicyOutcome; + +pub(crate) enum ExecPolicyOutcome { + Allow { + run_with_escalated_permissions: bool, + }, + Prompt { + run_with_escalated_permissions: bool, + }, + Forbidden, +} + +/// ExecPolicy with access to the MCP RequestContext so that it can leverage +/// elicitations. +pub(crate) struct McpEscalationPolicy { + policy: ExecPolicy, + context: RequestContext, +} + +impl McpEscalationPolicy { + pub(crate) fn new(policy: ExecPolicy, context: RequestContext) -> Self { + Self { policy, context } + } + + async fn prompt( + &self, + _file: &Path, + argv: &[String], + workdir: &Path, + context: RequestContext, + ) -> Result { + let command = shlex::try_join(argv.iter().map(String::as_str)).unwrap_or_default(); + context + .peer + .create_elicitation(CreateElicitationRequestParam { + message: format!("Allow Codex to run `{command:?}` in `{workdir:?}`?"), + #[allow(clippy::expect_used)] + requested_schema: ElicitationSchema::builder() + .property("dummy", PrimitiveSchema::String(StringSchema::new())) + .build() + .expect("failed to build elicitation schema"), + }) + .await + .map_err(|e| McpError::internal_error(e.to_string(), None)) + } +} + +#[async_trait::async_trait] +impl EscalationPolicy for McpEscalationPolicy { + async fn determine_action( + &self, + file: &Path, + argv: &[String], + workdir: &Path, + ) -> Result { + let outcome = (self.policy)(file, argv, workdir); + let action = match outcome { + ExecPolicyOutcome::Allow { + run_with_escalated_permissions, + } => { + if run_with_escalated_permissions { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ExecPolicyOutcome::Prompt { + run_with_escalated_permissions, + } => { + let result = self + .prompt(file, argv, workdir, self.context.clone()) + .await?; + // TODO: Extract reason from `result.content`. + match result.action { + ElicitationAction::Accept => { + if run_with_escalated_permissions { + EscalateAction::Escalate + } else { + EscalateAction::Run + } + } + ElicitationAction::Decline => EscalateAction::Deny { + reason: Some("User declined execution".to_string()), + }, + ElicitationAction::Cancel => EscalateAction::Deny { + reason: Some("User cancelled execution".to_string()), + }, + } + } + ExecPolicyOutcome::Forbidden => EscalateAction::Deny { + reason: Some("Execution forbidden by policy".to_string()), + }, + }; + Ok(action) + } +} From 2fde03b4a040babc9b98c389680cb568b464da85 Mon Sep 17 00:00:00 2001 From: iceweasel-oai Date: Wed, 19 Nov 2025 13:59:17 -0800 Subject: [PATCH 2/9] stop over-reporting world-writable directories (#6936) Fix world-writable audit false positives by expanding generic permissions with MapGenericMask and then checking only concrete write bits. The earlier check looked for FILE_GENERIC_WRITE/generic masks directly, which shares bits with read permissions and could flag an Everyone read ACE as writable. --- codex-rs/windows-sandbox-rs/src/audit.rs | 58 +++++++++++++----------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/codex-rs/windows-sandbox-rs/src/audit.rs b/codex-rs/windows-sandbox-rs/src/audit.rs index 8873bc2e91..cba8d06335 100644 --- a/codex-rs/windows-sandbox-rs/src/audit.rs +++ b/codex-rs/windows-sandbox-rs/src/audit.rs @@ -9,15 +9,29 @@ use std::path::PathBuf; use std::time::Duration; use std::time::Instant; use windows_sys::Win32::Foundation::CloseHandle; -use windows_sys::Win32::Foundation::LocalFree; use windows_sys::Win32::Foundation::ERROR_SUCCESS; use windows_sys::Win32::Foundation::HLOCAL; use windows_sys::Win32::Foundation::INVALID_HANDLE_VALUE; +use windows_sys::Win32::Foundation::LocalFree; +use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE; +use windows_sys::Win32::Security::ACE_HEADER; +use windows_sys::Win32::Security::ACL; +use windows_sys::Win32::Security::ACL_SIZE_INFORMATION; +use windows_sys::Win32::Security::AclSizeInformation; use windows_sys::Win32::Security::Authorization::GetNamedSecurityInfoW; use windows_sys::Win32::Security::Authorization::GetSecurityInfo; +use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; +use windows_sys::Win32::Security::EqualSid; +use windows_sys::Win32::Security::GetAce; +use windows_sys::Win32::Security::GetAclInformation; +use windows_sys::Win32::Security::MapGenericMask; +use windows_sys::Win32::Security::GENERIC_MAPPING; use windows_sys::Win32::Storage::FileSystem::CreateFileW; +use windows_sys::Win32::Storage::FileSystem::FILE_ALL_ACCESS; use windows_sys::Win32::Storage::FileSystem::FILE_APPEND_DATA; use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_BACKUP_SEMANTICS; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_EXECUTE; +use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_READ; use windows_sys::Win32::Storage::FileSystem::FILE_GENERIC_WRITE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_DELETE; use windows_sys::Win32::Storage::FileSystem::FILE_SHARE_READ; @@ -26,17 +40,6 @@ use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_ATTRIBUTES; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_DATA; use windows_sys::Win32::Storage::FileSystem::FILE_WRITE_EA; use windows_sys::Win32::Storage::FileSystem::OPEN_EXISTING; -const GENERIC_ALL_MASK: u32 = 0x1000_0000; -const GENERIC_WRITE_MASK: u32 = 0x4000_0000; -use windows_sys::Win32::Security::AclSizeInformation; -use windows_sys::Win32::Security::EqualSid; -use windows_sys::Win32::Security::GetAce; -use windows_sys::Win32::Security::GetAclInformation; -use windows_sys::Win32::Security::ACCESS_ALLOWED_ACE; -use windows_sys::Win32::Security::ACE_HEADER; -use windows_sys::Win32::Security::ACL; -use windows_sys::Win32::Security::ACL_SIZE_INFORMATION; -use windows_sys::Win32::Security::DACL_SECURITY_INFORMATION; // Preflight scan limits const MAX_ITEMS_PER_DIR: i32 = 1000; @@ -304,7 +307,7 @@ pub fn world_writable_warning_details( } } // Fast mask-based check: does the DACL contain any ACCESS_ALLOWED ACE for -// Everyone that includes generic or specific write bits? Skips inherit-only +// Everyone that grants write after generic bits are expanded? Skips inherit-only // ACEs (do not apply to the current object). unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut c_void) -> bool { if p_dacl.is_null() { @@ -321,6 +324,12 @@ unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut if ok == 0 { return false; } + let mapping = GENERIC_MAPPING { + GenericRead: FILE_GENERIC_READ, + GenericWrite: FILE_GENERIC_WRITE, + GenericExecute: FILE_GENERIC_EXECUTE, + GenericAll: FILE_ALL_ACCESS, + }; for i in 0..(info.AceCount as usize) { let mut p_ace: *mut c_void = std::ptr::null_mut(); if GetAce(p_dacl as *const ACL, i as u32, &mut p_ace) == 0 { @@ -337,19 +346,16 @@ unsafe fn dacl_quick_world_write_mask_allows(p_dacl: *mut ACL, psid_world: *mut let base = p_ace as usize; let sid_ptr = (base + std::mem::size_of::() + std::mem::size_of::()) as *mut c_void; // skip header + mask - if EqualSid(sid_ptr, psid_world) != 0 { - let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE); - let mask = ace.Mask; - let writey = FILE_GENERIC_WRITE - | FILE_WRITE_DATA - | FILE_APPEND_DATA - | FILE_WRITE_EA - | FILE_WRITE_ATTRIBUTES - | GENERIC_WRITE_MASK - | GENERIC_ALL_MASK; - if (mask & writey) != 0 { - return true; - } + if EqualSid(sid_ptr, psid_world) == 0 { + continue; + } + let ace = &*(p_ace as *const ACCESS_ALLOWED_ACE); + let mut mask = ace.Mask; + // Expand generic bits to concrete file rights before checking for write. + MapGenericMask(&mut mask, &mapping); + let write_mask = FILE_WRITE_DATA | FILE_APPEND_DATA | FILE_WRITE_EA | FILE_WRITE_ATTRIBUTES; + if (mask & write_mask) != 0 { + return true; } } false From 692989c2770065baafe57d7b6edc89bbd7c61273 Mon Sep 17 00:00:00 2001 From: Beehive Innovations Date: Thu, 20 Nov 2025 02:50:07 +0400 Subject: [PATCH 3/9] fix(context left after review): review footer context after `/review` (#5610) ## Summary - show live review token usage while `/review` runs and restore the main session indicator afterward - add regression coverage for the footer behavior ## Testing - just fmt - cargo test -p codex-tui Fixes #5604 --------- Signed-off-by: Fahad --- codex-rs/tui/src/bottom_pane/mod.rs | 5 ++ codex-rs/tui/src/chatwidget.rs | 47 +++++++++++--- codex-rs/tui/src/chatwidget/tests.rs | 94 ++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 8 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index da2efb63c1..6738d7672d 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -114,6 +114,11 @@ impl BottomPane { self.status.as_ref() } + #[cfg(test)] + pub(crate) fn context_window_percent(&self) -> Option { + self.context_window_percent + } + fn active_view(&self) -> Option<&dyn BottomPaneView> { self.view_stack.last().map(std::convert::AsRef::as_ref) } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e832b58902..3a79330b74 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -290,6 +290,8 @@ pub(crate) struct ChatWidget { pending_notification: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, + // Snapshot of token usage to restore after review mode exits. + pre_review_token_info: Option>, // Whether to add a final message separator after the last message needs_final_message_separator: bool, @@ -489,16 +491,39 @@ impl ChatWidget { } pub(crate) fn set_token_info(&mut self, info: Option) { - if let Some(info) = info { - let context_window = info - .model_context_window - .or(self.config.model_context_window); - let percent = context_window.map(|window| { + match info { + Some(info) => self.apply_token_info(info), + None => { + self.bottom_pane.set_context_window_percent(None); + self.token_info = None; + } + } + } + + fn apply_token_info(&mut self, info: TokenUsageInfo) { + let percent = self.context_remaining_percent(&info); + self.bottom_pane.set_context_window_percent(percent); + self.token_info = Some(info); + } + + fn context_remaining_percent(&self, info: &TokenUsageInfo) -> Option { + info.model_context_window + .or(self.config.model_context_window) + .map(|window| { info.last_token_usage .percent_of_context_window_remaining(window) - }); - self.bottom_pane.set_context_window_percent(percent); - self.token_info = Some(info); + }) + } + + fn restore_pre_review_token_info(&mut self) { + if let Some(saved) = self.pre_review_token_info.take() { + match saved { + Some(info) => self.apply_token_info(info), + None => { + self.bottom_pane.set_context_window_percent(None); + self.token_info = None; + } + } } } @@ -1150,6 +1175,7 @@ impl ChatWidget { suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, + pre_review_token_info: None, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1223,6 +1249,7 @@ impl ChatWidget { suppress_session_configured_redraw: true, pending_notification: None, is_review_mode: false, + pre_review_token_info: None, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback, @@ -1693,6 +1720,9 @@ impl ChatWidget { fn on_entered_review_mode(&mut self, review: ReviewRequest) { // Enter review mode and emit a concise banner + if self.pre_review_token_info.is_none() { + self.pre_review_token_info = Some(self.token_info.clone()); + } self.is_review_mode = true; let banner = format!(">> Code review started: {} <<", review.user_facing_hint); self.add_to_history(history_cell::new_review_status_line(banner)); @@ -1733,6 +1763,7 @@ impl ChatWidget { } self.is_review_mode = false; + self.restore_pre_review_token_info(); // Append a finishing banner at the end of this turn. self.add_to_history(history_cell::new_review_status_line( "<< Code review finished >>".to_string(), diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a6ba546472..a4cef22418 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -38,6 +38,9 @@ use codex_core::protocol::ReviewRequest; use codex_core::protocol::StreamErrorEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TaskStartedEvent; +use codex_core::protocol::TokenCountEvent; +use codex_core::protocol::TokenUsage; +use codex_core::protocol::TokenUsageInfo; use codex_core::protocol::UndoCompletedEvent; use codex_core::protocol::UndoStartedEvent; use codex_core::protocol::ViewImageToolCallEvent; @@ -215,6 +218,81 @@ fn exited_review_mode_emits_results_and_finishes() { assert!(!chat.is_review_mode); } +/// Exiting review restores the pre-review context window indicator. +#[test] +fn review_restores_context_window_indicator() { + let (mut chat, mut rx, _ops) = make_chatwidget_manual(); + + let context_window = 13_000; + let pre_review_tokens = 12_700; // ~30% remaining after subtracting baseline. + let review_tokens = 12_030; // ~97% remaining after subtracting baseline. + + chat.handle_codex_event(Event { + id: "token-before".into(), + msg: EventMsg::TokenCount(TokenCountEvent { + info: Some(make_token_info(pre_review_tokens, context_window)), + rate_limits: None, + }), + }); + assert_eq!(chat.bottom_pane.context_window_percent(), Some(30)); + + chat.handle_codex_event(Event { + id: "review-start".into(), + msg: EventMsg::EnteredReviewMode(ReviewRequest { + prompt: "Review the latest changes".to_string(), + user_facing_hint: "feature branch".to_string(), + append_to_original_thread: true, + }), + }); + + chat.handle_codex_event(Event { + id: "token-review".into(), + msg: EventMsg::TokenCount(TokenCountEvent { + info: Some(make_token_info(review_tokens, context_window)), + rate_limits: None, + }), + }); + assert_eq!(chat.bottom_pane.context_window_percent(), Some(97)); + + chat.handle_codex_event(Event { + id: "review-end".into(), + msg: EventMsg::ExitedReviewMode(ExitedReviewModeEvent { + review_output: None, + }), + }); + let _ = drain_insert_history(&mut rx); + + assert_eq!(chat.bottom_pane.context_window_percent(), Some(30)); + assert!(!chat.is_review_mode); +} + +/// Receiving a TokenCount event without usage clears the context indicator. +#[test] +fn token_count_none_resets_context_indicator() { + let (mut chat, _rx, _ops) = make_chatwidget_manual(); + + let context_window = 13_000; + let pre_compact_tokens = 12_700; + + chat.handle_codex_event(Event { + id: "token-before".into(), + msg: EventMsg::TokenCount(TokenCountEvent { + info: Some(make_token_info(pre_compact_tokens, context_window)), + rate_limits: None, + }), + }); + assert_eq!(chat.bottom_pane.context_window_percent(), Some(30)); + + chat.handle_codex_event(Event { + id: "token-cleared".into(), + msg: EventMsg::TokenCount(TokenCountEvent { + info: None, + rate_limits: None, + }), + }); + assert_eq!(chat.bottom_pane.context_window_percent(), None); +} + #[cfg_attr( target_os = "macos", ignore = "system configuration APIs are blocked under macOS seatbelt" @@ -292,6 +370,7 @@ fn make_chatwidget_manual() -> ( suppress_session_configured_redraw: false, pending_notification: None, is_review_mode: false, + pre_review_token_info: None, needs_final_message_separator: false, last_rendered_width: std::cell::Cell::new(None), feedback: codex_feedback::CodexFeedback::new(), @@ -338,6 +417,21 @@ fn lines_to_single_string(lines: &[ratatui::text::Line<'static>]) -> String { s } +fn make_token_info(total_tokens: i64, context_window: i64) -> TokenUsageInfo { + fn usage(total_tokens: i64) -> TokenUsage { + TokenUsage { + total_tokens, + ..TokenUsage::default() + } + } + + TokenUsageInfo { + total_token_usage: usage(total_tokens), + last_token_usage: usage(total_tokens), + model_context_window: Some(context_window), + } +} + #[test] fn rate_limit_warnings_emit_thresholds() { let mut state = RateLimitWarningState::default(); From a6597a9958d6ae54e1a3601c653a42cd7d70979a Mon Sep 17 00:00:00 2001 From: Lionel Cheng <60159831+lionelchg@users.noreply.github.com> Date: Wed, 19 Nov 2025 15:52:24 -0800 Subject: [PATCH 4/9] Fix/correct reasoning display (#6749) This closes #6748 by implementing fallback to `model_family.default_reasoning_effort` in `reasoning_effort` display of `/status` when no `model_reasoning_effort` is set in the configuration. ## common/src/config_summary.rs - `create_config_summary_entries` now fills the "reasoning effort" entry with the explicit `config.model_reasoning_effort` when present and falls back to `config.model_family.default_reasoning_effort` when it is `None`, instead of emitting the literal string `none`. - This ensures downstream consumers such as `tui/src/status/helpers.rs` continue to work unchanged while automatically picking up model-family defaults when the user has not selected a reasoning effort. ## tui/src/status/helpers.rs / core/src/model_family.rs `ModelFamily::default_reasoning_effort` metadata is set to `medium` for both `gpt-5*-codex` and `gpt-5` models following the default behaviour of the API and recommendation of the codebase: - per https://platform.openai.com/docs/api-reference/responses/create `gpt-5` defaults to `medium` reasoning when no preset is passed - there is no mention of the preset for `gpt-5.1-codex` in the API docs but `medium` is the default setting for `gpt-5.1-codex` as per `codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__model_reasoning_selection_popup.snap` --------- Signed-off-by: lionelchg Co-authored-by: Eric Traut --- codex-rs/common/src/config_summary.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/codex-rs/common/src/config_summary.rs b/codex-rs/common/src/config_summary.rs index dabc606ce1..8fc1bb26f3 100644 --- a/codex-rs/common/src/config_summary.rs +++ b/codex-rs/common/src/config_summary.rs @@ -15,13 +15,12 @@ pub fn create_config_summary_entries(config: &Config) -> Vec<(&'static str, Stri if config.model_provider.wire_api == WireApi::Responses && config.model_family.supports_reasoning_summaries { - entries.push(( - "reasoning effort", - config - .model_reasoning_effort - .map(|effort| effort.to_string()) - .unwrap_or_else(|| "none".to_string()), - )); + let reasoning_effort = config + .model_reasoning_effort + .or(config.model_family.default_reasoning_effort) + .map(|effort| effort.to_string()) + .unwrap_or_else(|| "none".to_string()); + entries.push(("reasoning effort", reasoning_effort)); entries.push(( "reasoning summaries", config.model_reasoning_summary.to_string(), From 13d378f2ce3a95bdcea912c573c19fc4dca87ca3 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 19 Nov 2025 16:38:14 -0800 Subject: [PATCH 5/9] chore: refactor exec-server to prepare it for standalone MCP use (#6944) This PR reorganizes things slightly so that: - Instead of a single multitool executable, `codex-exec-server`, we now have two executables: - `codex-exec-mcp-server` to launch the MCP server - `codex-execve-wrapper` is the `execve(2)` wrapper to use with the `BASH_EXEC_WRAPPER` environment variable - `BASH_EXEC_WRAPPER` must be a single executable: it cannot be a command string composed of an executable with args (i.e., it no longer adds the `escalate` subcommand, as before) - `codex-exec-mcp-server` takes `--bash` and `--execve` as options. Though if `--execve` is not specified, the MCP server will check the directory containing `std::env::current_exe()` and attempt to use the file named `codex-execve-wrapper` within it. In development, this works out since these executables are side-by-side in the `target/debug` folder. With respect to testing, this also fixes an important bug in `dummy_exec_policy()`, as I was using `ends_with()` as if it applied to a `String`, but in this case, it is used with a `&Path`, so the semantics are slightly different. Putting this all together, I was able to test this by running the following: ``` ~/code/codex/codex-rs$ npx @modelcontextprotocol/inspector \ ./target/debug/codex-exec-mcp-server --bash ~/code/bash/bash ``` If I try to run `git status` in `/Users/mbolin/code/codex` via the `shell` tool from the MCP server: image then I get prompted with the following elicitation, as expected: image Though a current limitation is that the `shell` tool defaults to a timeout of 10s, which means I only have 10s to respond to the elicitation. Ideally, the time spent waiting for a response from a human should not count against the timeout for the command execution. I will address this in a subsequent PR. --- Note `~/code/bash/bash` was created by doing: ``` cd ~/code git clone https://github.com/bminor/bash cd bash git checkout a8a1c2fac029404d3f42cd39f5a20f24b6e4fe4b ./configure make ``` The patch: ``` diff --git a/execute_cmd.c b/execute_cmd.c index 070f5119..d20ad2b9 100644 --- a/execute_cmd.c +++ b/execute_cmd.c @@ -6129,6 +6129,19 @@ shell_execve (char *command, char **args, char **env) char sample[HASH_BANG_BUFSIZ]; size_t larray; + char* exec_wrapper = getenv("BASH_EXEC_WRAPPER"); + if (exec_wrapper && *exec_wrapper && !whitespace (*exec_wrapper)) + { + char *orig_command = command; + + larray = strvec_len (args); + + memmove (args + 2, args, (++larray) * sizeof (char *)); + args[0] = exec_wrapper; + args[1] = orig_command; + command = exec_wrapper; + } + ``` --- codex-rs/exec-server/Cargo.toml | 12 +- .../src/bin/main_execve_wrapper.rs | 8 + .../exec-server/src/bin/main_mcp_server.rs | 8 + codex-rs/exec-server/src/lib.rs | 8 + codex-rs/exec-server/src/main.rs | 11 -- codex-rs/exec-server/src/posix.rs | 163 +++++++----------- .../exec-server/src/posix/escalate_server.rs | 18 +- codex-rs/exec-server/src/posix/mcp.rs | 8 +- 8 files changed, 113 insertions(+), 123 deletions(-) create mode 100644 codex-rs/exec-server/src/bin/main_execve_wrapper.rs create mode 100644 codex-rs/exec-server/src/bin/main_mcp_server.rs create mode 100644 codex-rs/exec-server/src/lib.rs delete mode 100644 codex-rs/exec-server/src/main.rs diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index 60e0facff1..54cead4118 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -4,8 +4,16 @@ name = "codex-exec-server" version = { workspace = true } [[bin]] -name = "codex-exec-server" -path = "src/main.rs" +name = "codex-execve-wrapper" +path = "src/bin/main_execve_wrapper.rs" + +[[bin]] +name = "codex-exec-mcp-server" +path = "src/bin/main_mcp_server.rs" + +[lib] +name = "codex_exec_server" +path = "src/lib.rs" [lints] workspace = true diff --git a/codex-rs/exec-server/src/bin/main_execve_wrapper.rs b/codex-rs/exec-server/src/bin/main_execve_wrapper.rs new file mode 100644 index 0000000000..3ab346e8ef --- /dev/null +++ b/codex-rs/exec-server/src/bin/main_execve_wrapper.rs @@ -0,0 +1,8 @@ +#[cfg(not(unix))] +fn main() { + eprintln!("codex-execve-wrapper is only implemented for UNIX"); + std::process::exit(1); +} + +#[cfg(unix)] +pub use codex_exec_server::main_execve_wrapper as main; diff --git a/codex-rs/exec-server/src/bin/main_mcp_server.rs b/codex-rs/exec-server/src/bin/main_mcp_server.rs new file mode 100644 index 0000000000..6c75ae4237 --- /dev/null +++ b/codex-rs/exec-server/src/bin/main_mcp_server.rs @@ -0,0 +1,8 @@ +#[cfg(not(unix))] +fn main() { + eprintln!("codex-exec-mcp-server is only implemented for UNIX"); + std::process::exit(1); +} + +#[cfg(unix)] +pub use codex_exec_server::main_mcp_server as main; diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs new file mode 100644 index 0000000000..adec09d4de --- /dev/null +++ b/codex-rs/exec-server/src/lib.rs @@ -0,0 +1,8 @@ +#[cfg(unix)] +mod posix; + +#[cfg(unix)] +pub use posix::main_execve_wrapper; + +#[cfg(unix)] +pub use posix::main_mcp_server; diff --git a/codex-rs/exec-server/src/main.rs b/codex-rs/exec-server/src/main.rs deleted file mode 100644 index 23a18b2525..0000000000 --- a/codex-rs/exec-server/src/main.rs +++ /dev/null @@ -1,11 +0,0 @@ -#[cfg(target_os = "windows")] -fn main() { - eprintln!("codex-exec-server is not implemented on Windows targets"); - std::process::exit(1); -} - -#[cfg(not(target_os = "windows"))] -mod posix; - -#[cfg(not(target_os = "windows"))] -pub use posix::main; diff --git a/codex-rs/exec-server/src/posix.rs b/codex-rs/exec-server/src/posix.rs index bbb624df09..239eaf61f2 100644 --- a/codex-rs/exec-server/src/posix.rs +++ b/codex-rs/exec-server/src/posix.rs @@ -56,15 +56,12 @@ //! o<-----x //! use std::path::Path; +use std::path::PathBuf; use clap::Parser; -use clap::Subcommand; use tracing_subscriber::EnvFilter; use tracing_subscriber::{self}; -use crate::posix::escalate_protocol::EscalateAction; -use crate::posix::escalate_server::EscalateServer; -use crate::posix::escalation_policy::EscalationPolicy; use crate::posix::mcp_escalation_policy::ExecPolicyOutcome; mod escalate_client; @@ -75,124 +72,84 @@ mod mcp; mod mcp_escalation_policy; mod socket; +/// Default value of --execve option relative to the current executable. +/// Note this must match the name of the binary as specified in Cargo.toml. +const CODEX_EXECVE_WRAPPER_EXE_NAME: &str = "codex-execve-wrapper"; + #[derive(Parser)] -#[command(version)] -pub struct Cli { - #[command(subcommand)] - subcommand: Option, -} +struct McpServerCli { + /// Executable to delegate execve(2) calls to in Bash. + #[arg(long = "execve")] + execve_wrapper: Option, -#[derive(Subcommand)] -enum Commands { - Escalate(EscalateArgs), - ShellExec(ShellExecArgs), -} - -/// Invoked from within the sandbox to (potentially) escalate permissions. -#[derive(Parser, Debug)] -struct EscalateArgs { - file: String, - - #[arg(trailing_var_arg = true)] - argv: Vec, -} - -impl EscalateArgs { - /// This is the escalate client. It talks to the escalate server to determine whether to exec() - /// the command directly or to proxy to the escalate server. - async fn run(self) -> anyhow::Result { - let EscalateArgs { file, argv } = self; - escalate_client::run(file, argv).await - } -} - -/// Debugging command to emulate an MCP "shell" tool call. -#[derive(Parser, Debug)] -struct ShellExecArgs { - command: String, + /// Path to Bash that has been patched to support execve() wrapping. + #[arg(long = "bash")] + bash_path: Option, } #[tokio::main] -pub async fn main() -> anyhow::Result<()> { - let cli = Cli::parse(); +pub async fn main_mcp_server() -> anyhow::Result<()> { tracing_subscriber::fmt() .with_env_filter(EnvFilter::from_default_env()) .with_writer(std::io::stderr) .with_ansi(false) .init(); - match cli.subcommand { - Some(Commands::Escalate(args)) => { - std::process::exit(args.run().await?); - } - Some(Commands::ShellExec(args)) => { - let bash_path = mcp::get_bash_path()?; - let escalate_server = EscalateServer::new(bash_path, DummyEscalationPolicy {}); - let result = escalate_server - .exec( - args.command.clone(), - std::env::vars().collect(), - std::env::current_dir()?, - None, - ) - .await?; - println!("{result:?}"); - std::process::exit(result.exit_code); - } + let cli = McpServerCli::parse(); + let execve_wrapper = match cli.execve_wrapper { + Some(path) => path, None => { - let bash_path = mcp::get_bash_path()?; - - tracing::info!("Starting MCP server"); - let service = mcp::serve(bash_path, dummy_exec_policy) - .await - .inspect_err(|e| { - tracing::error!("serving error: {:?}", e); - })?; - - service.waiting().await?; - Ok(()) + let cwd = std::env::current_exe()?; + cwd.parent() + .map(|p| p.join(CODEX_EXECVE_WRAPPER_EXE_NAME)) + .ok_or_else(|| { + anyhow::anyhow!("failed to determine execve wrapper path from current exe") + })? } - } + }; + let bash_path = match cli.bash_path { + Some(path) => path, + None => mcp::get_bash_path()?, + }; + + tracing::info!("Starting MCP server"); + let service = mcp::serve(bash_path, execve_wrapper, dummy_exec_policy) + .await + .inspect_err(|e| { + tracing::error!("serving error: {:?}", e); + })?; + + service.waiting().await?; + Ok(()) +} + +#[derive(Parser)] +pub struct ExecveWrapperCli { + file: String, + + #[arg(trailing_var_arg = true)] + argv: Vec, +} + +#[tokio::main] +pub async fn main_execve_wrapper() -> anyhow::Result<()> { + tracing_subscriber::fmt() + .with_env_filter(EnvFilter::from_default_env()) + .with_writer(std::io::stderr) + .with_ansi(false) + .init(); + + let ExecveWrapperCli { file, argv } = ExecveWrapperCli::parse(); + let exit_code = escalate_client::run(file, argv).await?; + std::process::exit(exit_code); } // TODO: replace with execpolicy2 -struct DummyEscalationPolicy; - -#[async_trait::async_trait] -impl EscalationPolicy for DummyEscalationPolicy { - async fn determine_action( - &self, - file: &Path, - argv: &[String], - workdir: &Path, - ) -> Result { - let outcome = dummy_exec_policy(file, argv, workdir); - let action = match outcome { - ExecPolicyOutcome::Allow { - run_with_escalated_permissions, - } => { - if run_with_escalated_permissions { - EscalateAction::Escalate - } else { - EscalateAction::Run - } - } - ExecPolicyOutcome::Forbidden => EscalateAction::Deny { - reason: Some("Execution forbidden by policy".to_string()), - }, - ExecPolicyOutcome::Prompt { .. } => EscalateAction::Deny { - reason: Some("Could not prompt user for permission".to_string()), - }, - }; - Ok(action) - } -} - fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome { - if file.ends_with("/rm") { + if file.ends_with("rm") { ExecPolicyOutcome::Forbidden - } else if file.ends_with("/git") { + } else if file.ends_with("git") { ExecPolicyOutcome::Prompt { run_with_escalated_permissions: false, } diff --git a/codex-rs/exec-server/src/posix/escalate_server.rs b/codex-rs/exec-server/src/posix/escalate_server.rs index 4f259f0506..1f76687ce0 100644 --- a/codex-rs/exec-server/src/posix/escalate_server.rs +++ b/codex-rs/exec-server/src/posix/escalate_server.rs @@ -27,16 +27,18 @@ use crate::posix::socket::AsyncSocket; pub(crate) struct EscalateServer { bash_path: PathBuf, + execve_wrapper: PathBuf, policy: Arc, } impl EscalateServer { - pub fn new

(bash_path: PathBuf, policy: P) -> Self + pub fn new

(bash_path: PathBuf, execve_wrapper: PathBuf, policy: P) -> Self where P: EscalationPolicy + Send + Sync + 'static, { Self { bash_path, + execve_wrapper, policy: Arc::new(policy), } } @@ -60,8 +62,15 @@ impl EscalateServer { ); env.insert( BASH_EXEC_WRAPPER_ENV_VAR.to_string(), - format!("{} escalate", std::env::current_exe()?.to_string_lossy()), + self.execve_wrapper.to_string_lossy().to_string(), ); + + // TODO: use the sandbox policy and cwd from the calling client. + // Note that sandbox_cwd is ignored for ReadOnly, but needs to be legit + // for `SandboxPolicy::WorkspaceWrite`. + let sandbox_policy = SandboxPolicy::ReadOnly; + let sandbox_cwd = PathBuf::from("/__NONEXISTENT__"); + let result = process_exec_tool_call( codex_core::exec::ExecParams { command: vec![ @@ -77,9 +86,8 @@ impl EscalateServer { arg0: None, }, get_platform_sandbox().unwrap_or(SandboxType::None), - // TODO: use the sandbox policy and cwd from the calling client - &SandboxPolicy::ReadOnly, - &PathBuf::from("/__NONEXISTENT__"), // This is ignored for ReadOnly + &sandbox_policy, + &sandbox_cwd, &None, None, ) diff --git a/codex-rs/exec-server/src/posix/mcp.rs b/codex-rs/exec-server/src/posix/mcp.rs index 2a6e84dd9e..f5785dc5d0 100644 --- a/codex-rs/exec-server/src/posix/mcp.rs +++ b/codex-rs/exec-server/src/posix/mcp.rs @@ -65,15 +65,17 @@ impl From for ExecResult { pub struct ExecTool { tool_router: ToolRouter, bash_path: PathBuf, + execve_wrapper: PathBuf, policy: ExecPolicy, } #[tool_router] impl ExecTool { - pub fn new(bash_path: PathBuf, policy: ExecPolicy) -> Self { + pub fn new(bash_path: PathBuf, execve_wrapper: PathBuf, policy: ExecPolicy) -> Self { Self { tool_router: Self::tool_router(), bash_path, + execve_wrapper, policy, } } @@ -87,6 +89,7 @@ impl ExecTool { ) -> Result { let escalate_server = EscalateServer::new( self.bash_path.clone(), + self.execve_wrapper.clone(), McpEscalationPolicy::new(self.policy, context), ); let result = escalate_server @@ -130,8 +133,9 @@ impl ServerHandler for ExecTool { pub(crate) async fn serve( bash_path: PathBuf, + execve_wrapper: PathBuf, policy: ExecPolicy, ) -> Result, rmcp::service::ServerInitializeError> { - let tool = ExecTool::new(bash_path, policy); + let tool = ExecTool::new(bash_path, execve_wrapper, policy); tool.serve(stdio()).await } From b00a7cf40d7faceab3ed3789c0429d729c91f7b6 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Wed, 19 Nov 2025 16:41:38 -0800 Subject: [PATCH 6/9] fix(shell) fallback shells (#6948) ## Summary Add fallbacks when user_shell_path does not resolve to a known shell type ## Testing - [x] Tests still pass --- codex-rs/core/src/shell.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/codex-rs/core/src/shell.rs b/codex-rs/core/src/shell.rs index 7bfec089c5..1fb2b97dd7 100644 --- a/codex-rs/core/src/shell.rs +++ b/codex-rs/core/src/shell.rs @@ -204,10 +204,21 @@ pub async fn default_user_shell() -> Shell { if cfg!(windows) { get_shell(ShellType::PowerShell, None).unwrap_or(Shell::Unknown) } else { - get_user_shell_path() + let user_default_shell = get_user_shell_path() .and_then(|shell| detect_shell_type(&shell)) - .and_then(|shell_type| get_shell(shell_type, None)) - .unwrap_or(Shell::Unknown) + .and_then(|shell_type| get_shell(shell_type, None)); + + let shell_with_fallback = if cfg!(target_os = "macos") { + user_default_shell + .or_else(|| get_shell(ShellType::Zsh, None)) + .or_else(|| get_shell(ShellType::Bash, None)) + } else { + user_default_shell + .or_else(|| get_shell(ShellType::Bash, None)) + .or_else(|| get_shell(ShellType::Zsh, None)) + }; + + shell_with_fallback.unwrap_or(Shell::Unknown) } } From 65c13f1ae72c6aac8016a63bf9d436547954c9bb Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Wed, 19 Nov 2025 16:50:43 -0800 Subject: [PATCH 7/9] execpolicy2 core integration (#6641) This PR threads execpolicy2 into codex-core. activated via feature flag: exec_policy (on by default) reads and parses all .codexpolicy files in `codex_home/codex` refactored tool runtime API to integrate execpolicy logic --------- Co-authored-by: Michael Bolin --- README.md | 33 ++ codex-rs/Cargo.lock | 1 + codex-rs/Cargo.toml | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 13 + .../command_safety/is_dangerous_command.rs | 7 +- codex-rs/core/src/exec_policy.rs | 365 ++++++++++++++++++ codex-rs/core/src/features.rs | 8 + codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/sandboxing/mod.rs | 22 ++ codex-rs/core/src/tools/handlers/shell.rs | 9 + codex-rs/core/src/tools/orchestrator.rs | 70 ++-- codex-rs/core/src/tools/runtimes/shell.rs | 19 +- .../core/src/tools/runtimes/unified_exec.rs | 21 +- codex-rs/core/src/tools/sandboxing.rs | 52 ++- .../core/src/unified_exec/session_manager.rs | 9 + codex-rs/core/tests/suite/exec_policy.rs | 101 +++++ codex-rs/core/tests/suite/mod.rs | 1 + codex-rs/execpolicy2/README.md | 30 +- codex-rs/execpolicy2/src/policy.rs | 4 + codex-rs/tui/src/chatwidget/agent.rs | 19 +- 21 files changed, 692 insertions(+), 95 deletions(-) create mode 100644 codex-rs/core/src/exec_policy.rs create mode 100644 codex-rs/core/tests/suite/exec_policy.rs diff --git a/README.md b/README.md index 8141610035..e34afbd288 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,39 @@ Codex can access MCP servers. To configure them, refer to the [config docs](./do Codex CLI supports a rich set of configuration options, with preferences stored in `~/.codex/config.toml`. For full configuration options, see [Configuration](./docs/config.md). +### Execpolicy quickstart + +Codex can enforce your own rules-based execution policy before it runs shell commands. + +1. Create a policy directory: `mkdir -p ~/.codex/policy`. +2. Create one or more `.codexpolicy` files into that folder. Codex automatically loads every `.codexpolicy` file in there on startup. +3. Write `prefix_rule` entries to describe the commands you want to allow, prompt, or block: + +```starlark +prefix_rule( + pattern = ["git", ["push", "fetch"]], + decision = "prompt", # allow | prompt | forbidden + match = [["git", "push", "origin", "main"]], # examples that must match + not_match = [["git", "status"]], # examples that must not match +) +``` + +- `pattern` is a list of shell tokens, evaluated from left to right; wrap tokens in a nested list to express alternatives (e.g., match both `push` and `fetch`). +- `decision` sets the severity; Codex picks the strictest decision when multiple rules match. +- `match` and `not_match` act as (optional) unit tests. Codex validates them when it loads your policy, so you get feedback if an example has unexpected behavior. + +In this example rule, if Codex wants to run commands with the prefix `git push` or `git fetch`, it will first ask for user approval. + +Note: If Codex wants to run a command that matches with multiple rules, it will use the strictest decision among the matched rules (forbidden > prompt > allow). + +Use the [`execpolicy2` CLI](./codex-rs/execpolicy2/README.md) to preview decisions before you save a rule: + +```shell +cargo run -p codex-execpolicy2 -- check --policy ~/.codex/policy/default.codexpolicy git push origin main +``` + +Pass multiple `--policy` flags to test how several files combine. See the [`codex-rs/execpolicy2` README](./codex-rs/execpolicy2/README.md) for a more detailed walkthrough of the available syntax. + --- ### Docs & FAQ diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 66a1c208e4..798b8d71b3 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1086,6 +1086,7 @@ dependencies = [ "codex-apply-patch", "codex-arg0", "codex-async-utils", + "codex-execpolicy2", "codex-file-search", "codex-git", "codex-keyring-store", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 2e88aba713..b0cf1e70fb 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -67,6 +67,7 @@ codex-chatgpt = { path = "chatgpt" } codex-common = { path = "common" } codex-core = { path = "core" } codex-exec = { path = "exec" } +codex-execpolicy2 = { path = "execpolicy2" } codex-feedback = { path = "feedback" } codex-file-search = { path = "file-search" } codex-git = { path = "utils/git" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 4d8f43778c..db06f6b795 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -22,6 +22,7 @@ chrono = { workspace = true, features = ["serde"] } codex-app-server-protocol = { workspace = true } codex-apply-patch = { workspace = true } codex-async-utils = { workspace = true } +codex-execpolicy2 = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } codex-keyring-store = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 32589dabb3..897f1f2944 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -121,6 +121,7 @@ use crate::user_instructions::UserInstructions; use crate::user_notification::UserNotification; use crate::util::backoff; use codex_async_utils::OrCancelExt; +use codex_execpolicy2::Policy as ExecPolicy; use codex_otel::otel_event_manager::OtelEventManager; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; @@ -167,6 +168,10 @@ impl Codex { let user_instructions = get_user_instructions(&config).await; + let exec_policy = crate::exec_policy::exec_policy_for(&config.features, &config.codex_home) + .await + .map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?; + let config = Arc::new(config); let session_configuration = SessionConfiguration { @@ -183,6 +188,7 @@ impl Codex { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: config.features.clone(), + exec_policy, session_source, }; @@ -280,6 +286,7 @@ pub(crate) struct TurnContext { pub(crate) final_output_json_schema: Option, pub(crate) codex_linux_sandbox_exe: Option, pub(crate) tool_call_gate: Arc, + pub(crate) exec_policy: Arc, pub(crate) truncation_policy: TruncationPolicy, } @@ -336,6 +343,8 @@ pub(crate) struct SessionConfiguration { /// Set of feature flags for this session features: Features, + /// Execpolicy policy, applied only when enabled by feature flag. + exec_policy: Arc, // TODO(pakrym): Remove config from here original_config_do_not_use: Arc, @@ -436,6 +445,7 @@ impl Session { final_output_json_schema: None, codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), tool_call_gate: Arc::new(ReadinessFlag::new()), + exec_policy: session_configuration.exec_policy.clone(), truncation_policy: TruncationPolicy::new(&per_turn_config), } } @@ -1789,6 +1799,7 @@ async fn spawn_review_thread( final_output_json_schema: None, codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(), tool_call_gate: Arc::new(ReadinessFlag::new()), + exec_policy: parent_turn_context.exec_policy.clone(), truncation_policy: TruncationPolicy::new(&per_turn_config), }; @@ -2608,6 +2619,7 @@ mod tests { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: Features::default(), + exec_policy: Arc::new(codex_execpolicy2::Policy::empty()), session_source: SessionSource::Exec, }; @@ -2685,6 +2697,7 @@ mod tests { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: Features::default(), + exec_policy: Arc::new(codex_execpolicy2::Policy::empty()), session_source: SessionSource::Exec, }; diff --git a/codex-rs/core/src/command_safety/is_dangerous_command.rs b/codex-rs/core/src/command_safety/is_dangerous_command.rs index 09594bb1c5..5df2023f06 100644 --- a/codex-rs/core/src/command_safety/is_dangerous_command.rs +++ b/codex-rs/core/src/command_safety/is_dangerous_command.rs @@ -1,6 +1,8 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; +use crate::sandboxing::SandboxPermissions; + use crate::bash::parse_shell_lc_plain_commands; use crate::is_safe_command::is_known_safe_command; @@ -8,7 +10,7 @@ pub fn requires_initial_appoval( policy: AskForApproval, sandbox_policy: &SandboxPolicy, command: &[String], - with_escalated_permissions: bool, + sandbox_permissions: SandboxPermissions, ) -> bool { if is_known_safe_command(command) { return false; @@ -24,8 +26,7 @@ pub fn requires_initial_appoval( // In restricted sandboxes (ReadOnly/WorkspaceWrite), do not prompt for // non‑escalated, non‑dangerous commands — let the sandbox enforce // restrictions (e.g., block network/write) without a user prompt. - let wants_escalation: bool = with_escalated_permissions; - if wants_escalation { + if sandbox_permissions.requires_escalated_permissions() { return true; } command_might_be_dangerous(command) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs new file mode 100644 index 0000000000..c55dbfed23 --- /dev/null +++ b/codex-rs/core/src/exec_policy.rs @@ -0,0 +1,365 @@ +use std::io::ErrorKind; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +use crate::command_safety::is_dangerous_command::requires_initial_appoval; +use codex_execpolicy2::Decision; +use codex_execpolicy2::Evaluation; +use codex_execpolicy2::Policy; +use codex_execpolicy2::PolicyParser; +use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::SandboxPolicy; +use thiserror::Error; +use tokio::fs; + +use crate::bash::parse_shell_lc_plain_commands; +use crate::features::Feature; +use crate::features::Features; +use crate::sandboxing::SandboxPermissions; +use crate::tools::sandboxing::ApprovalRequirement; + +const FORBIDDEN_REASON: &str = "execpolicy forbids this command"; +const PROMPT_REASON: &str = "execpolicy requires approval for this command"; +const POLICY_DIR_NAME: &str = "policy"; +const POLICY_EXTENSION: &str = "codexpolicy"; + +#[derive(Debug, Error)] +pub enum ExecPolicyError { + #[error("failed to read execpolicy files from {dir}: {source}")] + ReadDir { + dir: PathBuf, + source: std::io::Error, + }, + + #[error("failed to read execpolicy file {path}: {source}")] + ReadFile { + path: PathBuf, + source: std::io::Error, + }, + + #[error("failed to parse execpolicy file {path}: {source}")] + ParsePolicy { + path: String, + source: codex_execpolicy2::Error, + }, +} + +pub(crate) async fn exec_policy_for( + features: &Features, + codex_home: &Path, +) -> Result, ExecPolicyError> { + if !features.enabled(Feature::ExecPolicy) { + return Ok(Arc::new(Policy::empty())); + } + + let policy_dir = codex_home.join(POLICY_DIR_NAME); + let policy_paths = collect_policy_files(&policy_dir).await?; + + let mut parser = PolicyParser::new(); + for policy_path in &policy_paths { + let contents = + fs::read_to_string(policy_path) + .await + .map_err(|source| ExecPolicyError::ReadFile { + path: policy_path.clone(), + source, + })?; + let identifier = policy_path.to_string_lossy().to_string(); + parser + .parse(&identifier, &contents) + .map_err(|source| ExecPolicyError::ParsePolicy { + path: identifier, + source, + })?; + } + + let policy = Arc::new(parser.build()); + tracing::debug!( + "loaded execpolicy from {} files in {}", + policy_paths.len(), + policy_dir.display() + ); + + Ok(policy) +} + +fn evaluate_with_policy( + policy: &Policy, + command: &[String], + approval_policy: AskForApproval, +) -> Option { + let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); + let evaluation = policy.check_multiple(commands.iter()); + + match evaluation { + Evaluation::Match { decision, .. } => match decision { + Decision::Forbidden => Some(ApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string(), + }), + Decision::Prompt => { + let reason = PROMPT_REASON.to_string(); + if matches!(approval_policy, AskForApproval::Never) { + Some(ApprovalRequirement::Forbidden { reason }) + } else { + Some(ApprovalRequirement::NeedsApproval { + reason: Some(reason), + }) + } + } + Decision::Allow => Some(ApprovalRequirement::Skip), + }, + Evaluation::NoMatch => None, + } +} + +pub(crate) fn create_approval_requirement_for_command( + policy: &Policy, + command: &[String], + approval_policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + sandbox_permissions: SandboxPermissions, +) -> ApprovalRequirement { + if let Some(requirement) = evaluate_with_policy(policy, command, approval_policy) { + return requirement; + } + + if requires_initial_appoval( + approval_policy, + sandbox_policy, + command, + sandbox_permissions, + ) { + ApprovalRequirement::NeedsApproval { reason: None } + } else { + ApprovalRequirement::Skip + } +} + +async fn collect_policy_files(dir: &Path) -> Result, ExecPolicyError> { + let mut read_dir = match fs::read_dir(dir).await { + Ok(read_dir) => read_dir, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()), + Err(source) => { + return Err(ExecPolicyError::ReadDir { + dir: dir.to_path_buf(), + source, + }); + } + }; + + let mut policy_paths = Vec::new(); + while let Some(entry) = + read_dir + .next_entry() + .await + .map_err(|source| ExecPolicyError::ReadDir { + dir: dir.to_path_buf(), + source, + })? + { + let path = entry.path(); + let file_type = entry + .file_type() + .await + .map_err(|source| ExecPolicyError::ReadDir { + dir: dir.to_path_buf(), + source, + })?; + + if path + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext == POLICY_EXTENSION) + && file_type.is_file() + { + policy_paths.push(path); + } + } + + policy_paths.sort(); + + Ok(policy_paths) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::features::Feature; + use crate::features::Features; + use codex_protocol::protocol::AskForApproval; + use codex_protocol::protocol::SandboxPolicy; + use pretty_assertions::assert_eq; + use std::fs; + use tempfile::tempdir; + + #[tokio::test] + async fn returns_empty_policy_when_feature_disabled() { + let mut features = Features::with_defaults(); + features.disable(Feature::ExecPolicy); + let temp_dir = tempdir().expect("create temp dir"); + + let policy = exec_policy_for(&features, temp_dir.path()) + .await + .expect("policy result"); + + let commands = [vec!["rm".to_string()]]; + assert!(matches!( + policy.check_multiple(commands.iter()), + Evaluation::NoMatch + )); + assert!(!temp_dir.path().join(POLICY_DIR_NAME).exists()); + } + + #[tokio::test] + async fn collect_policy_files_returns_empty_when_dir_missing() { + let temp_dir = tempdir().expect("create temp dir"); + + let policy_dir = temp_dir.path().join(POLICY_DIR_NAME); + let files = collect_policy_files(&policy_dir) + .await + .expect("collect policy files"); + + assert!(files.is_empty()); + } + + #[tokio::test] + async fn loads_policies_from_policy_subdirectory() { + let temp_dir = tempdir().expect("create temp dir"); + let policy_dir = temp_dir.path().join(POLICY_DIR_NAME); + fs::create_dir_all(&policy_dir).expect("create policy dir"); + fs::write( + policy_dir.join("deny.codexpolicy"), + r#"prefix_rule(pattern=["rm"], decision="forbidden")"#, + ) + .expect("write policy file"); + + let policy = exec_policy_for(&Features::with_defaults(), temp_dir.path()) + .await + .expect("policy result"); + let command = [vec!["rm".to_string()]]; + assert!(matches!( + policy.check_multiple(command.iter()), + Evaluation::Match { .. } + )); + } + + #[tokio::test] + async fn ignores_policies_outside_policy_dir() { + let temp_dir = tempdir().expect("create temp dir"); + fs::write( + temp_dir.path().join("root.codexpolicy"), + r#"prefix_rule(pattern=["ls"], decision="prompt")"#, + ) + .expect("write policy file"); + + let policy = exec_policy_for(&Features::with_defaults(), temp_dir.path()) + .await + .expect("policy result"); + let command = [vec!["ls".to_string()]]; + assert!(matches!( + policy.check_multiple(command.iter()), + Evaluation::NoMatch + )); + } + + #[test] + fn evaluates_bash_lc_inner_commands() { + let policy_src = r#" +prefix_rule(pattern=["rm"], decision="forbidden") +"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + + let forbidden_script = vec![ + "bash".to_string(), + "-lc".to_string(), + "rm -rf /tmp".to_string(), + ]; + + let requirement = + evaluate_with_policy(&policy, &forbidden_script, AskForApproval::OnRequest) + .expect("expected match for forbidden command"); + + assert_eq!( + requirement, + ApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string() + } + ); + } + + #[test] + fn approval_requirement_prefers_execpolicy_match() { + let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + let command = vec!["rm".to_string()]; + + let requirement = create_approval_requirement_for_command( + &policy, + &command, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { + reason: Some(PROMPT_REASON.to_string()) + } + ); + } + + #[test] + fn approval_requirement_respects_approval_policy() { + let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", policy_src) + .expect("parse policy"); + let policy = parser.build(); + let command = vec!["rm".to_string()]; + + let requirement = create_approval_requirement_for_command( + &policy, + &command, + AskForApproval::Never, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::Forbidden { + reason: PROMPT_REASON.to_string() + } + ); + } + + #[test] + fn approval_requirement_falls_back_to_heuristics() { + let command = vec!["python".to_string()]; + + let empty_policy = Policy::empty(); + let requirement = create_approval_requirement_for_command( + &empty_policy, + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { reason: None } + ); + } +} diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 1a7fa21b45..75622e5254 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -42,6 +42,8 @@ pub enum Feature { ViewImageTool, /// Allow the model to request web searches. WebSearchRequest, + /// Gate the execpolicy enforcement for shell/unified exec. + ExecPolicy, /// Enable the model-based risk assessments for sandboxed commands. SandboxCommandAssessment, /// Enable Windows sandbox (restricted token) on Windows. @@ -297,6 +299,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: false, }, + FeatureSpec { + id: Feature::ExecPolicy, + key: "exec_policy", + stage: Stage::Experimental, + default_enabled: true, + }, FeatureSpec { id: Feature::SandboxCommandAssessment, key: "experimental_sandbox_command_assessment", diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 3e74633456..2ae11d79d0 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -25,6 +25,7 @@ mod environment_context; pub mod error; pub mod exec; pub mod exec_env; +mod exec_policy; pub mod features; mod flags; pub mod git_info; diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 5e564f5109..4ecb2a8c12 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -26,6 +26,28 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum SandboxPermissions { + UseDefault, + RequireEscalated, +} + +impl SandboxPermissions { + pub fn requires_escalated_permissions(self) -> bool { + matches!(self, SandboxPermissions::RequireEscalated) + } +} + +impl From for SandboxPermissions { + fn from(with_escalated_permissions: bool) -> Self { + if with_escalated_permissions { + SandboxPermissions::RequireEscalated + } else { + SandboxPermissions::UseDefault + } + } +} + #[derive(Clone, Debug)] pub struct CommandSpec { pub program: String, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b7d6f7511d..fcd2f5b0c9 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -9,9 +9,11 @@ use crate::apply_patch::convert_apply_patch_to_protocol; use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; +use crate::exec_policy::create_approval_requirement_for_command; use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; use crate::protocol::ExecCommandSource; +use crate::sandboxing::SandboxPermissions; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -302,6 +304,13 @@ impl ShellHandler { env: exec_params.env.clone(), with_escalated_permissions: exec_params.with_escalated_permissions, justification: exec_params.justification.clone(), + approval_requirement: create_approval_requirement_for_command( + &turn.exec_policy, + &exec_params.command, + turn.approval_policy, + &turn.sandbox_policy, + SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)), + ), }; let mut orchestrator = ToolOrchestrator::new(); let mut runtime = ShellRuntime::new(); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 878e48e8be..7e8e152f67 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -11,11 +11,13 @@ use crate::error::get_error_message_ui; use crate::exec::ExecToolCallOutput; use crate::sandboxing::SandboxManager; use crate::tools::sandboxing::ApprovalCtx; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; +use crate::tools::sandboxing::default_approval_requirement; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; @@ -49,40 +51,52 @@ impl ToolOrchestrator { let otel_cfg = codex_otel::otel_event_manager::ToolDecisionSource::Config; // 1) Approval - let needs_initial_approval = - tool.wants_initial_approval(req, approval_policy, &turn_ctx.sandbox_policy); let mut already_approved = false; - if needs_initial_approval { - let mut risk = None; - - if let Some(metadata) = req.sandbox_retry_data() { - risk = tool_ctx - .session - .assess_sandbox_command(turn_ctx, &tool_ctx.call_id, &metadata.command, None) - .await; + let requirement = tool.approval_requirement(req).unwrap_or_else(|| { + default_approval_requirement(approval_policy, &turn_ctx.sandbox_policy) + }); + match requirement { + ApprovalRequirement::Skip => { + otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg); } + ApprovalRequirement::Forbidden { reason } => { + return Err(ToolError::Rejected(reason)); + } + ApprovalRequirement::NeedsApproval { reason } => { + let mut risk = None; - let approval_ctx = ApprovalCtx { - session: tool_ctx.session, - turn: turn_ctx, - call_id: &tool_ctx.call_id, - retry_reason: None, - risk, - }; - let decision = tool.start_approval_async(req, approval_ctx).await; - - otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone()); - - match decision { - ReviewDecision::Denied | ReviewDecision::Abort => { - return Err(ToolError::Rejected("rejected by user".to_string())); + if let Some(metadata) = req.sandbox_retry_data() { + risk = tool_ctx + .session + .assess_sandbox_command( + turn_ctx, + &tool_ctx.call_id, + &metadata.command, + None, + ) + .await; } - ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {} + + let approval_ctx = ApprovalCtx { + session: tool_ctx.session, + turn: turn_ctx, + call_id: &tool_ctx.call_id, + retry_reason: reason, + risk, + }; + let decision = tool.start_approval_async(req, approval_ctx).await; + + otel.tool_decision(otel_tn, otel_ci, decision, otel_user.clone()); + + match decision { + ReviewDecision::Denied | ReviewDecision::Abort => { + return Err(ToolError::Rejected("rejected by user".to_string())); + } + ReviewDecision::Approved | ReviewDecision::ApprovedForSession => {} + } + already_approved = true; } - already_approved = true; - } else { - otel.tool_decision(otel_tn, otel_ci, ReviewDecision::Approved, otel_cfg); } // 2) First attempt under the selected sandbox. diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index bf7ae7fa3b..d71c4498e6 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -4,13 +4,12 @@ Runtime: shell Executes shell requests under the orchestrator: asks for approval when needed, builds a CommandSpec, and runs it under the current SandboxAttempt. */ -use crate::command_safety::is_dangerous_command::requires_initial_appoval; use crate::exec::ExecToolCallOutput; -use crate::protocol::SandboxPolicy; use crate::sandboxing::execute_env; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxRetryData; @@ -20,7 +19,6 @@ use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; -use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::path::PathBuf; @@ -33,6 +31,7 @@ pub struct ShellRequest { pub env: std::collections::HashMap, pub with_escalated_permissions: Option, pub justification: Option, + pub approval_requirement: ApprovalRequirement, } impl ProvidesSandboxRetryData for ShellRequest { @@ -114,18 +113,8 @@ impl Approvable for ShellRuntime { }) } - fn wants_initial_approval( - &self, - req: &ShellRequest, - policy: AskForApproval, - sandbox_policy: &SandboxPolicy, - ) -> bool { - requires_initial_appoval( - policy, - sandbox_policy, - &req.command, - req.with_escalated_permissions.unwrap_or(false), - ) + fn approval_requirement(&self, req: &ShellRequest) -> Option { + Some(req.approval_requirement.clone()) } fn wants_escalated_first_attempt(&self, req: &ShellRequest) -> bool { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index cddac1924e..5b18476bfc 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -1,4 +1,3 @@ -use crate::command_safety::is_dangerous_command::requires_initial_appoval; /* Runtime: unified exec @@ -10,6 +9,7 @@ use crate::error::SandboxErr; use crate::tools::runtimes::build_command_spec; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; +use crate::tools::sandboxing::ApprovalRequirement; use crate::tools::sandboxing::ProvidesSandboxRetryData; use crate::tools::sandboxing::SandboxAttempt; use crate::tools::sandboxing::SandboxRetryData; @@ -22,9 +22,7 @@ use crate::tools::sandboxing::with_cached_approval; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecSession; use crate::unified_exec::UnifiedExecSessionManager; -use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; -use codex_protocol::protocol::SandboxPolicy; use futures::future::BoxFuture; use std::collections::HashMap; use std::path::PathBuf; @@ -36,6 +34,7 @@ pub struct UnifiedExecRequest { pub env: HashMap, pub with_escalated_permissions: Option, pub justification: Option, + pub approval_requirement: ApprovalRequirement, } impl ProvidesSandboxRetryData for UnifiedExecRequest { @@ -65,6 +64,7 @@ impl UnifiedExecRequest { env: HashMap, with_escalated_permissions: Option, justification: Option, + approval_requirement: ApprovalRequirement, ) -> Self { Self { command, @@ -72,6 +72,7 @@ impl UnifiedExecRequest { env, with_escalated_permissions, justification, + approval_requirement, } } } @@ -129,18 +130,8 @@ impl Approvable for UnifiedExecRuntime<'_> { }) } - fn wants_initial_approval( - &self, - req: &UnifiedExecRequest, - policy: AskForApproval, - sandbox_policy: &SandboxPolicy, - ) -> bool { - requires_initial_appoval( - policy, - sandbox_policy, - &req.command, - req.with_escalated_permissions.unwrap_or(false), - ) + fn approval_requirement(&self, req: &UnifiedExecRequest) -> Option { + Some(req.approval_requirement.clone()) } fn wants_escalated_first_attempt(&self, req: &UnifiedExecRequest) -> bool { diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index da1c22b549..e694c7fbef 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -86,6 +86,37 @@ pub(crate) struct ApprovalCtx<'a> { pub risk: Option, } +// Specifies what tool orchestrator should do with a given tool call. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum ApprovalRequirement { + /// No approval required for this tool call + Skip, + /// Approval required for this tool call + NeedsApproval { reason: Option }, + /// Execution forbidden for this tool call + Forbidden { reason: String }, +} + +/// - Never, OnFailure: do not ask +/// - OnRequest: ask unless sandbox policy is DangerFullAccess +/// - UnlessTrusted: always ask +pub(crate) fn default_approval_requirement( + policy: AskForApproval, + sandbox_policy: &SandboxPolicy, +) -> ApprovalRequirement { + let needs_approval = match policy { + AskForApproval::Never | AskForApproval::OnFailure => false, + AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess), + AskForApproval::UnlessTrusted => true, + }; + + if needs_approval { + ApprovalRequirement::NeedsApproval { reason: None } + } else { + ApprovalRequirement::Skip + } +} + pub(crate) trait Approvable { type ApprovalKey: Hash + Eq + Clone + Debug + Serialize; @@ -106,22 +137,11 @@ pub(crate) trait Approvable { matches!(policy, AskForApproval::Never) } - /// Decide whether an initial user approval should be requested before the - /// first attempt. Defaults to the orchestrator's behavior (pre‑refactor): - /// - Never, OnFailure: do not ask - /// - OnRequest: ask unless sandbox policy is DangerFullAccess - /// - UnlessTrusted: always ask - fn wants_initial_approval( - &self, - _req: &Req, - policy: AskForApproval, - sandbox_policy: &SandboxPolicy, - ) -> bool { - match policy { - AskForApproval::Never | AskForApproval::OnFailure => false, - AskForApproval::OnRequest => !matches!(sandbox_policy, SandboxPolicy::DangerFullAccess), - AskForApproval::UnlessTrusted => true, - } + /// Override the default approval requirement. Return `Some(_)` to specify + /// a custom requirement, or `None` to fall back to + /// policy-based default. + fn approval_requirement(&self, _req: &Req) -> Option { + None } /// Decide we can request an approval for no-sandbox execution. diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index f6dce8644c..93340bb2d4 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -11,10 +11,12 @@ use crate::codex::TurnContext; use crate::exec::ExecToolCallOutput; use crate::exec::StreamOutput; use crate::exec_env::create_env; +use crate::exec_policy::create_approval_requirement_for_command; use crate::protocol::BackgroundEventEvent; use crate::protocol::EventMsg; use crate::protocol::ExecCommandSource; use crate::sandboxing::ExecEnv; +use crate::sandboxing::SandboxPermissions; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::events::ToolEventFailure; @@ -449,6 +451,13 @@ impl UnifiedExecSessionManager { create_env(&context.turn.shell_environment_policy), with_escalated_permissions, justification, + create_approval_requirement_for_command( + &context.turn.exec_policy, + command, + context.turn.approval_policy, + &context.turn.sandbox_policy, + SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)), + ), ); let tool_ctx = ToolCtx { session: context.session.as_ref(), diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs new file mode 100644 index 0000000000..bca917a252 --- /dev/null +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -0,0 +1,101 @@ +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use anyhow::Result; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::EventMsg; +use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::user_input::UserInput; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_function_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use serde_json::json; +use std::fs; + +#[tokio::test] +async fn execpolicy_blocks_shell_invocation() -> Result<()> { + let mut builder = test_codex().with_config(|config| { + let policy_path = config.codex_home.join("policy").join("policy.codexpolicy"); + fs::create_dir_all( + policy_path + .parent() + .expect("policy directory must have a parent"), + ) + .expect("create policy directory"); + fs::write( + &policy_path, + r#"prefix_rule(pattern=["echo"], decision="forbidden")"#, + ) + .expect("write policy file"); + }); + let server = start_mock_server().await; + let test = builder.build(&server).await?; + + let call_id = "shell-forbidden"; + let args = json!({ + "command": ["echo", "blocked"], + "timeout_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_completed("resp-1"), + ]), + ) + .await; + mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + let session_model = test.session_configured.model.clone(); + test.codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "run shell command".into(), + }], + final_output_json_schema: None, + cwd: test.cwd_path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let EventMsg::ExecCommandEnd(end) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::ExecCommandEnd(_)) + }) + .await + else { + unreachable!() + }; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TaskComplete(_)) + }) + .await; + + assert!( + end.aggregated_output + .contains("execpolicy forbids this command"), + "unexpected output: {}", + end.aggregated_output + ); + + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index ef248901e5..60b828b1c3 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -28,6 +28,7 @@ mod compact_remote; mod compact_resume_fork; mod deprecation_notice; mod exec; +mod exec_policy; mod fork_conversation; mod grep_files; mod items; diff --git a/codex-rs/execpolicy2/README.md b/codex-rs/execpolicy2/README.md index 8cf5302fb6..d77ec3838d 100644 --- a/codex-rs/execpolicy2/README.md +++ b/codex-rs/execpolicy2/README.md @@ -5,7 +5,7 @@ - This release covers only the prefix-rule subset of the planned execpolicy v2 language; a richer language will follow. - Tokens are matched in order; any `pattern` element may be a list to denote alternatives. `decision` defaults to `allow`; valid values: `allow`, `prompt`, `forbidden`. - `match` / `not_match` supply example invocations that are validated at load time (think of them as unit tests); examples can be token arrays or strings (strings are tokenized with `shlex`). -- The CLI always prints the JSON serialization of the evaluation result (whether a match or not). +- The CLI always prints the JSON serialization of the evaluation result. ## Policy shapes - Prefix rules use Starlark syntax: @@ -18,6 +18,20 @@ prefix_rule( ) ``` +## CLI +- Provide one or more policy files (for example `src/default.codexpolicy`) to check a command: +```bash +cargo run -p codex-execpolicy2 -- check --policy path/to/policy.codexpolicy git status +``` +- Pass multiple `--policy` flags to merge rules, evaluated in the order provided: +```bash +cargo run -p codex-execpolicy2 -- check --policy base.codexpolicy --policy overrides.codexpolicy git status +``` +- Output is JSON by default; pass `--pretty` for pretty-printed JSON +- Example outcomes: + - Match: `{"match": { ... "decision": "allow" ... }}` + - No match: `"noMatch"` + ## Response shapes - Match: ```json @@ -43,17 +57,3 @@ prefix_rule( - `matchedRules` lists every rule whose prefix matched the command; `matchedPrefix` is the exact prefix that matched. - The effective `decision` is the strictest severity across all matches (`forbidden` > `prompt` > `allow`). - -## CLI -- Provide one or more policy files (for example `src/default.codexpolicy`) to check a command: -```bash -cargo run -p codex-execpolicy2 -- check --policy path/to/policy.codexpolicy git status -``` -- Pass multiple `--policy` flags to merge rules, evaluated in the order provided: -```bash -cargo run -p codex-execpolicy2 -- check --policy base.codexpolicy --policy overrides.codexpolicy git status -``` -- Output is newline-delimited JSON by default; pass `--pretty` for pretty-printed JSON if desired. -- Example outcomes: - - Match: `{"match": { ... "decision": "allow" ... }}` - - No match: `"noMatch"` diff --git a/codex-rs/execpolicy2/src/policy.rs b/codex-rs/execpolicy2/src/policy.rs index 12416b050b..a543e5c4b0 100644 --- a/codex-rs/execpolicy2/src/policy.rs +++ b/codex-rs/execpolicy2/src/policy.rs @@ -15,6 +15,10 @@ impl Policy { Self { rules_by_program } } + pub fn empty() -> Self { + Self::new(MultiMap::new()) + } + pub fn rules(&self) -> &MultiMap { &self.rules_by_program } diff --git a/codex-rs/tui/src/chatwidget/agent.rs b/codex-rs/tui/src/chatwidget/agent.rs index bf15b6c4ad..0abddac50d 100644 --- a/codex-rs/tui/src/chatwidget/agent.rs +++ b/codex-rs/tui/src/chatwidget/agent.rs @@ -4,6 +4,9 @@ use codex_core::CodexConversation; use codex_core::ConversationManager; use codex_core::NewConversation; use codex_core::config::Config; +use codex_core::protocol::ErrorEvent; +use codex_core::protocol::Event; +use codex_core::protocol::EventMsg; use codex_core::protocol::Op; use tokio::sync::mpsc::UnboundedSender; use tokio::sync::mpsc::unbounded_channel; @@ -28,9 +31,19 @@ pub(crate) fn spawn_agent( session_configured, } = match server.new_conversation(config).await { Ok(v) => v, - Err(e) => { - // TODO: surface this error to the user. - tracing::error!("failed to initialize codex: {e}"); + #[allow(clippy::print_stderr)] + Err(err) => { + let message = err.to_string(); + eprintln!("{message}"); + app_event_tx_clone.send(AppEvent::CodexEvent(Event { + id: "".to_string(), + msg: EventMsg::Error(ErrorEvent { + message, + http_status_code: None, + }), + })); + app_event_tx_clone.send(AppEvent::ExitRequest); + tracing::error!("failed to initialize codex: {err}"); return; } }; From 92e304673374c5da74673ee0a80ca3169897830e Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 19 Nov 2025 16:56:37 -0800 Subject: [PATCH 8/9] Single pass truncation (#6914) --- codex-rs/core/src/truncate.rs | 114 +++++++++++++++++++++++----------- 1 file changed, 79 insertions(+), 35 deletions(-) diff --git a/codex-rs/core/src/truncate.rs b/codex-rs/core/src/truncate.rs index 7756b0c09a..bf883c0612 100644 --- a/codex-rs/core/src/truncate.rs +++ b/codex-rs/core/src/truncate.rs @@ -185,6 +185,7 @@ fn truncate_with_byte_estimate(s: &str, policy: TruncationPolicy) -> String { if s.is_empty() { return String::new(); } + let total_chars = s.chars().count(); let max_bytes = policy.byte_budget(); @@ -204,24 +205,55 @@ fn truncate_with_byte_estimate(s: &str, policy: TruncationPolicy) -> String { let total_bytes = s.len(); let (left_budget, right_budget) = split_budget(max_bytes); - let prefix_end = pick_prefix_end(s, left_budget); - let mut suffix_start = pick_suffix_start(s, right_budget); - if suffix_start < prefix_end { - suffix_start = prefix_end; - } - let left_chars = s[..prefix_end].chars().count(); - let right_chars = s[suffix_start..].chars().count(); - let removed_chars = total_chars - .saturating_sub(left_chars) - .saturating_sub(right_chars); + let (removed_chars, left, right) = split_string(s, left_budget, right_budget); let marker = format_truncation_marker( policy, removed_units_for_source(policy, total_bytes.saturating_sub(max_bytes), removed_chars), ); - assemble_truncated_output(&s[..prefix_end], &s[suffix_start..], &marker) + assemble_truncated_output(left, right, &marker) +} + +fn split_string(s: &str, beginning_bytes: usize, end_bytes: usize) -> (usize, &str, &str) { + if s.is_empty() { + return (0, "", ""); + } + + let len = s.len(); + let tail_start_target = len.saturating_sub(end_bytes); + let mut prefix_end = 0usize; + let mut suffix_start = len; + let mut removed_chars = 0usize; + let mut suffix_started = false; + + for (idx, ch) in s.char_indices() { + let char_end = idx + ch.len_utf8(); + if char_end <= beginning_bytes { + prefix_end = char_end; + continue; + } + + if idx >= tail_start_target { + if !suffix_started { + suffix_start = idx; + suffix_started = true; + } + continue; + } + + removed_chars = removed_chars.saturating_add(1); + } + + if suffix_start < prefix_end { + suffix_start = prefix_end; + } + + let before = &s[..prefix_end]; + let after = &s[suffix_start..]; + + (removed_chars, before, after) } fn format_truncation_marker(policy: TruncationPolicy, removed_count: u64) -> String { @@ -270,42 +302,54 @@ fn approx_tokens_from_byte_count(bytes: usize) -> u64 { / (APPROX_BYTES_PER_TOKEN as u64) } -fn truncate_on_boundary(input: &str, max_len: usize) -> &str { - if input.len() <= max_len { - return input; - } - let mut end = max_len; - while end > 0 && !input.is_char_boundary(end) { - end -= 1; - } - &input[..end] -} - -fn pick_prefix_end(s: &str, left_budget: usize) -> usize { - truncate_on_boundary(s, left_budget).len() -} - -fn pick_suffix_start(s: &str, right_budget: usize) -> usize { - let start_tail = s.len().saturating_sub(right_budget); - let mut idx = start_tail.min(s.len()); - while idx < s.len() && !s.is_char_boundary(idx) { - idx += 1; - } - idx -} - #[cfg(test)] mod tests { use super::TruncationPolicy; use super::approx_token_count; use super::formatted_truncate_text; + use super::split_string; use super::truncate_function_output_items_with_policy; use super::truncate_text; use super::truncate_with_token_budget; use codex_protocol::models::FunctionCallOutputContentItem; use pretty_assertions::assert_eq; + #[test] + fn split_string_works() { + assert_eq!(split_string("hello world", 5, 5), (1, "hello", "world")); + assert_eq!(split_string("abc", 0, 0), (3, "", "")); + } + + #[test] + fn split_string_handles_empty_string() { + assert_eq!(split_string("", 4, 4), (0, "", "")); + } + + #[test] + fn split_string_only_keeps_prefix_when_tail_budget_is_zero() { + assert_eq!(split_string("abcdef", 3, 0), (3, "abc", "")); + } + + #[test] + fn split_string_only_keeps_suffix_when_prefix_budget_is_zero() { + assert_eq!(split_string("abcdef", 0, 3), (3, "", "def")); + } + + #[test] + fn split_string_handles_overlapping_budgets_without_removal() { + assert_eq!(split_string("abcdef", 4, 4), (0, "abcd", "ef")); + } + + #[test] + fn split_string_respects_utf8_boundaries() { + assert_eq!(split_string("😀abc😀", 5, 5), (1, "😀a", "c😀")); + + assert_eq!(split_string("😀😀😀😀😀", 1, 1), (5, "", "")); + assert_eq!(split_string("😀😀😀😀😀", 7, 7), (3, "😀", "😀")); + assert_eq!(split_string("😀😀😀😀😀", 8, 8), (1, "😀😀", "😀😀")); + } + #[test] fn truncate_bytes_less_than_placeholder_returns_placeholder() { let content = "example output"; From 74a75679d993908ee90d7194aec02386d507c8de Mon Sep 17 00:00:00 2001 From: zhao-oai Date: Wed, 19 Nov 2025 16:57:27 -0800 Subject: [PATCH 9/9] update execpolicy quickstart readme (#6952) --- README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index e34afbd288..4fed276405 100644 --- a/README.md +++ b/README.md @@ -69,12 +69,12 @@ Codex can access MCP servers. To configure them, refer to the [config docs](./do Codex CLI supports a rich set of configuration options, with preferences stored in `~/.codex/config.toml`. For full configuration options, see [Configuration](./docs/config.md). -### Execpolicy quickstart +### Execpolicy Quickstart Codex can enforce your own rules-based execution policy before it runs shell commands. 1. Create a policy directory: `mkdir -p ~/.codex/policy`. -2. Create one or more `.codexpolicy` files into that folder. Codex automatically loads every `.codexpolicy` file in there on startup. +2. Create one or more `.codexpolicy` files in that folder. Codex automatically loads every `.codexpolicy` file in there on startup. 3. Write `prefix_rule` entries to describe the commands you want to allow, prompt, or block: ```starlark @@ -87,14 +87,12 @@ prefix_rule( ``` - `pattern` is a list of shell tokens, evaluated from left to right; wrap tokens in a nested list to express alternatives (e.g., match both `push` and `fetch`). -- `decision` sets the severity; Codex picks the strictest decision when multiple rules match. +- `decision` sets the severity; Codex picks the strictest decision when multiple rules match (forbidden > prompt > allow). - `match` and `not_match` act as (optional) unit tests. Codex validates them when it loads your policy, so you get feedback if an example has unexpected behavior. In this example rule, if Codex wants to run commands with the prefix `git push` or `git fetch`, it will first ask for user approval. -Note: If Codex wants to run a command that matches with multiple rules, it will use the strictest decision among the matched rules (forbidden > prompt > allow). - -Use the [`execpolicy2` CLI](./codex-rs/execpolicy2/README.md) to preview decisions before you save a rule: +Use [`execpolicy2` CLI](./codex-rs/execpolicy2/README.md) to preview decisions for policy files: ```shell cargo run -p codex-execpolicy2 -- check --policy ~/.codex/policy/default.codexpolicy git push origin main