diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 49afdb5e7..fb63e758e 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -365,6 +365,12 @@ dependencies = [ "x11rb", ] +[[package]] +name = "arc-swap" +version = "1.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" + [[package]] name = "arrayvec" version = "0.7.6" @@ -1256,6 +1262,7 @@ name = "codex-core" version = "0.0.0" dependencies = [ "anyhow", + "arc-swap", "assert_cmd", "assert_matches", "async-channel", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 7cb0eb670..ec3334316 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -16,6 +16,7 @@ workspace = true anyhow = { workspace = true } async-channel = { workspace = true } async-trait = { workspace = true } +arc-swap = "1.7.1" base64 = { workspace = true } chardetng = { workspace = true } chrono = { workspace = true, features = ["serde"] } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index f96a4d5dc..cf4c30f18 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -13,7 +13,7 @@ use crate::compact; use crate::compact::run_inline_auto_compact_task; use crate::compact::should_use_remote_compact_task; use crate::compact_remote::run_inline_remote_auto_compact_task; -use crate::exec_policy::load_exec_policy_for_features; +use crate::exec_policy::ExecPolicyManager; use crate::features::Feature; use crate::features::Features; use crate::models_manager::manager::ModelsManager; @@ -148,7 +148,6 @@ use crate::user_instructions::UserInstructions; use crate::user_notification::UserNotification; use crate::util::backoff; use codex_async_utils::OrCancelExt; -use codex_execpolicy::Policy as ExecPolicy; use codex_otel::otel_manager::OtelManager; use codex_protocol::config_types::ReasoningSummary as ReasoningSummaryConfig; use codex_protocol::models::ContentItem; @@ -241,10 +240,9 @@ impl Codex { ) .await; - let exec_policy = load_exec_policy_for_features(&config.features, &config.codex_home) + let exec_policy = ExecPolicyManager::load(&config.features, &config.codex_home) .await .map_err(|err| CodexErr::Fatal(format!("failed to load execpolicy: {err}")))?; - let exec_policy = Arc::new(RwLock::new(exec_policy)); let config = Arc::new(config); if config.features.enabled(Feature::RemoteModels) @@ -266,7 +264,6 @@ impl Codex { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), - exec_policy, session_source, }; @@ -278,6 +275,7 @@ impl Codex { config.clone(), auth_manager.clone(), models_manager.clone(), + exec_policy, tx_event.clone(), conversation_history, session_source_clone, @@ -371,7 +369,6 @@ 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, } @@ -426,9 +423,6 @@ pub(crate) struct SessionConfiguration { /// operate deterministically. cwd: PathBuf, - /// Execpolicy policy, applied only when enabled by feature flag. - exec_policy: Arc>, - // TODO(pakrym): Remove config from here original_config_do_not_use: Arc, /// Source of the session (cli, vscode, exec, mcp, ...) @@ -533,7 +527,6 @@ impl Session { final_output_json_schema: None, codex_linux_sandbox_exe: per_turn_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.as_ref(), model_family.truncation_policy, @@ -547,6 +540,7 @@ impl Session { config: Arc, auth_manager: Arc, models_manager: Arc, + exec_policy: ExecPolicyManager, tx_event: Sender, initial_history: InitialHistory, session_source: SessionSource, @@ -666,6 +660,7 @@ impl Session { rollout: Mutex::new(Some(rollout_recorder)), user_shell: Arc::new(default_shell), show_raw_agent_reasoning: config.show_raw_agent_reasoning, + exec_policy, auth_manager: Arc::clone(&auth_manager), otel_manager, models_manager: Arc::clone(&models_manager), @@ -1025,29 +1020,24 @@ impl Session { amendment: &ExecPolicyAmendment, ) -> Result<(), ExecPolicyUpdateError> { let features = self.features.clone(); - let (codex_home, current_policy) = { - let state = self.state.lock().await; - ( - state - .session_configuration - .original_config_do_not_use - .codex_home - .clone(), - state.session_configuration.exec_policy.clone(), - ) - }; + let codex_home = self + .state + .lock() + .await + .session_configuration + .original_config_do_not_use + .codex_home + .clone(); if !features.enabled(Feature::ExecPolicy) { error!("attempted to append execpolicy rule while execpolicy feature is disabled"); return Err(ExecPolicyUpdateError::FeatureDisabled); } - crate::exec_policy::append_execpolicy_amendment_and_update( - &codex_home, - ¤t_policy, - &amendment.command, - ) - .await?; + self.services + .exec_policy + .append_amendment_and_update(&codex_home, amendment) + .await?; Ok(()) } @@ -2154,7 +2144,6 @@ 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, model_family.truncation_policy), }; @@ -2748,6 +2737,7 @@ mod tests { use crate::function_tool::FunctionCallError; use crate::shell::default_user_shell; use crate::tools::format_exec_output_str; + use codex_protocol::models::FunctionCallOutputPayload; use crate::protocol::CompactedItem; @@ -2842,7 +2832,6 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), - exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())), session_source: SessionSource::Exec, }; @@ -2909,7 +2898,6 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), - exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())), session_source: SessionSource::Exec, }; @@ -3102,6 +3090,7 @@ mod tests { let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")); let models_manager = Arc::new(ModelsManager::new(auth_manager.clone())); + let exec_policy = ExecPolicyManager::default(); let model = ModelsManager::get_model_offline(config.model.as_deref()); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), @@ -3116,7 +3105,6 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), - exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())), session_source: SessionSource::Exec, }; let per_turn_config = Session::build_per_turn_config(&session_configuration); @@ -3142,6 +3130,7 @@ mod tests { rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), show_raw_agent_reasoning: config.show_raw_agent_reasoning, + exec_policy, auth_manager: auth_manager.clone(), otel_manager: otel_manager.clone(), models_manager, @@ -3188,6 +3177,7 @@ mod tests { let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::from_api_key("Test API Key")); let models_manager = Arc::new(ModelsManager::new(auth_manager.clone())); + let exec_policy = ExecPolicyManager::default(); let model = ModelsManager::get_model_offline(config.model.as_deref()); let session_configuration = SessionConfiguration { provider: config.model_provider.clone(), @@ -3202,7 +3192,6 @@ mod tests { sandbox_policy: config.sandbox_policy.clone(), cwd: config.cwd.clone(), original_config_do_not_use: Arc::clone(&config), - exec_policy: Arc::new(RwLock::new(ExecPolicy::empty())), session_source: SessionSource::Exec, }; let per_turn_config = Session::build_per_turn_config(&session_configuration); @@ -3228,6 +3217,7 @@ mod tests { rollout: Mutex::new(None), user_shell: Arc::new(default_user_shell()), show_raw_agent_reasoning: config.show_raw_agent_reasoning, + exec_policy, auth_manager: Arc::clone(&auth_manager), otel_manager: otel_manager.clone(), models_manager, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 8917610ff..6bbde29d3 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -3,6 +3,8 @@ use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use arc_swap::ArcSwap; + use crate::command_safety::is_dangerous_command::requires_initial_appoval; use codex_execpolicy::AmendError; use codex_execpolicy::Decision; @@ -17,7 +19,6 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use thiserror::Error; use tokio::fs; -use tokio::sync::RwLock; use tokio::task::spawn_blocking; use crate::bash::parse_shell_lc_plain_commands; @@ -80,7 +81,118 @@ pub enum ExecPolicyUpdateError { FeatureDisabled, } -pub(crate) async fn load_exec_policy_for_features( +pub(crate) struct ExecPolicyManager { + policy: ArcSwap, +} + +impl ExecPolicyManager { + pub(crate) fn new(policy: Arc) -> Self { + Self { + policy: ArcSwap::from(policy), + } + } + + pub(crate) async fn load( + features: &Features, + codex_home: &Path, + ) -> Result { + let policy = load_exec_policy_for_features(features, codex_home).await?; + Ok(Self::new(Arc::new(policy))) + } + + pub(crate) fn current(&self) -> Arc { + self.policy.load_full() + } + + pub(crate) async fn create_exec_approval_requirement_for_command( + &self, + features: &Features, + command: &[String], + approval_policy: AskForApproval, + sandbox_policy: &SandboxPolicy, + sandbox_permissions: SandboxPermissions, + ) -> ExecApprovalRequirement { + let exec_policy = self.current(); + let commands = + parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); + let heuristics_fallback = |cmd: &[String]| { + if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) { + Decision::Prompt + } else { + Decision::Allow + } + }; + let evaluation = exec_policy.check_multiple(commands.iter(), &heuristics_fallback); + + match evaluation.decision { + Decision::Forbidden => ExecApprovalRequirement::Forbidden { + reason: FORBIDDEN_REASON.to_string(), + }, + Decision::Prompt => { + if matches!(approval_policy, AskForApproval::Never) { + ExecApprovalRequirement::Forbidden { + reason: PROMPT_CONFLICT_REASON.to_string(), + } + } else { + ExecApprovalRequirement::NeedsApproval { + reason: derive_prompt_reason(&evaluation), + proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { + try_derive_execpolicy_amendment_for_prompt_rules( + &evaluation.matched_rules, + ) + } else { + None + }, + } + } + } + Decision::Allow => ExecApprovalRequirement::Skip { + // Bypass sandbox if execpolicy allows the command + bypass_sandbox: evaluation.matched_rules.iter().any(|rule_match| { + is_policy_match(rule_match) && rule_match.decision() == Decision::Allow + }), + proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { + try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules) + } else { + None + }, + }, + } + } + + pub(crate) async fn append_amendment_and_update( + &self, + codex_home: &Path, + amendment: &ExecPolicyAmendment, + ) -> Result<(), ExecPolicyUpdateError> { + let policy_path = default_policy_path(codex_home); + let prefix = amendment.command.clone(); + spawn_blocking({ + let policy_path = policy_path.clone(); + let prefix = prefix.clone(); + move || blocking_append_allow_prefix_rule(&policy_path, &prefix) + }) + .await + .map_err(|source| ExecPolicyUpdateError::JoinBlockingTask { source })? + .map_err(|source| ExecPolicyUpdateError::AppendRule { + path: policy_path, + source, + })?; + + let mut updated_policy = self.current().as_ref().clone(); + updated_policy.add_prefix_rule(&prefix, Decision::Allow)?; + self.policy.store(Arc::new(updated_policy)); + Ok(()) + } +} + +impl Default for ExecPolicyManager { + fn default() -> Self { + Self::new(Arc::new(Policy::empty())) + } +} + +async fn load_exec_policy_for_features( features: &Features, codex_home: &Path, ) -> Result { @@ -123,37 +235,10 @@ pub async fn load_exec_policy(codex_home: &Path) -> Result PathBuf { +fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE) } -pub(crate) async fn append_execpolicy_amendment_and_update( - codex_home: &Path, - current_policy: &Arc>, - prefix: &[String], -) -> Result<(), ExecPolicyUpdateError> { - let policy_path = default_policy_path(codex_home); - let prefix = prefix.to_vec(); - spawn_blocking({ - let policy_path = policy_path.clone(); - let prefix = prefix.clone(); - move || blocking_append_allow_prefix_rule(&policy_path, &prefix) - }) - .await - .map_err(|source| ExecPolicyUpdateError::JoinBlockingTask { source })? - .map_err(|source| ExecPolicyUpdateError::AppendRule { - path: policy_path, - source, - })?; - - current_policy - .write() - .await - .add_prefix_rule(&prefix, Decision::Allow)?; - - Ok(()) -} - /// Derive a proposed execpolicy amendment when a command requires user approval /// - If any execpolicy rule prompts, return None, because an amendment would not skip that policy requirement. /// - Otherwise return the first heuristics Prompt. @@ -217,59 +302,6 @@ fn derive_prompt_reason(evaluation: &Evaluation) -> Option { }) } -pub(crate) async fn create_exec_approval_requirement_for_command( - exec_policy: &Arc>, - features: &Features, - command: &[String], - approval_policy: AskForApproval, - sandbox_policy: &SandboxPolicy, - sandbox_permissions: SandboxPermissions, -) -> ExecApprovalRequirement { - let commands = parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); - let heuristics_fallback = |cmd: &[String]| { - if requires_initial_appoval(approval_policy, sandbox_policy, cmd, sandbox_permissions) { - Decision::Prompt - } else { - Decision::Allow - } - }; - let policy = exec_policy.read().await; - let evaluation = policy.check_multiple(commands.iter(), &heuristics_fallback); - - match evaluation.decision { - Decision::Forbidden => ExecApprovalRequirement::Forbidden { - reason: FORBIDDEN_REASON.to_string(), - }, - Decision::Prompt => { - if matches!(approval_policy, AskForApproval::Never) { - ExecApprovalRequirement::Forbidden { - reason: PROMPT_CONFLICT_REASON.to_string(), - } - } else { - ExecApprovalRequirement::NeedsApproval { - reason: derive_prompt_reason(&evaluation), - proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { - try_derive_execpolicy_amendment_for_prompt_rules(&evaluation.matched_rules) - } else { - None - }, - } - } - } - Decision::Allow => ExecApprovalRequirement::Skip { - // Bypass sandbox if execpolicy allows the command - bypass_sandbox: evaluation.matched_rules.iter().any(|rule_match| { - is_policy_match(rule_match) && rule_match.decision() == Decision::Allow - }), - proposed_execpolicy_amendment: if features.enabled(Feature::ExecPolicy) { - try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules) - } else { - None - }, - }, - } -} - async fn collect_policy_files(dir: &Path) -> Result, ExecPolicyError> { let mut read_dir = match fs::read_dir(dir).await { Ok(read_dir) => read_dir, @@ -334,9 +366,10 @@ mod tests { features.disable(Feature::ExecPolicy); let temp_dir = tempdir().expect("create temp dir"); - let policy = load_exec_policy_for_features(&features, temp_dir.path()) + let manager = ExecPolicyManager::load(&features, temp_dir.path()) .await - .expect("policy result"); + .expect("manager result"); + let policy = manager.current(); let commands = [vec!["rm".to_string()]]; assert_eq!( @@ -425,7 +458,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let forbidden_script = vec![ "bash".to_string(), @@ -433,15 +466,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") "rm -rf /tmp".to_string(), ]; - let requirement = create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &forbidden_script, - AskForApproval::OnRequest, - &SandboxPolicy::DangerFullAccess, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::new(policy); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &forbidden_script, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -458,18 +492,19 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::OnRequest, - &SandboxPolicy::DangerFullAccess, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::new(policy); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -487,18 +522,19 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::Never, - &SandboxPolicy::DangerFullAccess, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::new(policy); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::Never, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -512,16 +548,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") async fn exec_approval_requirement_falls_back_to_heuristics() { let command = vec!["cargo".to_string(), "build".to_string()]; - let empty_policy = Arc::new(RwLock::new(Policy::empty())); - let requirement = create_exec_approval_requirement_for_command( - &empty_policy, - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -539,7 +575,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec![ "bash".to_string(), "-lc".to_string(), @@ -547,15 +583,15 @@ prefix_rule(pattern=["rm"], decision="forbidden") ]; assert_eq!( - create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::DangerFullAccess, - SandboxPermissions::UseDefault, - ) - .await, + ExecPolicyManager::new(policy) + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ) + .await, ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ @@ -568,14 +604,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") #[tokio::test] async fn append_execpolicy_amendment_updates_policy_and_file() { let codex_home = tempdir().expect("create temp dir"); - let current_policy = Arc::new(RwLock::new(Policy::empty())); let prefix = vec!["echo".to_string(), "hello".to_string()]; + let manager = ExecPolicyManager::default(); - append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &prefix) + manager + .append_amendment_and_update(codex_home.path(), &ExecPolicyAmendment::from(prefix)) .await .expect("update policy"); + let updated_policy = manager.current(); - let evaluation = current_policy.read().await.check( + let evaluation = updated_policy.check( &["echo".to_string(), "hello".to_string(), "world".to_string()], &|_| Decision::Allow, ); @@ -599,10 +637,11 @@ prefix_rule(pattern=["rm"], decision="forbidden") #[tokio::test] async fn append_execpolicy_amendment_rejects_empty_prefix() { let codex_home = tempdir().expect("create temp dir"); - let current_policy = Arc::new(RwLock::new(Policy::empty())); + let manager = ExecPolicyManager::default(); - let result = - append_execpolicy_amendment_and_update(codex_home.path(), ¤t_policy, &[]).await; + let result = manager + .append_amendment_and_update(codex_home.path(), &ExecPolicyAmendment::from(vec![])) + .await; assert!(matches!( result, @@ -617,16 +656,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") async fn proposed_execpolicy_amendment_is_present_for_single_command_without_policy_match() { let command = vec!["cargo".to_string(), "build".to_string()]; - let empty_policy = Arc::new(RwLock::new(Policy::empty())); - let requirement = create_exec_approval_requirement_for_command( - &empty_policy, - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -644,15 +683,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") let mut features = Features::with_defaults(); features.disable(Feature::ExecPolicy); - let requirement = create_exec_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), - &features, - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command( + &features, + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -670,18 +710,19 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec!["rm".to_string()]; - let requirement = create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::OnRequest, - &SandboxPolicy::DangerFullAccess, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::new(policy); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::OnRequest, + &SandboxPolicy::DangerFullAccess, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -699,15 +740,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") "-lc".to_string(), "cargo build && echo ok".to_string(), ]; - let requirement = create_exec_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -728,7 +770,7 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec![ "bash".to_string(), @@ -737,15 +779,15 @@ prefix_rule(pattern=["rm"], decision="forbidden") ]; assert_eq!( - create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::UnlessTrusted, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await, + ExecPolicyManager::new(policy) + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::UnlessTrusted, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await, ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ @@ -759,15 +801,16 @@ prefix_rule(pattern=["rm"], decision="forbidden") async fn proposed_execpolicy_amendment_is_present_when_heuristics_allow() { let command = vec!["echo".to_string(), "safe".to_string()]; - let requirement = create_exec_approval_requirement_for_command( - &Arc::new(RwLock::new(Policy::empty())), - &Features::with_defaults(), - &command, - AskForApproval::OnRequest, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::default(); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::OnRequest, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, @@ -785,18 +828,19 @@ prefix_rule(pattern=["rm"], decision="forbidden") parser .parse("test.rules", policy_src) .expect("parse policy"); - let policy = Arc::new(RwLock::new(parser.build())); + let policy = Arc::new(parser.build()); let command = vec!["echo".to_string(), "safe".to_string()]; - let requirement = create_exec_approval_requirement_for_command( - &policy, - &Features::with_defaults(), - &command, - AskForApproval::OnRequest, - &SandboxPolicy::ReadOnly, - SandboxPermissions::UseDefault, - ) - .await; + let manager = ExecPolicyManager::new(policy); + let requirement = manager + .create_exec_approval_requirement_for_command( + &Features::with_defaults(), + &command, + AskForApproval::OnRequest, + &SandboxPolicy::ReadOnly, + SandboxPermissions::UseDefault, + ) + .await; assert_eq!( requirement, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 722c86274..63b66647e 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use crate::AuthManager; use crate::RolloutRecorder; +use crate::exec_policy::ExecPolicyManager; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::skills::SkillsManager; @@ -21,6 +22,7 @@ pub(crate) struct SessionServices { pub(crate) rollout: Mutex>, pub(crate) user_shell: Arc, pub(crate) show_raw_agent_reasoning: bool, + pub(crate) exec_policy: ExecPolicyManager, pub(crate) auth_manager: Arc, pub(crate) models_manager: Arc, pub(crate) otel_manager: OtelManager, diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 624094a5a..720361771 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -6,7 +6,6 @@ use std::sync::Arc; use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; -use crate::exec_policy::create_exec_approval_requirement_for_command; use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; use crate::protocol::ExecCommandSource; @@ -252,15 +251,17 @@ impl ShellHandler { emitter.begin(event_ctx).await; let features = session.features(); - let exec_approval_requirement = create_exec_approval_requirement_for_command( - &turn.exec_policy, - &features, - &exec_params.command, - turn.approval_policy, - &turn.sandbox_policy, - exec_params.sandbox_permissions, - ) - .await; + let exec_approval_requirement = session + .services + .exec_policy + .create_exec_approval_requirement_for_command( + &features, + &exec_params.command, + turn.approval_policy, + &turn.sandbox_policy, + exec_params.sandbox_permissions, + ) + .await; let req = ShellRequest { command: exec_params.command.clone(), diff --git a/codex-rs/core/src/unified_exec/session_manager.rs b/codex-rs/core/src/unified_exec/session_manager.rs index 17d5ef671..c97820e46 100644 --- a/codex-rs/core/src/unified_exec/session_manager.rs +++ b/codex-rs/core/src/unified_exec/session_manager.rs @@ -14,7 +14,6 @@ use crate::bash::extract_bash_command; use crate::codex::Session; use crate::codex::TurnContext; use crate::exec_env::create_env; -use crate::exec_policy::create_exec_approval_requirement_for_command; use crate::protocol::BackgroundEventEvent; use crate::protocol::EventMsg; use crate::sandboxing::ExecEnv; @@ -484,15 +483,18 @@ impl UnifiedExecSessionManager { let features = context.session.features(); let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new(self); - let exec_approval_requirement = create_exec_approval_requirement_for_command( - &context.turn.exec_policy, - &features, - command, - context.turn.approval_policy, - &context.turn.sandbox_policy, - sandbox_permissions, - ) - .await; + let exec_approval_requirement = context + .session + .services + .exec_policy + .create_exec_approval_requirement_for_command( + &features, + command, + context.turn.approval_policy, + &context.turn.sandbox_policy, + sandbox_permissions, + ) + .await; let req = UnifiedExecToolRequest::new( command.to_vec(), cwd,