diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 191340d407..d7921ac05f 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -179,6 +179,75 @@ impl NetworkProxySpec { EnvironmentNetworkPolicy::from_config(&self.config, self.hard_deny_allowlist_misses) } + pub(crate) fn for_environment( + controller: Option<&Self>, + policy: &EnvironmentNetworkPolicy, + permission_profile: &PermissionProfile, + exec_policy: &Policy, + ) -> std::io::Result { + if matches!(permission_profile, PermissionProfile::Disabled) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "environment network policy requires managed network enforcement", + )); + } + if controller.is_some_and(|controller| !controller.enabled()) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "environment network policy cannot override a disabled controller proxy", + )); + } + let mut spec = match controller { + Some(controller) => controller.recompute_for_permission_profile(permission_profile)?, + None => Self::from_config_and_constraints( + NetworkProxyConfig { + enabled: true, + // Without a controller, the owner supplies the entire permission ceiling. + dangerously_allow_all_unix_sockets: true, + allow_local_binding: true, + ..NetworkProxyConfig::default() + }, + /*requirements*/ None, + permission_profile, + )?, + }; + policy.apply_to(&mut spec.config); + let protected_denials = spec.config.denied_domains().unwrap_or_default(); + + // A fixed controller allowlist remains a ceiling; an expandable one is only a baseline. + // Profiles without managed approvals must not reuse approvals cached by another profile. + let fixed_allowlist = spec.hard_deny_allowlist_misses + || spec.constraints.allowlist_expansion_enabled == Some(false); + spec.hard_deny_allowlist_misses |= policy.managed_allowed_domains_only + || !Self::managed_sandbox_active(permission_profile); + let allow_owner_grants = !spec.hard_deny_allowlist_misses && !fixed_allowlist; + if fixed_allowlist { + spec.constraints.allowlist_expansion_enabled = None; + } else { + spec.constraints.allowed_domains = + Some(spec.config.allowed_domains().unwrap_or_default()); + spec.constraints.allowlist_expansion_enabled = Some(allow_owner_grants); + } + spec.constraints.denylist_expansion_enabled = Some(true); + + // Saved grants can extend a reviewable owner policy; owner denials are restored last. + let (allowed_domains, denied_domains) = exec_policy.compiled_network_domains(); + if allow_owner_grants { + upsert_network_domains(&mut spec.config, allowed_domains, /*allow*/ true); + } + upsert_network_domains(&mut spec.config, denied_domains, /*allow*/ false); + upsert_network_domains(&mut spec.config, protected_denials, /*allow*/ false); + spec.constraints.denied_domains = spec.config.denied_domains(); + + validate_policy_against_constraints(&spec.config, &spec.constraints).map_err(|error| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("environment network policy violates managed requirements: {error}"), + ) + })?; + Ok(spec) + } + pub(crate) fn with_exec_policy_network_rules( &self, exec_policy: &Policy, @@ -208,7 +277,7 @@ impl NetworkProxySpec { }) } - fn build_state_with_audit_metadata( + pub(crate) fn build_state_with_audit_metadata( &self, audit_metadata: NetworkProxyAuditMetadata, ) -> std::io::Result { diff --git a/codex-rs/core/src/config/network_proxy_spec_tests.rs b/codex-rs/core/src/config/network_proxy_spec_tests.rs index ff90af0c74..d069a1f2e4 100644 --- a/codex-rs/core/src/config/network_proxy_spec_tests.rs +++ b/codex-rs/core/src/config/network_proxy_spec_tests.rs @@ -1,7 +1,11 @@ use super::*; use codex_config::NetworkDomainPermissionToml; use codex_config::NetworkDomainPermissionsToml; +use codex_execpolicy::Decision::Allow; +use codex_execpolicy::NetworkRuleProtocol::Https; use codex_network_proxy::NetworkDomainPermission; +use codex_network_proxy::NetworkUnixSocketPermission; +use codex_network_proxy::NetworkUnixSocketPermissions; use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; @@ -40,6 +44,117 @@ fn build_state_with_audit_metadata_threads_metadata_to_state() { assert_eq!(state.audit_metadata(), &metadata); } +#[test] +fn environment_policy_replaces_soft_controller_allowlist_and_preserves_denials() { + let requirements = NetworkConstraints { + domains: Some(domain_permissions([ + ("controller.example", NetworkDomainPermissionToml::Allow), + ("blocked.example", NetworkDomainPermissionToml::Deny), + ])), + ..Default::default() + }; + let profile = PermissionProfile::workspace_write(); + let spec = NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig { + enabled: true, + allow_upstream_proxy: false, + unix_sockets: Some(NetworkUnixSocketPermissions { + entries: [ + ( + "/tmp/controller.sock".to_string(), + NetworkUnixSocketPermission::Deny, + ), + ( + "/tmp/allowed.sock".to_string(), + NetworkUnixSocketPermission::Allow, + ), + ] + .into(), + }), + ..NetworkProxyConfig::default() + }, + Some(requirements), + &profile, + ) + .expect("controller policy should be valid"); + let mut owner = NetworkProxyConfig::default(); + owner.set_allowed_domains(vec!["owner.example".to_string()]); + owner.set_denied_domains(vec!["owner-blocked.example".to_string()]); + owner.set_allow_unix_sockets(vec![ + "/tmp/controller.sock".to_string(), + "/private/tmp/controller.sock".to_string(), + "/tmp/allowed.sock".to_string(), + ]); + owner.dangerously_allow_all_unix_sockets = true; + owner.allow_local_binding = true; + let owner_policy = + EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); + let compose = NetworkProxySpec::for_environment; + let empty = Policy::empty(); + let disabled_controller = NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig::default(), + /*requirements*/ None, + &profile, + ) + .expect("disabled controller policy should be valid"); + assert!(compose(Some(&disabled_controller), &owner_policy, &profile, &empty).is_err()); + let restricted = compose(Some(&spec), &owner_policy, &profile, &empty) + .expect("owner policy should replace soft controller grants"); + let mut saved = Policy::empty(); + for host in ["saved.example", "owner-blocked.example"] { + saved + .add_network_rule(host, Https, Allow, /*justification*/ None) + .expect("saved network grant should be valid"); + } + let rootless = compose(/*controller*/ None, &owner_policy, &profile, &saved) + .expect("an owner policy can create executor-side proxy state"); + assert_eq!( + rootless.config.allowed_domains().unwrap(), + ["owner.example", "saved.example"] + ); + + owner.upsert_domain_permission( + "blocked.example".to_string(), + NetworkDomainPermission::Deny, + normalize_host, + ); + owner.unix_sockets.clone_from(&spec.config.unix_sockets); + owner.allow_upstream_proxy = false; + owner.dangerously_allow_all_unix_sockets = false; + owner.allow_local_binding = false; + assert_eq!( + restricted.environment_policy(), + EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false) + ); + let external = PermissionProfile::External { + network: NetworkSandboxPolicy::Enabled, + }; + let external_rootless = compose(/*controller*/ None, &owner_policy, &external, &saved) + .expect("an externally sandboxed owner policy should remain strict"); + assert_eq!( + external_rootless.environment_policy(), + EnvironmentNetworkPolicy { + managed_allowed_domains_only: true, + ..owner_policy.clone() + } + ); + let controller_policy = spec.environment_policy(); + let external_rooted = compose(Some(&spec), &controller_policy, &external, &saved) + .expect("an externally sandboxed owner policy may retain its controller allowlist"); + assert_eq!( + external_rooted.environment_policy(), + EnvironmentNetworkPolicy { + managed_allowed_domains_only: true, + ..controller_policy + } + ); + assert!(compose(Some(&spec), &owner_policy, &external, &empty).is_err()); + owner.set_allowed_domains(vec!["*".to_string()]); + let wildcard_policy = + EnvironmentNetworkPolicy::from_config(&owner, /*managed_allowed_domains_only*/ false); + assert!(compose(Some(&spec), &wildcard_policy, &profile, &empty).is_err()); +} + #[test] fn requirements_allowed_domains_are_a_baseline_for_user_allowlist() { let mut config = NetworkProxyConfig::default(); diff --git a/codex-rs/core/src/session/environment.rs b/codex-rs/core/src/session/environment.rs index 09b7803cc2..106be766fb 100644 --- a/codex-rs/core/src/session/environment.rs +++ b/codex-rs/core/src/session/environment.rs @@ -1,7 +1,9 @@ use std::collections::HashSet; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::MAX_SELECTED_CAPABILITY_ROOTS; use codex_exec_server::SelectedCapabilityRootsStatus; +use codex_execpolicy::Policy; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; @@ -11,6 +13,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection; use crate::config::ConstraintError; use crate::config::ConstraintResult; +use crate::config::NetworkProxySpec; use crate::session::session::Session; use crate::session::session::SessionConfiguration; use crate::session::session::SessionSettingsUpdate; @@ -35,12 +38,31 @@ fn validate_environment_config( selection: &TurnEnvironmentSelection, config: &EnvironmentConfig, ) -> CodexResult<()> { - // The public type can be used by owners before runtime enforcement lands. Do not - // accept restrictions here until the managed proxy can actually enforce them. - if config.network_policy.is_some() { - return Err(CodexErr::InvalidRequest( - "attachment-owned network policy is not supported yet".to_string(), - )); + if let Some(policy) = config.network_policy.as_ref() { + if selection.environment_id == LOCAL_ENVIRONMENT_ID { + return Err(CodexErr::InvalidRequest( + "attachment-owned network policy requires a remote executor".to_string(), + )); + } + if config + .exec_policy + .as_ref() + .is_some_and(|policy| !policy.as_ref().network_rules().is_empty()) + { + return Err(CodexErr::InvalidRequest( + "environment network restrictions must use network_policy".to_string(), + )); + } + // Validate owner policy on its own; controller compatibility is checked at execution. + NetworkProxySpec::for_environment( + /*controller*/ None, + policy, + config.permission_profile.permission_profile(), + &Policy::empty(), + ) + .map_err(|error| { + CodexErr::InvalidRequest(format!("invalid environment network policy: {error}")) + })?; } if config.selected_capability_roots.len() > MAX_SELECTED_CAPABILITY_ROOTS { return Err(CodexErr::InvalidRequest(format!( diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index e776ae059e..f348ae6fb0 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -1,7 +1,9 @@ +use crate::config::NetworkProxySpec; use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::GuardianReviewContext; use crate::network_policy_decision::denied_network_policy_message; use crate::session::session::Session; +use crate::session::turn_context::TurnContext; use crate::session::turn_context::TurnEnvironment; use crate::tools::approvals::ApprovalAction; use crate::tools::approvals::ApprovalContext; @@ -9,6 +11,7 @@ use crate::tools::events::truncate_rejection_message; use crate::tools::sandboxing::ToolError; use codex_network_proxy::BlockedRequest; use codex_network_proxy::BlockedRequestObserver; +use codex_network_proxy::EnvironmentNetworkPolicy; use codex_network_proxy::NetworkDecision; use codex_network_proxy::NetworkPolicyDecider; use codex_network_proxy::NetworkPolicyRequest; @@ -45,7 +48,7 @@ use uuid::Uuid; const ABANDONED_NETWORK_APPROVAL_MESSAGE: &str = "network approval was cancelled before a decision was returned"; -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct NetworkApprovalSpec { pub network: Option, pub trigger: GuardianNetworkAccessTrigger, @@ -54,6 +57,7 @@ pub(crate) struct NetworkApprovalSpec { pub command: String, pub environment_id: String, pub permission_profile: PermissionProfile, + pub network_policy: Option, } #[derive(Clone, Debug)] @@ -1021,8 +1025,8 @@ pub(crate) fn build_network_policy_decider( } pub(crate) async fn begin_network_approval( - session: &Session, - turn_id: &str, + session: &Arc, + turn: &TurnContext, managed_network_active: bool, spec: Option, ) -> Result, ToolError> { @@ -1033,21 +1037,93 @@ pub(crate) async fn begin_network_approval( command, environment_id, permission_profile, + network_policy, } = match spec { Some(spec) => spec, None => return Ok(None), }; - let Some(network) = network else { - return Ok(None); - }; if !managed_network_active { return Ok(None); } + let controller = turn.config.permissions.network.as_ref(); + let owner_spec = network_policy + .as_ref() + .map(|policy| { + // Resolve owner policy once, where the command's actual enforcing proxy is built. + NetworkProxySpec::for_environment( + controller, + policy, + &permission_profile, + session.services.exec_policy.current().as_ref(), + ) + }) + .transpose() + .map_err(|error| { + ToolError::Rejected(format!( + "failed to resolve environment network policy: {error}" + )) + })?; + let network = if let Some(owner_spec) = owner_spec.as_ref() { + if controller.is_some() { + network + .or_else(|| { + session + .services + .network_proxy + .load_full() + .map(|controller| controller.proxy()) + }) + .ok_or_else(|| { + ToolError::Rejected( + "environment network policy requires its configured controller proxy" + .to_string(), + ) + })? + } else { + // This carrier never listens: the executor starts the real per-command proxy. + let state = owner_spec + .build_state_with_audit_metadata( + session.services.network_proxy_audit_metadata.clone(), + ) + .map_err(|error| { + ToolError::Rejected(format!( + "failed to build environment network policy: {error}" + )) + })?; + NetworkProxy::builder() + .state(Arc::new(state)) + .managed_by_codex(/*managed_by_codex*/ false) + .build() + .await + .map_err(|error| { + ToolError::Rejected(format!( + "failed to build execution-scoped network proxy: {error}" + )) + })? + } + } else if let Some(network) = network { + network + } else { + return Ok(None); + }; + let environment_policy = owner_spec.map(|spec| spec.environment_policy()); + let fallback_policy_decider = environment_policy.as_ref().map(|_| { + build_network_policy_decider( + Arc::clone(&session.services.network_approval), + Arc::new(RwLock::new(Arc::downgrade(session))), + ) + }); let registration_id = Uuid::new_v4().to_string(); let attribution_token = Uuid::new_v4().to_string(); let execution_proxy = network - .for_execution(&environment_id, ®istration_id, attribution_token) + .for_execution( + &environment_id, + ®istration_id, + attribution_token, + environment_policy, + fallback_policy_decider, + ) .map_err(|err| { ToolError::Codex(codex_protocol::error::CodexErr::Io(io::Error::other( format!("failed to create execution-scoped network proxy: {err}"), @@ -1059,7 +1135,7 @@ pub(crate) async fn begin_network_approval( .network_approval .register_call(ActiveNetworkApprovalCall { registration_id: registration_id.clone(), - turn_id: turn_id.to_string(), + turn_id: turn.sub_id.clone(), trigger, tool_name, command, diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index fe5272fe5d..709d5ba767 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -6,12 +6,14 @@ simple sequence for any ToolRuntime: approval → select sandbox → attempt → retry with an escalated sandbox strategy on denial (no re‑approval thanks to caching). */ +use crate::config::NetworkProxySpec; use crate::guardian::GuardianReviewContext; use crate::network_policy_decision::network_approval_context_from_payload; use crate::tools::approvals::ApprovalContext; use crate::tools::flat_tool_name; use crate::tools::network_approval::ActiveNetworkApproval; use crate::tools::network_approval::DeferredNetworkApproval; +use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::network_approval::begin_network_approval; use crate::tools::network_approval::finish_deferred_network_approval; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -31,6 +33,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxManager; use codex_sandboxing::SandboxType; +use codex_sandboxing::policy_transforms::effective_network_sandbox_policy; use std::sync::Arc; use std::time::Instant; @@ -55,16 +58,16 @@ impl ToolOrchestrator { req: &Rq, tool_ctx: &ToolCtx, attempt: &SandboxAttempt<'_>, - managed_network_active: bool, + network_approval_spec: Option, ) -> (Result, Option) where T: ToolRuntime, { let network_approval = match begin_network_approval( &tool_ctx.session, - &tool_ctx.step_context.turn.sub_id, - managed_network_active, - tool.network_approval_spec(req, tool_ctx), + &tool_ctx.step_context.turn, + attempt.enforce_managed_network, + network_approval_spec, ) .await { @@ -141,6 +144,17 @@ impl ToolOrchestrator { let mut already_approved = false; let environment = tool.turn_environment(req); + let owner_network_policy = environment.config().network_policy.is_some(); + if owner_network_policy + && tool + .sandbox_permissions(req) + .requires_escalated_permissions() + { + return Err(ToolError::Rejected( + "attachment-owned network policy cannot be bypassed by sandbox escalation" + .to_string(), + )); + } let workspace_roots = environment.workspace_roots(); let executor_managed_process_sandbox = tool.uses_executor_managed_process_sandbox(req); let permission_profile = environment.permission_profile(); @@ -212,12 +226,37 @@ impl ToolOrchestrator { } // 2) First attempt under the selected sandbox. - let sandbox_override = sandbox_override_for_first_attempt( - tool.sandbox_permissions(req), - &requirement, - &file_system_sandbox_policy, - ); - let managed_network_active = turn_ctx.network.is_some(); + let unsandboxed_allowed = + !owner_network_policy && unsandboxed_execution_allowed(&file_system_sandbox_policy); + let sandbox_override = if unsandboxed_allowed { + sandbox_override_for_first_attempt( + tool.sandbox_permissions(req), + &requirement, + &file_system_sandbox_policy, + ) + } else { + SandboxOverride::NoOverride + }; + let network_approval_spec = tool.network_approval_spec(req, tool_ctx); + // Offline owner attachments stay offline unless approved command permissions grant + // networking. Existing enabled controller proxies remain independently authoritative. + let managed_network_active = if owner_network_policy { + turn_ctx + .config + .permissions + .network + .as_ref() + .is_some_and(NetworkProxySpec::enabled) + || network_approval_spec.as_ref().is_some_and(|spec| { + effective_network_sandbox_policy( + permission_profile.network_sandbox_policy(), + spec.trigger.additional_permissions.as_ref(), + ) + .is_enabled() + }) + } else { + turn_ctx.network.is_some() + }; let sandbox_preference = tool.sandbox_preference(); let sandbox_requested = match sandbox_override { SandboxOverride::BypassSandboxFirstAttempt => false, @@ -265,14 +304,8 @@ impl ToolOrchestrator { }; let initial_attempt_start = Instant::now(); - let (first_result, first_deferred_network_approval) = Self::run_attempt( - tool, - req, - tool_ctx, - &initial_attempt, - managed_network_active, - ) - .await; + let (first_result, first_deferred_network_approval) = + Self::run_attempt(tool, req, tool_ctx, &initial_attempt, network_approval_spec).await; let initial_duration = initial_attempt_start.elapsed(); match first_result { Ok(out) => { @@ -327,8 +360,6 @@ impl ToolOrchestrator { ); return Err(ToolError::Codex(err)); } - let unsandboxed_allowed = - unsandboxed_execution_allowed(&file_system_sandbox_policy); // Under `Never` or `OnRequest`, do not retry without sandbox; // surface a concise sandbox denial that preserves the // original output. @@ -449,9 +480,10 @@ impl ToolOrchestrator { }; // Second attempt. + let network_approval_spec = tool.network_approval_spec(req, tool_ctx); let escalated_attempt_start = Instant::now(); let (retry_result, retry_deferred_network_approval) = - Self::run_attempt(tool, req, tool_ctx, &retry_attempt, managed_network_active) + Self::run_attempt(tool, req, tool_ctx, &retry_attempt, network_approval_spec) .await; let escalated_duration = escalated_attempt_start.elapsed(); match retry_result { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index faed4864ef..84144f674d 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -223,9 +223,14 @@ impl<'a> ToolRuntime for UnifiedExecRunt &file_system_sandbox_policy, ); let network = - managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions)?; + managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions) + .cloned(); + // No-proxy fast path; owners still need a spec for execution-only proxies. + if network.is_none() && req.turn_environment.config().network_policy.is_none() { + return None; + } Some(NetworkApprovalSpec { - network: Some(network.clone()), + network, tool_name: ctx.tool_name.clone(), trigger: GuardianNetworkAccessTrigger { call_id: ctx.call_id.clone(), @@ -240,6 +245,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt command: req.hook_command.clone(), environment_id: req.turn_environment.selection.environment_id.clone(), permission_profile: req.turn_environment.permission_profile().clone(), + network_policy: req.turn_environment.config().network_policy.clone(), }) } diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 6d5d89a6cc..481586b03a 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -419,7 +419,12 @@ impl<'a> SandboxAttempt<'a> { &'b self, fallback: Option<&'b NetworkProxy>, ) -> Option<&'b NetworkProxy> { - fallback.map(|fallback| self.network_proxy.unwrap_or(fallback)) + // Execution-only proxies need no fallback; offline attempts must not revive one. + if self.enforce_managed_network { + self.network_proxy.or(fallback) + } else { + None + } } pub fn env_for( diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index 5642216432..4de596b850 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -1,8 +1,11 @@ use anyhow::Context; use anyhow::Result; use codex_config::types::ApprovalsReviewer; +use codex_core::EnvironmentConfig; +use codex_core::EnvironmentNetworkPolicy; use codex_core::TurnInputRequest; use codex_core::config::Constrained; +use codex_core::config::NetworkProxySpec; use codex_core::shell::ShellType; use codex_core::shell::get_shell; use codex_exec_server::CreateDirectoryOptions; @@ -10,6 +13,7 @@ use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::RemoveOptions; use codex_features::Feature; +use codex_network_proxy::NetworkProxyConfig; use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::approvals::NetworkPolicyAmendment; @@ -19,6 +23,7 @@ use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::models::NetworkPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::models::PermissionProfileSnapshot; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EnvironmentConfigState; @@ -1936,6 +1941,198 @@ async fn remote_guardian_network_decisions_are_scoped_to_each_request_and_enviro Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn owner_network_policy_follows_the_selected_remote_command() -> Result<()> { + skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture"); + skip_if_host_windows!(Ok(())); + skip_if_no_network!(Ok(())); + skip_if_sandbox!(Ok(())); + skip_if_no_remote_env!(Ok(())); + + let server = start_mock_server().await; + let mut scenarios = vec![("ROOTED", managed_network_unified_exec_test(&server).await?)]; + for (scenario, configured_controller) in [("ROOTLESS", false), ("USER_ROOTED", true)] { + let mut builder = test_codex().with_config(move |config| { + for feature in [Feature::UnifiedExec, Feature::ExecPermissionApprovals] { + config + .features + .enable(feature) + .expect("test config should allow feature update"); + } + config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + config + .permissions + .set_permission_profile(PermissionProfile::workspace_write_with( + &[], + NetworkSandboxPolicy::Enabled, + /*exclude_tmpdir_env_var*/ false, + /*exclude_slash_tmp*/ false, + )) + .expect("set permission profile"); + config.permissions.network = configured_controller.then(|| { + NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig { + enabled: true, + allow_local_binding: true, + ..NetworkProxyConfig::default() + }, + /*requirements*/ None, + config.permissions.permission_profile(), + ) + .expect("build user-configured controller proxy") + }); + }); + let test = builder.build_with_remote_and_local_env(&server).await?; + assert!(!test.config.managed_network_requirements_enabled()); + assert_eq!( + test.session_configured.network_proxy.is_some(), + configured_controller + ); + scenarios.push((scenario, test)); + } + + for (scenario, test) in scenarios { + let mut remote = test.executor_environment().selection().clone(); + + for (suffix, allowed_domain, expected) in [ + ("ALLOWED", NETWORK_TEST_HOST, "HTTP/1.1 502"), + ("DENIED", "owner-only.invalid", "HTTP/1.1 403"), + ("REVIEWED", "owner-only.invalid", "HTTP/1.1 502"), + ( + "ESCALATED", + NETWORK_TEST_HOST, + "attachment-owned network policy cannot be bypassed", + ), + ("OFFLINE", "owner-only.invalid", "ROOTLESS_OWNER_OFFLINE"), + ("GRANTED_DENIED", "owner-only.invalid", "HTTP/1.1 403"), + ] { + let restricted = matches!(suffix, "OFFLINE" | "GRANTED_DENIED"); + if scenario != "ROOTLESS" && (restricted || suffix == "ESCALATED") { + continue; + } + let marker = format!("{scenario}_OWNER_{suffix}"); + let mut proxy_config = NetworkProxyConfig { + allow_local_binding: true, + ..NetworkProxyConfig::default() + }; + proxy_config.set_allowed_domains(vec![allowed_domain.to_string()]); + let owner_config = EnvironmentConfig { + allow_login_shell: test.config.permissions.allow_login_shell, + permission_profile: PermissionProfileSnapshot::legacy(if restricted { + PermissionProfile::workspace_write() + } else { + test.config.permissions.permission_profile().clone() + }), + shell_environment_policy: test.config.permissions.shell_environment_policy.clone(), + exec_policy: None, + mcp_policy: None, + network_policy: Some(EnvironmentNetworkPolicy::from_config( + &proxy_config, + /*managed_allowed_domains_only*/ suffix != "REVIEWED", + )), + selected_capability_roots: Vec::new(), + }; + let mut primary = local(test.cwd.path().abs()); + primary.config = EnvironmentConfigState::Ready(EnvironmentConfig { + permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::Disabled), + network_policy: None, + ..owner_config.clone() + }); + remote.config = EnvironmentConfigState::Ready(owner_config); + + // Restricted owners run offline until an approved grant enables their filtered proxy. + let command = if suffix == "OFFLINE" { + format!( + "python3 -c \"import socket; sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM); sock.connect(('198.51.100.1', 9))\" 2>/dev/null || printf {marker}" + ) + } else { + remote_network_proxy_request_command(&marker) + }; + let mut args = network_exec_args(&command); + args["environment_id"] = json!(REMOTE_ENVIRONMENT_ID); + if suffix == "ESCALATED" { + args["sandbox_permissions"] = json!("require_escalated"); + args["justification"] = json!("attempt to bypass the owner network policy"); + } else if suffix == "GRANTED_DENIED" { + args["sandbox_permissions"] = json!("with_additional_permissions"); + args["additional_permissions"] = json!({"network": {"enabled": true}}); + args["justification"] = json!("exercise attachment-scoped network access"); + } + let guardian = if suffix == "REVIEWED" { + Some( + mount_sse_once_match( + &server, + is_guardian_request, + sse(vec![ + ev_response_created("resp-owner-network-guardian"), + ev_assistant_message( + "msg-owner-network-guardian", + r#"{"outcome":"allow"}"#, + ), + ev_completed("resp-owner-network-guardian"), + ]), + ) + .await, + ) + } else { + None + }; + let responses = mount_exec_network_turn(&server, &marker, &marker, args).await?; + submit_managed_network_turn( + &test, + "exercise the selected environment's network policy", + vec![primary, remote.clone()], + if suffix == "REVIEWED" { + ApprovalsReviewer::AutoReview + } else { + ApprovalsReviewer::User + }, + if matches!(suffix, "REVIEWED" | "ESCALATED" | "GRANTED_DENIED") { + AskForApproval::OnRequest + } else { + AskForApproval::Never + }, + ) + .await?; + if suffix == "GRANTED_DENIED" { + let event = wait_for_event(&test.codex, |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }) + .await; + let EventMsg::ExecApprovalRequest(approval) = event else { + anyhow::bail!("expected additional permissions approval before completion") + }; + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: Some(approval.turn_id), + decision: ReviewDecision::Approved, + }) + .await?; + } + wait_for_completion_without_network_prompt(&test).await; + if let Some(guardian) = guardian { + assert_eq!( + guardian_network_triggers(&[&guardian])?, + vec![(marker.clone(), command)] + ); + } + let output = responses + .function_call_output_text(&marker) + .context("expected remote network output")?; + assert!( + output.contains(expected), + "unexpected network output for {marker}: {output}" + ); + } + } + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn approved_network_host_for_one_environment_still_prompts_in_another() -> Result<()> { skip_if_target_windows!(Ok(()), "uses the POSIX/Python network fixture"); diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 1fad9556eb..6e0c3161e1 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -1425,7 +1425,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn owner_network_policy_is_rejected_until_runtime_enforcement_exists() -> Result<()> { +async fn owner_network_policy_rejects_unsupported_environment_authority() -> Result<()> { let server = start_mock_server().await; let test = test_codex().build_with_auto_env(&server).await?; let selections = test.codex.environment_selections().await; @@ -1434,9 +1434,7 @@ async fn owner_network_policy_is_rejected_until_runtime_enforcement_exists() -> .context("thread should select its executor environment")?; let owner_config = EnvironmentConfig { allow_login_shell: test.config.permissions.allow_login_shell, - permission_profile: PermissionProfileSnapshot::legacy( - test.config.permissions.permission_profile().clone(), - ), + permission_profile: PermissionProfileSnapshot::legacy(PermissionProfile::Disabled), shell_environment_policy: test.config.permissions.shell_environment_policy.clone(), exec_policy: None, mcp_policy: None, @@ -1460,15 +1458,23 @@ async fn owner_network_policy_is_rejected_until_runtime_enforcement_exists() -> }) .await .err() - .context("preview must not accept an unenforced policy")?; + .context("preview must not accept an unsupported environment policy")?; let ready_error = test .codex .environment_ready(selection, owner_config) .await - .expect_err("readiness must not accept an unenforced policy"); + .expect_err("readiness must not accept an unsupported environment policy"); + let expected = if selection.environment_id == LOCAL_ENVIRONMENT_ID { + "attachment-owned network policy requires a remote executor" + } else { + "environment network policy requires managed network enforcement" + }; for error in [preview_error.to_string(), ready_error.to_string()] { - assert!(error.contains("attachment-owned network policy is not supported yet")); + assert!( + error.contains(expected), + "unexpected validation error: {error}" + ); } assert_eq!(test.codex.environment_selections().await, selections); Ok(()) diff --git a/codex-rs/network-proxy/src/environment_policy.rs b/codex-rs/network-proxy/src/environment_policy.rs index 2cd5f264a3..365f7b2690 100644 --- a/codex-rs/network-proxy/src/environment_policy.rs +++ b/codex-rs/network-proxy/src/environment_policy.rs @@ -1,13 +1,14 @@ use crate::NetworkDomainPermissions; use crate::NetworkProxyConfig; +use crate::NetworkUnixSocketPermission; use crate::NetworkUnixSocketPermissions; use serde::Deserialize; use serde::Serialize; /// Traffic restrictions supplied by the owner of one execution environment. /// -/// Proxy enablement, listeners, network mode, MITM, and credentials remain owned by the -/// controller's network-proxy runtime. +/// Proxy enablement, listeners, network mode, MITM, and credentials remain outside +/// attachment-owned traffic policy. #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct EnvironmentNetworkPolicy { @@ -31,4 +32,53 @@ impl EnvironmentNetworkPolicy { managed_allowed_domains_only, } } + + /// Applies attachment-owned traffic settings while preserving inherited denials and proxy setup. + pub fn apply_to(&self, config: &mut NetworkProxyConfig) { + // Use the owner's domain rules without dropping controller denials. + let inherited_denials = config.denied_domains().unwrap_or_default(); + config.domains.clone_from(&self.domains); + for domain in inherited_denials { + config.upsert_domain_permission( + domain, + crate::NetworkDomainPermission::Deny, + crate::normalize_host, + ); + } + let inherited_sockets = config.unix_sockets.take().unwrap_or_default(); + let mut effective_sockets = self.unix_sockets.clone().unwrap_or_default(); + + // "Allow all" cannot override a socket denied by either policy. + let inherited_permits_all = config.dangerously_allow_all_unix_sockets + && !inherited_sockets + .entries + .values() + .any(|permission| matches!(permission, NetworkUnixSocketPermission::Deny)); + let owner_permits_all = self.dangerously_allow_all_unix_sockets + && !effective_sockets + .entries + .values() + .any(|permission| matches!(permission, NetworkUnixSocketPermission::Deny)); + + // Keep shared socket grants; controller denials always take priority. + effective_sockets.entries.retain(|path, permission| { + matches!(permission, NetworkUnixSocketPermission::Deny) + || inherited_permits_all + || matches!( + inherited_sockets.entries.get(path), + Some(NetworkUnixSocketPermission::Allow) + ) + }); + for (path, permission) in inherited_sockets.entries { + if owner_permits_all || matches!(permission, NetworkUnixSocketPermission::Deny) { + effective_sockets.entries.insert(path, permission); + } + } + + // Enable permissions only when both controller and owner allow them. + config.unix_sockets = (!effective_sockets.entries.is_empty()).then_some(effective_sockets); + config.dangerously_allow_all_unix_sockets = inherited_permits_all && owner_permits_all; + config.allow_upstream_proxy &= self.allow_upstream_proxy; + config.allow_local_binding &= self.allow_local_binding; + } } diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 6577259405..c723686fed 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -867,6 +867,7 @@ impl NetworkProxy { pub async fn remote_launch_config(&self) -> Result { let (mut config, brokerage_created_default_allowlist) = self.state.current_cfg_with_brokerage_provenance().await?; + // Proxy enablement and credential brokerage remain controller-owned. let mut broker_only_config = config::NetworkProxyConfig { enabled: config.enabled, credential_broker_openai_host: config.credential_broker_openai_host.clone(), @@ -877,6 +878,14 @@ impl NetworkProxy { broker_only_config.set_credential_broker_enabled(/*enabled*/ true); broker_only_config.set_allowed_domains(vec!["*".to_string()]); let broker_only = brokerage_created_default_allowlist && config == broker_only_config; + + let environment_policy = self + .execution_scope + .as_ref() + .and_then(|scope| scope.environment_policy.as_ref()); + if let Some(policy) = environment_policy { + policy.apply_to(&mut config); + } if config.credential_broker { config.enabled &= !broker_only; config.credential_broker = false; @@ -885,6 +894,10 @@ impl NetworkProxy { config.mitm = false; } } + anyhow::ensure!( + environment_policy.is_none() || config.enabled, + "environment network policy requires an enabled executor proxy" + ); let proxy = crate::RemoteNetworkProxyConfig::from_effective_config(&config)?; let (environment_id, execution_id) = self .execution_scope @@ -913,6 +926,7 @@ impl NetworkProxy { let state = Arc::clone(&self.state); let environment_id = scope.environment_id.clone(); let execution_id = scope.execution_id.clone(); + let environment_policy_applies = scope.environment_policy.is_some(); let execution_lifetime = scope.lifetime_tx.subscribe(); Some(Arc::new(move |mut request: crate::NetworkPolicyRequest| { let decider = Arc::clone(&decider); @@ -928,8 +942,12 @@ impl NetworkProxy { } decision = async { match state.host_blocked(&request.host, request.port).await { - Ok(HostBlockDecision::Allowed) => NetworkDecision::Allow, - Ok(HostBlockDecision::Blocked(HostBlockReason::NotAllowed)) => { + // Controller approval alone cannot bypass attachment policy. + Ok(HostBlockDecision::Allowed) if !environment_policy_applies => { + NetworkDecision::Allow + } + Ok(HostBlockDecision::Allowed) + | Ok(HostBlockDecision::Blocked(HostBlockReason::NotAllowed)) => { decider.decide(request).await } Ok(HostBlockDecision::Blocked(reason)) => { @@ -1710,13 +1728,15 @@ mod tests { async fn remote_policy_decider_rechecks_live_policy_and_restores_attribution() -> Result<()> { let captured = Arc::new(Mutex::new(None)); let captured_request = Arc::clone(&captured); - let config = NetworkProxyConfig { + let mut config = NetworkProxyConfig { allow_local_binding: true, ..NetworkProxyConfig::default() }; - let proxy = NetworkProxy::builder() - .state(Arc::new(network_proxy_state_for_policy(config))) - .policy_decider(move |request: crate::NetworkPolicyRequest| { + config.set_allowed_domains(vec!["controller.example".to_string()]); + let mut owner_config = NetworkProxyConfig::default(); + owner_config.set_allowed_domains(vec!["owner.example".to_string()]); + let fallback_policy_decider: Arc = + Arc::new(move |request: crate::NetworkPolicyRequest| { let captured = Arc::clone(&captured_request); async move { *captured @@ -1725,15 +1745,26 @@ mod tests { Some((request.host, request.environment_id, request.execution_id)); crate::NetworkDecision::Allow } - }) + }); + let proxy = NetworkProxy::builder() + .state(Arc::new(network_proxy_state_for_policy(config))) .managed_by_codex(/*managed_by_codex*/ false) .build() .await?; - let scoped = proxy.for_execution("remote", "execution-1", "token-1".to_string())?; + let scoped = proxy.for_execution( + "remote", + "execution-1", + "token-1".to_string(), + Some(crate::EnvironmentNetworkPolicy::from_config( + &owner_config, + /*managed_allowed_domains_only*/ false, + )), + Some(Arc::clone(&fallback_policy_decider)), + )?; let decider = scoped .remote_policy_decider() .expect("execution-scoped proxy should expose its policy decider"); - let mut request = https_request("allowed.example"); + let mut request = https_request("controller.example"); request.environment_id = Some("forged-environment".to_string()); request.execution_id = Some("forged-execution".to_string()); @@ -1743,7 +1774,7 @@ mod tests { .lock() .unwrap_or_else(std::sync::PoisonError::into_inner), Some(( - "allowed.example".to_string(), + "controller.example".to_string(), Some("remote".to_string()), Some("execution-1".to_string()) )) @@ -1762,11 +1793,28 @@ mod tests { .lock() .unwrap_or_else(std::sync::PoisonError::into_inner), Some(( - "allowed.example".to_string(), + "controller.example".to_string(), Some("remote".to_string()), Some("execution-1".to_string()) )) ); + + owner_config.set_denied_domains(vec!["denied.example".to_string()]); + assert_eq!( + scoped.remote_launch_config().await?.proxy.domains, + owner_config.domains + ); + let strict_scoped = proxy.for_execution( + "owner-environment", + "execution-3", + "token-3".to_string(), + Some(crate::EnvironmentNetworkPolicy::from_config( + &owner_config, + /*managed_allowed_domains_only*/ true, + )), + Some(fallback_policy_decider), + )?; + assert!(strict_scoped.remote_policy_decider().is_none()); Ok(()) } @@ -1790,7 +1838,13 @@ mod tests { .managed_by_codex(/*managed_by_codex*/ false) .build() .await?; - let scoped = proxy.for_execution("remote", "execution-1", "token-1".to_string())?; + let scoped = proxy.for_execution( + "remote", + "execution-1", + "token-1".to_string(), + /*environment_policy*/ None, + /*fallback_policy_decider*/ None, + )?; let decider = scoped .remote_policy_decider() .expect("execution-scoped proxy should expose its policy decider"); @@ -2052,7 +2106,13 @@ mod tests { } }; - let scoped = proxy.for_execution("remote-env", "execution-1", "token-1".to_string())?; + let scoped = proxy.for_execution( + "remote-env", + "execution-1", + "token-1".to_string(), + /*environment_policy*/ None, + /*fallback_policy_decider*/ None, + )?; let launch = scoped.remote_launch_config().await?; let prepared = scoped.prepare_for_optional_environment( HashMap::from([( @@ -2065,6 +2125,20 @@ mod tests { assert_eq!(launch.environment_id.as_deref(), Some("remote-env")); assert_eq!(launch.execution_id.as_deref(), Some("execution-1")); assert!(!launch.proxy.enabled); + + let mut owner_config = NetworkProxyConfig::default(); + owner_config.set_allowed_domains(vec!["owner.example".to_string()]); + let owner_scoped = proxy.for_execution( + "remote-env", + "execution-2", + "token-2".to_string(), + Some(crate::EnvironmentNetworkPolicy::from_config( + &owner_config, + /*managed_allowed_domains_only*/ false, + )), + /*fallback_policy_decider*/ None, + )?; + assert!(owner_scoped.remote_launch_config().await.is_err()); for (enabled, mode, allowed_domain, proxy_url) in [ (false, config::NetworkMode::Full, Some("*"), None), (false, config::NetworkMode::Full, Some("example.com"), None), diff --git a/codex-rs/network-proxy/src/proxy/execution_scope.rs b/codex-rs/network-proxy/src/proxy/execution_scope.rs index ac5fecbaee..e01e115e78 100644 --- a/codex-rs/network-proxy/src/proxy/execution_scope.rs +++ b/codex-rs/network-proxy/src/proxy/execution_scope.rs @@ -4,6 +4,7 @@ pub(super) struct ExecutionScope { pub(super) environment_id: String, pub(super) execution_id: String, pub(super) attribution_token: String, + pub(super) environment_policy: Option, // Dropping the execution scope closes this channel and cancels remote reviews. pub(super) lifetime_tx: tokio::sync::watch::Sender<()>, state: Arc, @@ -16,12 +17,14 @@ impl Drop for ExecutionScope { } impl NetworkProxy { - /// Returns a proxy that attributes trusted bridge connections to one execution. + /// Returns a proxy with execution-specific attribution and attachment policy. pub fn for_execution( &self, environment_id: &str, execution_id: &str, attribution_token: String, + environment_policy: Option, + fallback_policy_decider: Option>, ) -> Result { anyhow::ensure!( self.execution_scope.is_none(), @@ -32,10 +35,16 @@ impl NetworkProxy { let (lifetime_tx, _) = tokio::sync::watch::channel(()); let mut proxy = self.clone(); + proxy.policy_decider = proxy.policy_decider.or(fallback_policy_decider); + // Strict attachment allowlists cannot be expanded through approval callbacks. + if matches!(&environment_policy, Some(policy) if policy.managed_allowed_domains_only) { + proxy.policy_decider = None; + } proxy.execution_scope = Some(Arc::new(ExecutionScope { environment_id: environment_id.to_string(), execution_id: execution_id.to_string(), attribution_token, + environment_policy, lifetime_tx, state: Arc::clone(&self.state), })); diff --git a/codex-rs/protocol/src/environment.rs b/codex-rs/protocol/src/environment.rs index c61dd94b1c..21e7275ee3 100644 --- a/codex-rs/protocol/src/environment.rs +++ b/codex-rs/protocol/src/environment.rs @@ -33,7 +33,6 @@ pub struct EnvironmentConfig { /// Additional managed MCP restrictions for this environment attachment. pub mcp_policy: Option, /// Owner-provided traffic restrictions. `None` keeps the existing controller policy. - /// Core rejects `Some` until attachment-owned network enforcement is implemented. pub network_policy: Option, /// Capability roots selected for this thread's environment attachment. pub selected_capability_roots: Vec,