diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index cbffcd7e43..8f31f150b3 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -554,12 +554,19 @@ pub fn format_exec_policy_error_with_source(error: &ExecPolicyError) -> String { } } -async fn load_exec_policy_with_warning( +pub(crate) async fn load_exec_policy_with_warning( config_stack: &ConfigLayerStack, ) -> Result<(Policy, Option), ExecPolicyError> { match load_exec_policy(config_stack).await { Ok(policy) => Ok((policy, None)), - Err(err @ ExecPolicyError::ParsePolicy { .. }) => Ok((Policy::empty(), Some(err))), + Err(err @ ExecPolicyError::ParsePolicy { .. }) => { + let policy = config_stack + .requirements() + .exec_policy + .as_deref() + .map_or_else(Policy::empty, |policy| policy.as_ref().clone()); + Ok((policy, Some(err))) + } Err(err) => Err(err), } } diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index cf17387ef5..95d29e0bac 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -374,6 +374,42 @@ async fn merges_requirements_exec_policy_network_rules() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn malformed_custom_rules_preserve_requirements_exec_policy() -> anyhow::Result<()> { + let temp_dir = tempdir()?; + let policy_dir = temp_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&policy_dir)?; + fs::write(policy_dir.join("broken.rules"), "prefix_rule(")?; + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy.add_prefix_rule(&["rm".to_string()], Decision::Forbidden)?; + let requirements = ConfigRequirements { + exec_policy: Some(Sourced::new( + RequirementsExecPolicy::new(requirements_exec_policy), + RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path())?; + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + let config_stack = + ConfigLayerStack::new(vec![layer], requirements, ConfigRequirementsToml::default())?; + + let (policy, warning) = load_exec_policy_with_warning(&config_stack).await?; + + assert!(matches!(warning, Some(ExecPolicyError::ParsePolicy { .. }))); + assert_eq!( + policy + .check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow) + .decision, + Decision::Forbidden + ); + Ok(()) +} + #[tokio::test] async fn preserves_host_executables_when_requirements_overlay_is_present() -> anyhow::Result<()> { let temp_dir = tempdir()?; diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 0a4fe2d7c2..62ed44ad81 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -2,9 +2,8 @@ use crate::config::find_codex_home; use crate::config::is_builtin_permission_profile_name; use crate::config::reject_unknown_builtin_permission_profile; use crate::config::resolve_permission_profile; -use crate::exec_policy::ExecPolicyError; use crate::exec_policy::format_exec_policy_error_with_source; -use crate::exec_policy::load_exec_policy; +use crate::exec_policy::load_exec_policy_with_warning; use anyhow::Context; use anyhow::Result; use codex_config::CONFIG_TOML_FILE; @@ -64,13 +63,15 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec (policy, None), - Err(err @ ExecPolicyError::ParsePolicy { .. }) => { - (codex_execpolicy::Policy::empty(), Some(err)) - } - Err(err) => return Err(err.into()), - }; + let layer_mtimes = collect_layer_mtimes(&config_layer_stack); + let state = build_config_state_from_layers(&config_layer_stack).await?; + Ok((state, layer_mtimes)) +} + +async fn build_config_state_from_layers( + config_layer_stack: &ConfigLayerStack, +) -> Result { + let (exec_policy, warning) = load_exec_policy_with_warning(config_layer_stack).await?; if let Some(err) = warning.as_ref() { tracing::warn!( "failed to parse execpolicy while building network proxy state: {}", @@ -78,12 +79,10 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec Vec { diff --git a/codex-rs/core/src/network_proxy_loader_tests.rs b/codex-rs/core/src/network_proxy_loader_tests.rs index d6c8efc9ae..f7c7c2e452 100644 --- a/codex-rs/core/src/network_proxy_loader_tests.rs +++ b/codex-rs/core/src/network_proxy_loader_tests.rs @@ -5,6 +5,9 @@ use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; +use codex_config::RequirementSource; +use codex_config::RequirementsExecPolicy; +use codex_config::Sourced; use codex_config::permissions_toml::NetworkDomainPermissionToml; use codex_config::permissions_toml::NetworkDomainPermissionsToml; use codex_execpolicy::Decision; @@ -12,6 +15,8 @@ use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::fs; +use tempfile::tempdir; #[test] fn higher_precedence_profile_network_overlays_domain_entries() { @@ -225,6 +230,51 @@ fn execpolicy_network_rules_overlay_network_lists() { ); } +#[tokio::test] +async fn malformed_custom_rules_preserve_managed_denied_domain() { + let temp_dir = tempdir().expect("create temp dir"); + let policy_dir = temp_dir.path().join("rules"); + fs::create_dir_all(&policy_dir).expect("create policy dir"); + fs::write(policy_dir.join("broken.rules"), "prefix_rule(").expect("write malformed policy"); + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy + .add_network_rule( + "blocked.example.com", + NetworkRuleProtocol::Https, + Decision::Forbidden, + /*justification*/ None, + ) + .expect("managed network rule should be valid"); + let requirements = ConfigRequirements { + exec_policy: Some(Sourced::new( + RequirementsExecPolicy::new(requirements_exec_policy), + RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path()) + .expect("dot codex folder should be absolute"); + let layers = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + toml::Value::Table(Default::default()), + )], + requirements, + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let state = build_config_state_from_layers(&layers) + .await + .expect("proxy state should tolerate malformed custom rules"); + + assert_eq!( + state.config.network.denied_domains(), + Some(vec!["blocked.example.com".to_string()]) + ); +} + #[test] fn apply_network_constraints_includes_allow_all_unix_sockets_flag() { let config: toml::Value = toml::from_str( diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 4e64c66b0a..0c73b74ab2 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used)] use anyhow::Result; +use codex_config::test_support::CloudConfigBundleFixture; use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -17,6 +18,7 @@ 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::skip_if_target_windows; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; @@ -255,6 +257,84 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { Ok(()) } +#[tokio::test] +async fn malformed_custom_rules_preserve_managed_forbidden_prefix() -> Result<()> { + skip_if_target_windows!( + Ok(()), + "managed prefix fixture uses POSIX executable semantics" + ); + + let mut builder = test_codex() + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#" +[rules] +prefix_rules = [ + { pattern = [{ token = "echo" }], decision = "forbidden" }, +] +"#, + ), + ) + .with_config(|config| { + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + let policy_path = config.codex_home.join("rules").join("broken.rules"); + fs::create_dir_all( + policy_path + .parent() + .expect("policy directory must have a parent"), + ) + .expect("create policy directory"); + fs::write(policy_path, "prefix_rule(").expect("write malformed policy file"); + }); + let server = start_mock_server().await; + let test = builder.build_with_auto_env(&server).await?; + let call_id = "managed-shell-forbidden"; + let args = json!({ + "cmd": "echo blocked", + "yield_time_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-managed-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-managed-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-managed-1", "done"), + ev_completed("resp-managed-2"), + ]), + ) + .await; + + test.submit_turn_with_approval_and_permission_profile( + "run shell command", + AskForApproval::Never, + PermissionProfile::Disabled, + ) + .await?; + + let output_item = results_mock.single_request().function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("function call output should include a string output payload"); + assert!( + output.contains("policy forbids commands starting with `echo`"), + "unexpected output: {output}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_command_empty_script_with_collaboration_mode_does_not_panic() -> Result<()> { let server = start_mock_server().await;