diff --git a/codex-rs/app-server/src/request_processors/command_exec_processor.rs b/codex-rs/app-server/src/request_processors/command_exec_processor.rs index 0c03ceede4..fde1b9e895 100644 --- a/codex-rs/app-server/src/request_processors/command_exec_processor.rs +++ b/codex-rs/app-server/src/request_processors/command_exec_processor.rs @@ -238,26 +238,12 @@ impl CommandExecRequestProcessor { config.effective_workspace_roots(), ) } else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) { - self.config - .permissions - .can_set_legacy_sandbox_policy(&policy, &sandbox_cwd) - .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; - let file_system_sandbox_policy = - codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd); - let network_sandbox_policy = - codex_protocol::permissions::NetworkSandboxPolicy::from(&policy); - let permission_profile = - codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement( - codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&policy), - &file_system_sandbox_policy, - network_sandbox_policy, - ); - self.config - .permissions - .can_set_permission_profile(&permission_profile) + let mut permissions = self.config.permissions.clone(); + permissions + .set_legacy_sandbox_policy(policy, &sandbox_cwd) .map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?; ( - permission_profile, + permissions.effective_permission_profile(), self.config.permissions.network.clone(), self.config.permissions.permission_profile().clone(), self.config.managed_network_requirements_enabled(), diff --git a/codex-rs/app-server/tests/suite/v2/command_exec.rs b/codex-rs/app-server/tests/suite/v2/command_exec.rs index eec0797b9e..ea833ea211 100644 --- a/codex-rs/app-server/tests/suite/v2/command_exec.rs +++ b/codex-rs/app-server/tests/suite/v2/command_exec.rs @@ -330,6 +330,119 @@ async fn command_exec_accepts_permission_profile() -> Result<()> { Ok(()) } +#[cfg(unix)] +#[tokio::test] +async fn command_exec_enforces_managed_deny_read_requirements() -> Result<()> { + let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri(), "never")?; + let denied_root = codex_home.path().join("private"); + let nested_root = denied_root.join("nested"); + std::fs::create_dir_all(&nested_root)?; + let denied_path = nested_root.join("secret.txt"); + std::fs::write(&denied_path, "managed secret")?; + let user_denied_root = codex_home.path().join("user-private"); + std::fs::create_dir_all(&user_denied_root)?; + let user_denied_path = user_denied_root.join("secret.txt"); + std::fs::write(&user_denied_path, "user secret")?; + std::fs::write( + codex_home.path().join("requirements.toml"), + format!("[permissions.filesystem]\ndeny_read = [{denied_root:?}]\n"), + )?; + insert_command_exec_config( + codex_home.path(), + &format!( + "default_permissions = \"thread-policy\"\n\n[permissions.thread-policy.filesystem]\n\":root\" = \"read\"\n{user_denied_root:?} = \"deny\"\n\n[permissions.nested.filesystem]\n\":root\" = \"read\"\n{nested_root:?} = \"write\"\n" + ), + )?; + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .without_auto_env() + .build_initialized_with_timeout(DEFAULT_READ_TIMEOUT) + .await?; + + let request = CommandExecParams { + command: vec![ + "sh".to_string(), + "-c".to_string(), + "cat \"$1\"".to_string(), + "sh".to_string(), + denied_path.to_string_lossy().into_owned(), + ], + process_id: None, + tty: false, + stream_stdin: false, + stream_stdout_stderr: false, + output_bytes_cap: None, + disable_output_cap: false, + disable_timeout: false, + timeout_ms: None, + cwd: Some(codex_home.path().to_path_buf()), + env: None, + size: None, + sandbox_policy: Some(SandboxPolicy::ReadOnly { + network_access: false, + }), + permission_profile: None, + }; + let request_id = app_server + .send_command_exec_request(request.clone()) + .await?; + let response: CommandExecResponse = app_server.read_response(request_id).await?; + assert_ne!(response.exit_code, 0); + assert!(!response.stdout.contains("managed secret")); + + let mut user_request = request.clone(); + *user_request + .command + .last_mut() + .expect("cat command includes a file path") = user_denied_path.to_string_lossy().into(); + let request_id = app_server.send_command_exec_request(user_request).await?; + let response: CommandExecResponse = app_server.read_response(request_id).await?; + assert_eq!( + response, + CommandExecResponse { + exit_code: 0, + stdout: "user secret".to_string(), + stderr: String::new(), + } + ); + + for sandbox_policy in [ + SandboxPolicy::DangerFullAccess, + SandboxPolicy::WorkspaceWrite { + writable_roots: vec![nested_root.try_into()?], + network_access: false, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }, + ] { + let request_id = app_server + .send_command_exec_request(CommandExecParams { + sandbox_policy: Some(sandbox_policy), + ..request.clone() + }) + .await?; + let error = app_server + .read_stream_until_error_message(RequestId::Integer(request_id)) + .await?; + assert!(error.error.message.contains("invalid sandbox policy")); + } + + let request_id = app_server + .send_command_exec_request(CommandExecParams { + sandbox_policy: None, + permission_profile: Some("nested".to_string()), + ..request + }) + .await?; + let error = app_server + .read_stream_until_error_message(RequestId::Integer(request_id)) + .await?; + assert!(error.error.message.contains("invalid permission profile")); + Ok(()) +} + #[tokio::test] async fn command_exec_permission_profile_starts_selected_network_proxy() -> Result<()> { let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 78d92c9b89..0d0ac94a9e 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -114,8 +114,10 @@ use codex_protocol::models::SandboxEnforcement; use codex_protocol::openai_models::ModelMessages; use codex_protocol::openai_models::ModelsResponse; use codex_protocol::openai_models::ReasoningEffort; +use codex_protocol::permissions::FileSystemPath; use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::permissions::ReadDenyMatcher; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SandboxPolicy; @@ -297,6 +299,8 @@ pub struct Permissions { /// Constrained permission profile plus its selected profile identity, if /// the profile came from a built-in or named config profile. permission_profile_state: PermissionProfileState, + /// Managed deny-read rules retained independently from user-defined denies. + managed_deny_read_policy: Option>, /// Thread-scoped runtime workspace roots. Symbolic `:workspace_roots` /// entries in the permission profile are materialized against these roots. workspace_roots: Vec, @@ -332,6 +336,7 @@ impl Permissions { permission_profile_state: PermissionProfileState::from_constrained_legacy( permission_profile, )?, + managed_deny_read_policy: None, workspace_roots: Vec::new(), network: None, allow_login_shell: true, @@ -388,8 +393,10 @@ impl Permissions { &self, permission_profile: &PermissionProfile, ) -> ConstraintResult<()> { + let permission_profile = + self.permission_profile_preserving_managed_denied_reads(permission_profile.clone()); self.permission_profile_state - .can_set_legacy_permission_profile(permission_profile) + .can_set_legacy_permission_profile(&permission_profile) } pub fn set_workspace_roots(&mut self, workspace_roots: Vec) { @@ -455,8 +462,7 @@ impl Permissions { &file_system_sandbox_policy, network_sandbox_policy, ); - self.permission_profile_state - .can_set_legacy_permission_profile(&permission_profile) + self.can_set_permission_profile(&permission_profile) } /// Set permissions from a legacy sandbox policy and keep every permission @@ -496,9 +502,7 @@ impl Permissions { ], }; - self.permission_profile_state - .set_legacy_permission_profile(permission_profile)?; - Ok(()) + self.set_permission_profile(permission_profile) } /// Set permissions from the canonical profile. @@ -506,9 +510,34 @@ impl Permissions { &mut self, permission_profile: PermissionProfile, ) -> ConstraintResult<()> { + let permission_profile = + self.permission_profile_preserving_managed_denied_reads(permission_profile); self.permission_profile_state .set_legacy_permission_profile(permission_profile) } + + fn permission_profile_preserving_managed_denied_reads( + &self, + permission_profile: PermissionProfile, + ) -> PermissionProfile { + let Some(managed_deny_read_policy) = self.managed_deny_read_policy.as_ref() else { + return permission_profile; + }; + if matches!( + sandbox_mode_requirement_for_permission_profile(&permission_profile), + SandboxModeRequirement::DangerFullAccess | SandboxModeRequirement::ExternalSandbox + ) { + return permission_profile; + } + let enforcement = permission_profile.enforcement(); + let (mut file_system_policy, network_policy) = permission_profile.to_runtime_permissions(); + file_system_policy.preserve_deny_read_restrictions_from(managed_deny_read_policy); + PermissionProfile::from_runtime_permissions_with_enforcement( + enforcement, + &file_system_policy, + network_policy, + ) + } } // A profile override only inherits the selected profile's proxy/allowlist config @@ -3985,15 +4014,17 @@ impl Config { effective_file_system_sandbox_policy .preserve_deny_read_restrictions_from(&file_system_sandbox_policy); } - if let Some(Sourced { - value: filesystem_requirements, - .. - }) = filesystem_requirements.as_ref() - { - apply_managed_filesystem_constraints( - &mut effective_file_system_sandbox_policy, - filesystem_requirements, - ); + let managed_deny_read_policy = filesystem_requirements + .as_ref() + .filter(|Sourced { value, .. }| !value.deny_read.is_empty()) + .map(|Sourced { value, .. }| { + let mut policy = FileSystemSandboxPolicy::restricted(Vec::new()); + apply_managed_filesystem_constraints(&mut policy, value); + Arc::new(policy) + }); + if let Some(managed_deny_read_policy) = managed_deny_read_policy.as_ref() { + effective_file_system_sandbox_policy + .preserve_deny_read_restrictions_from(managed_deny_read_policy); } let effective_file_system_sandbox_policy = effective_file_system_sandbox_policy .with_additional_readable_roots(resolved_cwd.as_path(), &helper_readable_roots); @@ -4006,6 +4037,58 @@ impl Config { .value .set(effective_permission_profile) .map_err(std::io::Error::from)?; + if let Some(Sourced { + source: requirement_source, + .. + }) = filesystem_requirements.as_ref() + && let Some(managed_file_system_policy) = managed_deny_read_policy.as_ref() + { + let _initial_matcher = + ReadDenyMatcher::try_new(managed_file_system_policy, resolved_cwd.as_path()) + .map_err(std::io::Error::other)?; + let managed_file_system_policy = Arc::clone(managed_file_system_policy); + let permission_cwd = resolved_cwd.clone(); + let requirement_source = requirement_source.clone(); + constrained_permission_profile + .value + .add_validator(move |permission_profile| { + let managed_deny_matcher = + ReadDenyMatcher::new(&managed_file_system_policy, permission_cwd.as_path()); + let file_system_policy = permission_profile.file_system_sandbox_policy(); + let missing_required_deny = managed_file_system_policy + .entries + .iter() + .any(|entry| !file_system_policy.entries.contains(entry)); + let violating_root = file_system_policy + .entries + .iter() + .filter(|entry| entry.access.can_read()) + .find_map(|entry| { + let FileSystemPath::Path { path } = &entry.path else { + return None; + }; + let path = path.to_abs_path().ok()?; + managed_deny_matcher + .as_ref() + .is_some_and(|matcher| matcher.is_read_denied(path.as_path())) + .then_some(path) + }); + if missing_required_deny || violating_root.is_some() { + return Err(ConstraintError::InvalidValue { + field_name: "permissions.filesystem", + candidate: violating_root + .map_or_else(|| "missing managed deny".to_string(), |path| { + path.to_string_lossy().into_owned() + }), + allowed: "all managed deny_read restrictions".to_string(), + requirement_source: requirement_source.clone(), + }); + } + + Ok(()) + }) + .map_err(std::io::Error::from)?; + } let permission_profile_state = PermissionProfileState::from_constrained_active_profile( constrained_permission_profile.value, active_permission_profile, @@ -4031,6 +4114,7 @@ impl Config { permissions: Permissions { approval_policy: constrained_approval_policy.value, permission_profile_state, + managed_deny_read_policy, workspace_roots, network, allow_login_shell, diff --git a/codex-rs/core/tests/suite/cloud_config.rs b/codex-rs/core/tests/suite/cloud_config.rs index a2afc7584d..a8d49a69ab 100644 --- a/codex-rs/core/tests/suite/cloud_config.rs +++ b/codex-rs/core/tests/suite/cloud_config.rs @@ -1,9 +1,17 @@ +use anyhow::Context; use anyhow::Result; use codex_config::CloudConfigBundleLoader; use codex_config::test_support::CloudConfigBundleFixture; +use codex_core::CodexThreadSettingsOverrides; use codex_features::Feature; +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_target_windows; +use core_test_support::submit_thread_settings; use core_test_support::test_codex::test_codex; use pretty_assertions::assert_eq; use std::sync::Arc; @@ -71,6 +79,72 @@ async fn refreshed_cloud_bundle_updates_later_sessions() -> Result<()> { Ok(()) } +#[tokio::test] +async fn managed_deny_read_requirements_follow_thread_permission_updates() -> Result<()> { + skip_if_target_windows!( + Ok(()), + "Windows restricted-token sandbox cannot enforce deny-read policies" + ); + + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + let denied_root = home.path().join("managed-private"); + let nested_root = denied_root.join("nested"); + std::fs::create_dir_all(&nested_root)?; + let nested_root = AbsolutePathBuf::from_absolute_path(nested_root)?; + + let mut builder = test_codex() + .with_home(home) + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement(format!( + "[permissions.filesystem]\ndeny_read = [{denied_root:?}]\n" + )), + ) + .with_config(|config| { + config + .permissions + .set_permission_profile(PermissionProfile::workspace_write()) + .expect("safe workspace permissions should preserve managed denies"); + }); + let test = builder.build_with_auto_env(&server).await?; + + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + permission_profile: Some(PermissionProfile::read_only()), + ..Default::default() + }, + ) + .await?; + let snapshot = test.codex.config_snapshot().await; + assert!( + !snapshot + .permission_profile + .file_system_sandbox_policy() + .can_read_path_with_cwd(nested_root.as_path(), snapshot.cwd().as_path()), + "a live permission change must preserve managed deny-read rules" + ); + + let conflicting_profile = PermissionProfile::workspace_write_with( + std::slice::from_ref(&nested_root), + NetworkSandboxPolicy::Restricted, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + ); + let error = test + .codex + .preview_thread_settings_overrides(CodexThreadSettingsOverrides { + permission_profile: Some(conflicting_profile), + ..Default::default() + }) + .await + .err() + .context("a concrete writable root must not override a managed deny")?; + assert!(error.to_string().contains("permissions.filesystem")); + + Ok(()) +} + #[tokio::test] async fn managed_guardian_v1_requirements_disable_guardian_v2() -> Result<()> { let server = start_mock_server().await;