From 24a9b7ba28b77c331b41191be92ef209633af46e Mon Sep 17 00:00:00 2001 From: kevin zhao Date: Thu, 13 Nov 2025 21:04:27 -0500 Subject: [PATCH] first pass at integrating execpolicy2 into codex --- codex-rs/Cargo.lock | 1 + codex-rs/Cargo.toml | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/codex.rs | 12 + codex-rs/core/src/exec_policy.rs | 230 ++++++++++++++++++ codex-rs/core/src/features.rs | 8 + codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/tools/handlers/shell.rs | 5 + codex-rs/core/src/tools/orchestrator.rs | 66 ++--- codex-rs/core/src/tools/runtimes/shell.rs | 122 +++++++++- .../core/src/tools/runtimes/unified_exec.rs | 37 ++- codex-rs/core/src/tools/sandboxing.rs | 24 +- .../core/src/unified_exec/session_manager.rs | 1 + codex-rs/core/tests/suite/execpolicy2.rs | 78 ++++++ codex-rs/core/tests/suite/mod.rs | 2 + 15 files changed, 537 insertions(+), 52 deletions(-) create mode 100644 codex-rs/core/src/exec_policy.rs create mode 100644 codex-rs/core/tests/suite/execpolicy2.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e98e937929..b12999885e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1070,6 +1070,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 525f8695ba..1dd0fc193d 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -64,6 +64,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 ab732c910c..2484099a33 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -25,6 +25,7 @@ codex-async-utils = { workspace = true } codex-file-search = { workspace = true } codex-git = { workspace = true } codex-keyring-store = { workspace = true } +codex-execpolicy2 = { workspace = true } codex-otel = { workspace = true, features = ["otel"] } codex-protocol = { workspace = true } codex-rmcp-client = { workspace = true } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 4422b3884e..e6639da7bc 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -122,6 +122,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 ExecPolicyV2; use codex_otel::otel_event_manager::OtelEventManager; use codex_protocol::config_types::ReasoningEffort as ReasoningEffortConfig; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; @@ -166,6 +167,9 @@ impl Codex { let user_instructions = get_user_instructions(&config).await; + let exec_policy_v2 = crate::exec_policy::exec_policy_for(&config.features, &config.cwd) + .map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy2: {err}")))?; + let config = Arc::new(config); let session_configuration = SessionConfiguration { @@ -182,6 +186,7 @@ impl Codex { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: config.features.clone(), + exec_policy_v2, session_source, }; @@ -279,6 +284,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_v2: Option>, } impl TurnContext { @@ -335,6 +341,8 @@ pub(crate) struct SessionConfiguration { /// Set of feature flags for this session features: Features, + /// Optional execpolicy2 policy, applied only when enabled by feature flag. + exec_policy_v2: Option>, // TODO(pakrym): Remove config from here original_config_do_not_use: Arc, @@ -435,6 +443,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_v2: session_configuration.exec_policy_v2.clone(), } } @@ -1763,6 +1772,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_v2: parent_turn_context.exec_policy_v2.clone(), }; // Seed the child task with the review prompt as the initial user message. @@ -2612,6 +2622,7 @@ mod tests { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: Features::default(), + exec_policy_v2: None, session_source: SessionSource::Exec, }; @@ -2688,6 +2699,7 @@ mod tests { cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), features: Features::default(), + exec_policy_v2: None, session_source: SessionSource::Exec, }; diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs new file mode 100644 index 0000000000..5d5b3789ef --- /dev/null +++ b/codex-rs/core/src/exec_policy.rs @@ -0,0 +1,230 @@ +use std::fs; +use std::io::ErrorKind; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; +use std::sync::OnceLock; + +use codex_execpolicy2::Decision; +use codex_execpolicy2::Evaluation; +use codex_execpolicy2::Policy; +use codex_execpolicy2::PolicyParser; +use codex_protocol::protocol::AskForApproval; +use thiserror::Error; + +use crate::bash::parse_shell_lc_plain_commands; +use crate::features::Feature; +use crate::features::Features; +use crate::tools::sandboxing::ApprovalRequirement; + +const FORBIDDEN_REASON: &str = "execpolicy forbids this command"; +const PROMPT_REASON: &str = "execpolicy requires approval for this command"; + +#[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, + }, +} + +#[derive(Debug)] +struct ExecPolicyState { + cwd: PathBuf, + policy: Arc, +} + +static EXEC_POLICY: OnceLock> = OnceLock::new(); + +pub(crate) fn exec_policy_for( + features: &Features, + cwd: &Path, +) -> Result>, ExecPolicyError> { + if !features.enabled(Feature::ExecPolicyV2) { + return Ok(None); + } + + if let Some(state) = EXEC_POLICY.get() { + return Ok(state.as_ref().map(|state| { + if state.cwd != cwd { + tracing::warn!( + "exec_policy_v2 loaded from {}, reusing for cwd {}", + state.cwd.display(), + cwd.display() + ); + } + Arc::clone(&state.policy) + })); + } + + let loaded = load_policy(cwd)?; + let state = EXEC_POLICY.get_or_init(|| loaded); + + Ok(state.as_ref().map(|state| { + if state.cwd != cwd { + tracing::warn!( + "exec_policy_v2 loaded from {}, reusing for cwd {}", + state.cwd.display(), + cwd.display() + ); + } + Arc::clone(&state.policy) + })) +} + +pub(crate) fn commands_for_policy(command: &[String]) -> Vec> { + if let Some(commands) = parse_shell_lc_plain_commands(command) + && !commands.is_empty() + { + return commands; + } + + vec![command.to_vec()] +} + +pub(crate) fn evaluate_with_policy( + policy: &Policy, + command: &[String], + approval_policy: AskForApproval, +) -> Option { + let commands = commands_for_policy(command); + let evaluation = if let [single] = commands.as_slice() { + policy.check(single) + } else { + 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, + } +} + +fn load_policy(cwd: &Path) -> Result, ExecPolicyError> { + let codex_dir = cwd.join(".codex"); + let entries = match fs::read_dir(&codex_dir) { + Ok(entries) => entries, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(source) => { + return Err(ExecPolicyError::ReadDir { + dir: codex_dir, + source, + }); + } + }; + + let mut policy_paths: Vec = Vec::new(); + for entry in entries { + let entry = entry.map_err(|source| ExecPolicyError::ReadDir { + dir: codex_dir.clone(), + source, + })?; + let path = entry.path(); + if path + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext == "codexpolicy") + && path.is_file() + { + policy_paths.push(path); + } + } + + if policy_paths.is_empty() { + return Ok(None); + } + + policy_paths.sort(); + + let mut parser = PolicyParser::new(); + for policy_path in &policy_paths { + let contents = + fs::read_to_string(policy_path).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!( + file_count = policy_paths.len(), + "loaded execpolicy2 from {}", + codex_dir.display() + ); + + Ok(Some(ExecPolicyState { + cwd: cwd.to_path_buf(), + policy, + })) +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_protocol::protocol::AskForApproval; + use pretty_assertions::assert_eq; + + #[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() + } + ); + } +} diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 2a49eb1ed7..e57d17feaa 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -40,6 +40,8 @@ pub enum Feature { ViewImageTool, /// Allow the model to request web searches. WebSearchRequest, + /// Gate the execpolicy2 enforcement for shell/unified exec. + ExecPolicyV2, /// Enable the model-based risk assessments for sandboxed commands. SandboxCommandAssessment, /// Create a ghost commit at each turn. @@ -283,6 +285,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Stable, default_enabled: false, }, + FeatureSpec { + id: Feature::ExecPolicyV2, + key: "exec_policy_v2", + stage: Stage::Experimental, + default_enabled: false, + }, 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 5229d00606..b2620830e7 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -24,6 +24,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/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 39f271b2da..1906b788f9 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -300,6 +300,11 @@ impl ShellHandler { env: exec_params.env.clone(), with_escalated_permissions: exec_params.with_escalated_permissions, justification: exec_params.justification.clone(), + exec_policy: if is_user_shell_command { + None + } else { + turn.exec_policy_v2.clone() + }, }; 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..0b2efe1963 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -11,6 +11,7 @@ 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; @@ -49,40 +50,49 @@ 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; + match tool.approval_requirement(req, approval_policy, &turn_ctx.sandbox_policy) { + 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..52b406004f 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -6,11 +6,13 @@ 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::exec_policy::evaluate_with_policy; 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,10 +22,12 @@ use crate::tools::sandboxing::ToolCtx; use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; +use codex_execpolicy2::Policy as ExecPolicyV2; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use futures::future::BoxFuture; use std::path::PathBuf; +use std::sync::Arc; #[derive(Clone, Debug)] pub struct ShellRequest { @@ -33,6 +37,7 @@ pub struct ShellRequest { pub env: std::collections::HashMap, pub with_escalated_permissions: Option, pub justification: Option, + pub exec_policy: Option>, } impl ProvidesSandboxRetryData for ShellRequest { @@ -66,6 +71,24 @@ impl ShellRuntime { tx_event: ctx.session.get_tx_event(), }) } + + fn base_approval_requirement( + &self, + req: &ShellRequest, + policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + ) -> ApprovalRequirement { + if requires_initial_appoval( + policy, + sandbox_policy, + &req.command, + req.with_escalated_permissions.unwrap_or(false), + ) { + ApprovalRequirement::NeedsApproval { reason: None } + } else { + ApprovalRequirement::Skip + } + } } impl Sandboxable for ShellRuntime { @@ -114,18 +137,19 @@ impl Approvable for ShellRuntime { }) } - fn wants_initial_approval( + fn approval_requirement( &self, req: &ShellRequest, policy: AskForApproval, sandbox_policy: &SandboxPolicy, - ) -> bool { - requires_initial_appoval( - policy, - sandbox_policy, - &req.command, - req.with_escalated_permissions.unwrap_or(false), - ) + ) -> ApprovalRequirement { + if let Some(exec_policy) = &req.exec_policy + && let Some(requirement) = evaluate_with_policy(exec_policy, &req.command, policy) + { + return requirement; + } + + self.base_approval_requirement(req, policy, sandbox_policy) } fn wants_escalated_first_attempt(&self, req: &ShellRequest) -> bool { @@ -157,3 +181,85 @@ impl ToolRuntime for ShellRuntime { Ok(out) } } + +#[cfg(test)] +mod tests { + use super::*; + use codex_execpolicy2::PolicyParser; + use pretty_assertions::assert_eq; + use std::collections::HashMap; + + fn parse_policy(src: &str) -> Arc { + let mut parser = PolicyParser::new(); + parser + .parse("test.codexpolicy", src) + .expect("parse execpolicy2 file"); + Arc::new(parser.build()) + } + + fn shell_request(command: &[&str], exec_policy: Option>) -> ShellRequest { + ShellRequest { + command: command.iter().map(ToString::to_string).collect(), + cwd: PathBuf::from("."), + timeout_ms: None, + env: HashMap::new(), + with_escalated_permissions: None, + justification: None, + exec_policy, + } + } + + #[test] + fn prompt_decision_requires_approval() { + let policy = parse_policy(r#"prefix_rule(pattern=["echo"], decision="prompt")"#); + let req = shell_request(&["echo", "hi"], Some(policy)); + let runtime = ShellRuntime::new(); + + let requirement = runtime.approval_requirement( + &req, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + ); + + assert_eq!( + requirement, + ApprovalRequirement::NeedsApproval { + reason: Some("execpolicy requires approval for this command".to_string()) + } + ); + } + + #[test] + fn prompt_blocked_when_approval_disabled() { + let policy = parse_policy(r#"prefix_rule(pattern=["echo"], decision="prompt")"#); + let req = shell_request(&["echo", "hi"], Some(policy)); + let runtime = ShellRuntime::new(); + + let requirement = runtime.approval_requirement( + &req, + AskForApproval::Never, + &SandboxPolicy::DangerFullAccess, + ); + + assert_eq!( + requirement, + ApprovalRequirement::Forbidden { + reason: "execpolicy requires approval for this command".to_string() + } + ); + } + + #[test] + fn user_shell_commands_skip_execpolicy() { + let req = shell_request(&["echo", "hi"], None); + let runtime = ShellRuntime::new(); + + let requirement = runtime.approval_requirement( + &req, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + ); + + assert_eq!(requirement, ApprovalRequirement::Skip); + } +} diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index cddac1924e..9458acf1da 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -1,4 +1,5 @@ use crate::command_safety::is_dangerous_command::requires_initial_appoval; +use crate::exec_policy::evaluate_with_policy; /* Runtime: unified exec @@ -10,6 +11,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,18 +24,21 @@ use crate::tools::sandboxing::with_cached_approval; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecSession; use crate::unified_exec::UnifiedExecSessionManager; +use codex_execpolicy2::Policy as ExecPolicyV2; 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; +use std::sync::Arc; #[derive(Clone, Debug)] pub struct UnifiedExecRequest { pub command: Vec, pub cwd: PathBuf, pub env: HashMap, + pub exec_policy: Option>, pub with_escalated_permissions: Option, pub justification: Option, } @@ -63,6 +68,7 @@ impl UnifiedExecRequest { command: Vec, cwd: PathBuf, env: HashMap, + exec_policy: Option>, with_escalated_permissions: Option, justification: Option, ) -> Self { @@ -70,6 +76,7 @@ impl UnifiedExecRequest { command, cwd, env, + exec_policy, with_escalated_permissions, justification, } @@ -80,6 +87,19 @@ impl<'a> UnifiedExecRuntime<'a> { pub fn new(manager: &'a UnifiedExecSessionManager) -> Self { Self { manager } } + + fn base_approval_requirement( + &self, + req: &UnifiedExecRequest, + policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + ) -> ApprovalRequirement { + if requires_initial_appoval(policy, sandbox_policy, &req.command, false) { + ApprovalRequirement::NeedsApproval { reason: None } + } else { + ApprovalRequirement::Skip + } + } } impl Sandboxable for UnifiedExecRuntime<'_> { @@ -129,18 +149,19 @@ impl Approvable for UnifiedExecRuntime<'_> { }) } - fn wants_initial_approval( + fn approval_requirement( &self, req: &UnifiedExecRequest, policy: AskForApproval, sandbox_policy: &SandboxPolicy, - ) -> bool { - requires_initial_appoval( - policy, - sandbox_policy, - &req.command, - req.with_escalated_permissions.unwrap_or(false), - ) + ) -> ApprovalRequirement { + if let Some(exec_policy) = &req.exec_policy + && let Some(requirement) = evaluate_with_policy(exec_policy, &req.command, policy) + { + return requirement; + } + + self.base_approval_requirement(req, policy, sandbox_policy) } 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..bd61c28833 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -86,6 +86,13 @@ pub(crate) struct ApprovalCtx<'a> { pub risk: Option, } +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum ApprovalRequirement { + Skip, + NeedsApproval { reason: Option }, + Forbidden { reason: String }, +} + pub(crate) trait Approvable { type ApprovalKey: Hash + Eq + Clone + Debug + Serialize; @@ -106,21 +113,22 @@ 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( + fn approval_requirement( &self, _req: &Req, policy: AskForApproval, sandbox_policy: &SandboxPolicy, - ) -> bool { - match policy { + ) -> 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 } } diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 55e9102b7e..0ebbd2f443 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -325,6 +325,7 @@ impl UnifiedExecSessionManager { command.to_vec(), cwd, create_env(&context.turn.shell_environment_policy), + context.turn.exec_policy_v2.clone(), with_escalated_permissions, justification, ); diff --git a/codex-rs/core/tests/suite/execpolicy2.rs b/codex-rs/core/tests/suite/execpolicy2.rs new file mode 100644 index 0000000000..20a6512678 --- /dev/null +++ b/codex-rs/core/tests/suite/execpolicy2.rs @@ -0,0 +1,78 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use anyhow::Result; +use codex_core::features::Feature; +use codex_core::protocol::EventMsg; +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(flavor = "multi_thread", worker_threads = 2)] +async fn execpolicy2_blocks_shell_invocation() -> Result<()> { + let mut builder = test_codex().with_config(|config| { + config.features.enable(Feature::ExecPolicyV2); + let policy_dir = config.cwd.join(".codex"); + fs::create_dir_all(&policy_dir).expect("create .codex directory"); + let policy_path = policy_dir.join("policy.codexpolicy"); + 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; + + test.submit_turn("run shell command").await?; + + let EventMsg::ExecCommandEnd(end) = wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::ExecCommandEnd(_)) + }) + .await + else { + unreachable!() + }; + + assert_eq!(end.exit_code, -1); + 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 2224939afd..963c046306 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -27,6 +27,8 @@ mod compact; mod compact_resume_fork; mod deprecation_notice; mod exec; +#[cfg(not(target_os = "windows"))] +mod execpolicy2; mod fork_conversation; mod grep_files; mod items;