diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 6af66294c..21d8784c3 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -1438,6 +1438,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -3315,6 +3326,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { @@ -6308,6 +6343,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index c6b84ac5d..6b12cee9c 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -2059,6 +2059,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -4207,6 +4218,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index a3ef24452..5f352004c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -3451,6 +3451,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -6263,6 +6274,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "NewConversationParams": { "properties": { "approvalPolicy": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index 12db241e1..225ea3f42 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -1438,6 +1438,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -3315,6 +3326,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index dd7606b5e..2b305281c 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -1438,6 +1438,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -3315,6 +3326,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index 89ab825d3..d4bbf908d 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -1438,6 +1438,17 @@ "description": "The command's working directory.", "type": "string" }, + "network_approval_context": { + "anyOf": [ + { + "$ref": "#/definitions/NetworkApprovalContext" + }, + { + "type": "null" + } + ], + "description": "Optional network context for a blocked request that can be approved." + }, "parsed_cmd": { "items": { "$ref": "#/definitions/ParsedCommand" @@ -3315,6 +3326,30 @@ ], "type": "string" }, + "NetworkApprovalContext": { + "properties": { + "host": { + "type": "string" + }, + "protocol": { + "$ref": "#/definitions/NetworkApprovalProtocol" + } + }, + "required": [ + "host", + "protocol" + ], + "type": "object" + }, + "NetworkApprovalProtocol": { + "enum": [ + "http", + "https", + "socks5_tcp", + "socks5_udp" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index 66db8fd40..542ed879b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -2,6 +2,7 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { ExecPolicyAmendment } from "./ExecPolicyAmendment"; +import type { NetworkApprovalContext } from "./NetworkApprovalContext"; import type { ParsedCommand } from "./ParsedCommand"; export type ExecApprovalRequestEvent = { @@ -26,6 +27,10 @@ cwd: string, * Optional human-readable reason for the approval (e.g. retry without sandbox). */ reason: string | null, +/** + * Optional network context for a blocked request that can be approved. + */ +network_approval_context?: NetworkApprovalContext, /** * Proposed execpolicy amendment that can be applied to allow future runs. */ diff --git a/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalContext.ts b/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalContext.ts new file mode 100644 index 000000000..b4b78e473 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalContext.ts @@ -0,0 +1,6 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; + +export type NetworkApprovalContext = { host: string, protocol: NetworkApprovalProtocol, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalProtocol.ts b/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalProtocol.ts new file mode 100644 index 000000000..a33eab566 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/NetworkApprovalProtocol.ts @@ -0,0 +1,5 @@ +// GENERATED CODE! DO NOT MODIFY BY HAND! + +// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. + +export type NetworkApprovalProtocol = "http" | "https" | "socks5_tcp" | "socks5_udp"; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index ef17362ff..3c0826712 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -126,6 +126,8 @@ export type { McpToolCallEndEvent } from "./McpToolCallEndEvent"; export type { MessagePhase } from "./MessagePhase"; export type { ModeKind } from "./ModeKind"; export type { NetworkAccess } from "./NetworkAccess"; +export type { NetworkApprovalContext } from "./NetworkApprovalContext"; +export type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; export type { NewConversationParams } from "./NewConversationParams"; export type { NewConversationResponse } from "./NewConversationResponse"; export type { ParsedCommand } from "./ParsedCommand"; diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index c1f36e1ef..da7abe059 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -206,6 +206,7 @@ pub(crate) async fn apply_bespoke_event_handling( reason, proposed_execpolicy_amendment, parsed_cmd, + .. }) => match api_version { ApiVersion::V1 => { let params = ExecCommandApprovalParams { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index b7bffa70d..0439068db 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -1688,8 +1688,18 @@ impl CodexMessageProcessor { let timeout_ms = params .timeout_ms .and_then(|timeout_ms| u64::try_from(timeout_ms).ok()); + let managed_network_requirements_enabled = + self.config.managed_network_requirements_enabled(); let started_network_proxy = match self.config.permissions.network.as_ref() { - Some(spec) => match spec.start_proxy().await { + Some(spec) => match spec + .start_proxy( + self.config.permissions.sandbox_policy.get(), + None, + None, + managed_network_requirements_enabled, + ) + .await + { Ok(started) => Some(started), Err(err) => { let error = JSONRPCErrorError { @@ -1712,6 +1722,7 @@ impl CodexMessageProcessor { network: started_network_proxy .as_ref() .map(codex_core::config::StartedNetworkProxy::proxy), + network_attempt_id: None, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level, justification: None, diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index c19d58189..a65fa486e 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -213,12 +213,19 @@ async fn run_command_under_sandbox( #[cfg(not(target_os = "macos"))] let _ = log_denials; + let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); + // This proxy should only live for the lifetime of the child process. let network_proxy = match config.permissions.network.as_ref() { Some(spec) => Some( - spec.start_proxy() - .await - .map_err(|err| anyhow::anyhow!("failed to start managed network proxy: {err}"))?, + spec.start_proxy( + config.permissions.sandbox_policy.get(), + None, + None, + managed_network_requirements_enabled, + ) + .await + .map_err(|err| anyhow::anyhow!("failed to start managed network proxy: {err}"))?, ), None => None, }; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 05cb42e9e..753337eb6 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -170,6 +170,7 @@ use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::ExecApprovalRequestEvent; use crate::protocol::McpServerRefreshConfig; +use crate::protocol::NetworkApprovalContext; use crate::protocol::Op; use crate::protocol::PlanDeltaEvent; use crate::protocol::RateLimitSnapshot; @@ -223,6 +224,9 @@ use crate::tasks::SessionTaskContext; use crate::tools::ToolRouter; use crate::tools::context::SharedTurnDiffTracker; use crate::tools::js_repl::JsReplHandle; +use crate::tools::network_approval::NetworkApprovalService; +use crate::tools::network_approval::build_blocked_request_observer; +use crate::tools::network_approval::build_network_policy_decider; use crate::tools::parallel::ToolCallRuntime; use crate::tools::sandboxing::ApprovalStore; use crate::tools::spec::ToolsConfig; @@ -519,7 +523,6 @@ pub(crate) struct Session { js_repl: Arc, next_internal_sub_id: AtomicU64, } - /// The context needed for a single turn of the thread. #[derive(Debug)] pub(crate) struct TurnContext { @@ -822,6 +825,33 @@ impl Session { } } + async fn start_managed_network_proxy( + spec: &crate::config::NetworkProxySpec, + sandbox_policy: &SandboxPolicy, + network_policy_decider: Option>, + blocked_request_observer: Option>, + managed_network_requirements_enabled: bool, + ) -> anyhow::Result<(StartedNetworkProxy, SessionNetworkProxyRuntime)> { + let network_proxy = spec + .start_proxy( + sandbox_policy, + network_policy_decider, + blocked_request_observer, + managed_network_requirements_enabled, + ) + .await + .map_err(|err| anyhow::anyhow!("failed to start managed network proxy: {err}"))?; + let session_network_proxy = { + let proxy = network_proxy.proxy(); + SessionNetworkProxyRuntime { + http_addr: proxy.http_addr().to_string(), + socks_addr: proxy.socks_addr().to_string(), + admin_addr: proxy.admin_addr().to_string(), + } + }; + Ok((network_proxy, session_network_proxy)) + } + /// Don't expand the number of mutated arguments on config. We are in the process of getting rid of it. pub(crate) fn build_per_turn_config(session_configuration: &SessionConfiguration) -> Config { // todo(aibrahim): store this state somewhere else so we don't need to mut config @@ -1178,21 +1208,50 @@ impl Session { }; session_configuration.thread_name = thread_name.clone(); let mut state = SessionState::new(session_configuration.clone()); - let network_proxy = - match config.permissions.network.as_ref() { - Some(spec) => Some(spec.start_proxy().await.map_err(|err| { - anyhow::anyhow!("failed to start managed network proxy: {err}") - })?), - None => None, + let managed_network_requirements_enabled = config.managed_network_requirements_enabled(); + let network_approval = Arc::new(NetworkApprovalService::default()); + // The managed proxy can call back into core for allowlist-miss decisions. + let network_policy_decider_session = if managed_network_requirements_enabled { + config + .permissions + .network + .as_ref() + .map(|_| Arc::new(RwLock::new(std::sync::Weak::::new()))) + } else { + None + }; + let blocked_request_observer = if managed_network_requirements_enabled { + config + .permissions + .network + .as_ref() + .map(|_| build_blocked_request_observer(Arc::clone(&network_approval))) + } else { + None + }; + let network_policy_decider = + network_policy_decider_session + .as_ref() + .map(|network_policy_decider_session| { + build_network_policy_decider( + Arc::clone(&network_approval), + Arc::clone(network_policy_decider_session), + ) + }); + let (network_proxy, session_network_proxy) = + if let Some(spec) = config.permissions.network.as_ref() { + let (network_proxy, session_network_proxy) = Self::start_managed_network_proxy( + spec, + config.permissions.sandbox_policy.get(), + network_policy_decider.as_ref().map(Arc::clone), + blocked_request_observer.as_ref().map(Arc::clone), + managed_network_requirements_enabled, + ) + .await?; + (Some(network_proxy), Some(session_network_proxy)) + } else { + (None, None) }; - let session_network_proxy = network_proxy.as_ref().map(|started| { - let proxy = started.proxy(); - SessionNetworkProxyRuntime { - http_addr: proxy.http_addr().to_string(), - socks_addr: proxy.socks_addr().to_string(), - admin_addr: proxy.admin_addr().to_string(), - } - }); let services = SessionServices { mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), @@ -1218,6 +1277,7 @@ impl Session { file_watcher, agent_control, network_proxy, + network_approval: Arc::clone(&network_approval), state_db: state_db_ctx.clone(), model_client: ModelClient::new( Some(Arc::clone(&auth_manager)), @@ -1260,6 +1320,10 @@ impl Session { js_repl, next_internal_sub_id: AtomicU64::new(0), }); + if let Some(network_policy_decider_session) = network_policy_decider_session { + let mut guard = network_policy_decider_session.write().await; + *guard = Arc::downgrade(&sess); + } // Dispatch the SessionConfiguredEvent first and then report any errors. // If resuming, include converted initial messages in the payload so UIs can render them immediately. @@ -2082,7 +2146,7 @@ impl Session { Ok(()) } - async fn turn_context_for_sub_id(&self, sub_id: &str) -> Option> { + pub(crate) async fn turn_context_for_sub_id(&self, sub_id: &str) -> Option> { let active = self.active_turn.lock().await; active .as_ref() @@ -2144,6 +2208,7 @@ impl Session { command: Vec, cwd: PathBuf, reason: Option, + network_approval_context: Option, proposed_execpolicy_amendment: Option, ) -> ReviewDecision { // Add the tx_approve callback to the map before sending the request. @@ -2170,6 +2235,7 @@ impl Session { command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, parsed_cmd, }); @@ -6948,6 +7014,7 @@ mod tests { let mut state = SessionState::new(session_configuration.clone()); mark_state_initial_context_seeded(&mut state); let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); + let network_approval = Arc::new(NetworkApprovalService::default()); let file_watcher = Arc::new(FileWatcher::noop()); let services = SessionServices { @@ -6974,6 +7041,7 @@ mod tests { file_watcher, agent_control, network_proxy: None, + network_approval: Arc::clone(&network_approval), state_db: None, model_client: ModelClient::new( Some(auth_manager.clone()), @@ -7094,6 +7162,7 @@ mod tests { let mut state = SessionState::new(session_configuration.clone()); mark_state_initial_context_seeded(&mut state); let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); + let network_approval = Arc::new(NetworkApprovalService::default()); let file_watcher = Arc::new(FileWatcher::noop()); let services = SessionServices { @@ -7120,6 +7189,7 @@ mod tests { file_watcher, agent_control, network_proxy: None, + network_approval: Arc::clone(&network_approval), state_db: None, model_client: ModelClient::new( Some(Arc::clone(&auth_manager)), @@ -7882,6 +7952,7 @@ mod tests { expiration: timeout_ms.into(), env: HashMap::new(), network: None, + network_attempt_id: None, sandbox_permissions, windows_sandbox_level: turn_context.windows_sandbox_level, justification: Some("test".to_string()), @@ -7895,6 +7966,7 @@ mod tests { expiration: timeout_ms.into(), env: HashMap::new(), network: None, + network_attempt_id: None, windows_sandbox_level: turn_context.windows_sandbox_level, justification: params.justification.clone(), arg0: None, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 1f4240f68..69efeefec 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -316,6 +316,7 @@ async fn handle_exec_approval( command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, .. } = event; @@ -327,6 +328,7 @@ async fn handle_exec_approval( command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, ); let decision = diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 91ab7e5b7..5d887a6a5 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1950,6 +1950,13 @@ impl Config { self.permissions.windows_sandbox_mode }; } + + pub fn managed_network_requirements_enabled(&self) -> bool { + self.config_layer_stack + .requirements_toml() + .network + .is_some() + } } pub(crate) fn uses_deprecated_instructions_file(config_layer_stack: &ConfigLayerStack) -> bool { diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index ee3e8ebed..183eafcb1 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -1,8 +1,11 @@ use crate::config; use crate::config_loader::NetworkConstraints; use async_trait::async_trait; +use codex_network_proxy::BlockedRequestObserver; use codex_network_proxy::ConfigReloader; use codex_network_proxy::ConfigState; +use codex_network_proxy::NetworkDecision; +use codex_network_proxy::NetworkPolicyDecider; use codex_network_proxy::NetworkProxy; use codex_network_proxy::NetworkProxyConfig; use codex_network_proxy::NetworkProxyConstraints; @@ -11,6 +14,7 @@ use codex_network_proxy::NetworkProxyState; use codex_network_proxy::build_config_state; use codex_network_proxy::host_and_port_from_network_addr; use codex_network_proxy::validate_policy_against_constraints; +use codex_protocol::protocol::SandboxPolicy; use std::sync::Arc; #[derive(Debug, Clone, PartialEq, Eq)] @@ -92,20 +96,41 @@ impl NetworkProxySpec { }) } - pub async fn start_proxy(&self) -> std::io::Result { + pub async fn start_proxy( + &self, + sandbox_policy: &SandboxPolicy, + policy_decider: Option>, + blocked_request_observer: Option>, + enable_network_approval_flow: bool, + ) -> std::io::Result { let state = build_config_state(self.config.clone(), self.constraints.clone()).map_err(|err| { std::io::Error::other(format!("failed to build network proxy state: {err}")) })?; let reloader = Arc::new(StaticNetworkProxyReloader::new(state.clone())); let state = NetworkProxyState::with_reloader(state, reloader); - let proxy = NetworkProxy::builder() - .state(Arc::new(state)) - .build() - .await - .map_err(|err| { - std::io::Error::other(format!("failed to build network proxy: {err}")) - })?; + let mut builder = NetworkProxy::builder().state(Arc::new(state)); + if enable_network_approval_flow + && matches!( + sandbox_policy, + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } + ) + { + builder = match policy_decider { + Some(policy_decider) => builder.policy_decider_arc(policy_decider), + None => builder.policy_decider(|_request| async { + // In restricted sandbox modes, allowlist misses should ask for + // explicit network approval instead of hard-denying. + NetworkDecision::ask("not_allowed") + }), + }; + } + if let Some(blocked_request_observer) = blocked_request_observer { + builder = builder.blocked_request_observer_arc(blocked_request_observer); + } + let proxy = builder.build().await.map_err(|err| { + std::io::Error::other(format!("failed to build network proxy: {err}")) + })?; let handle = proxy .run() .await diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 404fbc5a7..ad49c611f 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -1,4 +1,5 @@ use crate::exec::ExecToolCallOutput; +use crate::network_policy_decision::NetworkPolicyDecisionPayload; use crate::token_data::KnownPlan; use crate::token_data::PlanType; use crate::truncate::TruncationPolicy; @@ -31,7 +32,10 @@ pub enum SandboxErr { "sandbox denied exec error, exit code: {}, stdout: {}, stderr: {}", .output.exit_code, .output.stdout.text, .output.stderr.text )] - Denied { output: Box }, + Denied { + output: Box, + network_policy_decision: Option, + }, /// Error from linux seccomp filter setup #[cfg(target_os = "linux")] @@ -616,7 +620,7 @@ impl CodexErr { pub fn get_error_message_ui(e: &CodexErr) -> String { let message = match e { - CodexErr::Sandbox(SandboxErr::Denied { output }) => { + CodexErr::Sandbox(SandboxErr::Denied { output, .. }) => { let aggregated = output.aggregated_output.text.trim(); if !aggregated.is_empty() { output.aggregated_output.text.clone() @@ -736,6 +740,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "aggregate detail"); } @@ -752,6 +757,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "stderr detail\nstdout detail"); } @@ -768,6 +774,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!(get_error_message_ui(&err), "stdout only"); } @@ -811,6 +818,7 @@ mod tests { }; let err = CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, }); assert_eq!( get_error_message_ui(&err), diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 398a767ce..0f0f0609a 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -15,6 +15,7 @@ use tokio::io::AsyncReadExt; use tokio::io::BufReader; use tokio::process::Child; use tokio_util::sync::CancellationToken; +use uuid::Uuid; use crate::error::CodexErr; use crate::error::Result; @@ -66,6 +67,7 @@ pub struct ExecParams { pub expiration: ExecExpiration, pub env: HashMap, pub network: Option, + pub network_attempt_id: Option, pub sandbox_permissions: SandboxPermissions, pub windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel, pub justification: Option, @@ -184,13 +186,15 @@ pub async fn process_exec_tool_call( mut env, expiration, network, + network_attempt_id, sandbox_permissions, windows_sandbox_level, justification, arg0: _, } = params; + let network_attempt_id = network_attempt_id.map(|attempt_id| attempt_id.to_string()); if let Some(network) = network.as_ref() { - network.apply_to_env(&mut env); + network.apply_to_env_for_attempt(&mut env, network_attempt_id.as_deref()); } let (program, args) = command.split_first().ok_or_else(|| { CodexErr::Io(io::Error::new( @@ -238,6 +242,7 @@ pub(crate) async fn execute_exec_env( cwd, env, network, + network_attempt_id, expiration, sandbox, windows_sandbox_level, @@ -246,12 +251,18 @@ pub(crate) async fn execute_exec_env( arg0, } = env; + let network_attempt_id = match network_attempt_id.as_deref() { + Some(attempt_id) => Uuid::parse_str(attempt_id).ok(), + None => network.as_ref().map(|_| Uuid::new_v4()), + }; + let params = ExecParams { command, cwd, expiration, env, - network, + network: network.clone(), + network_attempt_id, sandbox_permissions, windows_sandbox_level, justification, @@ -345,12 +356,14 @@ async fn exec_windows_sandbox( cwd, mut env, network, + network_attempt_id, expiration, windows_sandbox_level, .. } = params; + let network_attempt_id = network_attempt_id.map(|attempt_id| attempt_id.to_string()); if let Some(network) = network.as_ref() { - network.apply_to_env(&mut env); + network.apply_to_env_for_attempt(&mut env, network_attempt_id.as_deref()); } // TODO(iceweasel-oai): run_windows_sandbox_capture should support all @@ -490,6 +503,7 @@ fn finalize_exec_result( if is_likely_sandbox_denied(sandbox_type, &exec_output) { return Err(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(exec_output), + network_policy_decision: None, })); } @@ -701,13 +715,19 @@ async fn exec( let ExecParams { command, cwd, - env, + mut env, network, + network_attempt_id, arg0, expiration, windows_sandbox_level: _, .. } = params; + let network_attempt_id = network_attempt_id.map(|attempt_id| attempt_id.to_string()); + + if let Some(network) = network.as_ref() { + network.apply_to_env_for_attempt(&mut env, network_attempt_id.as_deref()); + } let (program, args) = command.split_first().ok_or_else(|| { CodexErr::Io(io::Error::new( @@ -722,7 +742,10 @@ async fn exec( arg0: arg0_ref, cwd, sandbox_policy, - network: network.as_ref(), + // The environment already has attempt-scoped proxy settings from + // apply_to_env_for_attempt above. Passing network here would reapply + // non-attempt proxy vars and drop attempt correlation metadata. + network: None, stdio_policy: StdioPolicy::RedirectForShellTool, env, }) @@ -951,6 +974,17 @@ mod tests { assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); } + #[test] + fn sandbox_detection_ignores_network_policy_text_in_non_sandbox_mode() { + let output = make_exec_output( + 0, + "", + "", + r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","reason":"not_allowed","source":"decider","protocol":"http","host":"google.com","port":80}"#, + ); + assert!(!is_likely_sandbox_denied(SandboxType::None, &output)); + } + #[test] fn sandbox_detection_uses_aggregated_output() { let output = make_exec_output( @@ -965,6 +999,21 @@ mod tests { )); } + #[test] + fn sandbox_detection_ignores_network_policy_text_with_zero_exit_code() { + let output = make_exec_output( + 0, + "", + "", + r#"CODEX_NETWORK_POLICY_DECISION {"decision":"ask","source":"decider","protocol":"http","host":"google.com","port":80}"#, + ); + + assert!(!is_likely_sandbox_denied( + SandboxType::LinuxSeccomp, + &output + )); + } + #[tokio::test] async fn read_capped_limits_retained_bytes() { let (mut writer, reader) = tokio::io::duplex(1024); @@ -1088,6 +1137,7 @@ mod tests { expiration: 500.into(), env, network: None, + network_attempt_id: None, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, justification: None, @@ -1141,6 +1191,7 @@ mod tests { expiration: ExecExpiration::Cancellation(cancel_token), env, network: None, + network_attempt_id: None, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level: codex_protocol::config_types::WindowsSandboxLevel::Disabled, justification: None, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 8b396517e..91bbe042e 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -41,6 +41,7 @@ pub mod landlock; pub mod mcp; mod mcp_connection_manager; pub mod models_manager; +mod network_policy_decision; pub mod network_proxy_loader; pub use mcp_connection_manager::MCP_SANDBOX_STATE_CAPABILITY; pub use mcp_connection_manager::MCP_SANDBOX_STATE_METHOD; diff --git a/codex-rs/core/src/network_policy_decision.rs b/codex-rs/core/src/network_policy_decision.rs new file mode 100644 index 000000000..3da6d34d6 --- /dev/null +++ b/codex-rs/core/src/network_policy_decision.rs @@ -0,0 +1,254 @@ +use codex_network_proxy::BlockedRequest; +use codex_network_proxy::NetworkDecisionSource; +use codex_network_proxy::NetworkPolicyDecision; +use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol; +use serde::Deserialize; + +#[derive(Debug, Clone, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct NetworkPolicyDecisionPayload { + pub decision: NetworkPolicyDecision, + pub source: NetworkDecisionSource, + #[serde(default)] + pub protocol: Option, + pub host: Option, + pub reason: Option, + pub port: Option, +} + +impl NetworkPolicyDecisionPayload { + pub(crate) fn is_ask_from_decider(&self) -> bool { + self.decision == NetworkPolicyDecision::Ask && self.source == NetworkDecisionSource::Decider + } +} + +fn parse_network_policy_decision(value: &str) -> Option { + match value { + "deny" => Some(NetworkPolicyDecision::Deny), + "ask" => Some(NetworkPolicyDecision::Ask), + _ => None, + } +} + +pub(crate) fn network_approval_context_from_payload( + payload: &NetworkPolicyDecisionPayload, +) -> Option { + if !payload.is_ask_from_decider() { + return None; + } + + let protocol = payload.protocol?; + + let host = payload.host.as_deref()?.trim(); + if host.is_empty() { + return None; + } + + Some(NetworkApprovalContext { + host: host.to_string(), + protocol, + }) +} + +pub(crate) fn denied_network_policy_message(blocked: &BlockedRequest) -> Option { + let decision = blocked + .decision + .as_deref() + .and_then(parse_network_policy_decision); + if decision != Some(NetworkPolicyDecision::Deny) { + return None; + } + + let host = blocked.host.trim(); + if host.is_empty() { + return Some("Network access was blocked by policy.".to_string()); + } + + let detail = match blocked.reason.as_str() { + "denied" => "domain is explicitly denied by policy and cannot be approved from this prompt", + "not_allowed" => "domain is not on the allowlist for the current sandbox mode", + "not_allowed_local" => "local/private network addresses are blocked by policy", + "method_not_allowed" => "request method is blocked by the current network mode", + "proxy_disabled" => "managed network proxy is disabled", + _ => "request is blocked by network policy", + }; + + Some(format!( + "Network access to \"{host}\" was blocked: {detail}." + )) +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_network_proxy::BlockedRequest; + use pretty_assertions::assert_eq; + + #[test] + fn network_approval_context_requires_ask_from_decider() { + let payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Deny, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Https), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + + assert_eq!(network_approval_context_from_payload(&payload), None); + } + + #[test] + fn network_approval_context_maps_http_https_and_socks_protocols() { + let http_payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Ask, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Http), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(80), + }; + assert_eq!( + network_approval_context_from_payload(&http_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Http, + }) + ); + + let https_payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Ask, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Https), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + assert_eq!( + network_approval_context_from_payload(&https_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }) + ); + + let http_connect_payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Ask, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Https), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + assert_eq!( + network_approval_context_from_payload(&http_connect_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }) + ); + + let socks5_tcp_payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Ask, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Socks5Tcp), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + assert_eq!( + network_approval_context_from_payload(&socks5_tcp_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Socks5Tcp, + }) + ); + + let socks5_udp_payload = NetworkPolicyDecisionPayload { + decision: NetworkPolicyDecision::Ask, + source: NetworkDecisionSource::Decider, + protocol: Some(NetworkApprovalProtocol::Socks5Udp), + host: Some("example.com".to_string()), + reason: Some("not_allowed".to_string()), + port: Some(443), + }; + assert_eq!( + network_approval_context_from_payload(&socks5_udp_payload), + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Socks5Udp, + }) + ); + } + + #[test] + fn network_policy_decision_payload_deserializes_proxy_protocol_aliases() { + let payload: NetworkPolicyDecisionPayload = serde_json::from_str( + r#"{ + "decision":"ask", + "source":"decider", + "protocol":"https_connect", + "host":"example.com", + "reason":"not_allowed", + "port":443 + }"#, + ) + .expect("payload should deserialize"); + assert_eq!(payload.protocol, Some(NetworkApprovalProtocol::Https)); + + let payload: NetworkPolicyDecisionPayload = serde_json::from_str( + r#"{ + "decision":"ask", + "source":"decider", + "protocol":"http-connect", + "host":"example.com", + "reason":"not_allowed", + "port":443 + }"#, + ) + .expect("payload should deserialize"); + assert_eq!(payload.protocol, Some(NetworkApprovalProtocol::Https)); + } + + #[test] + fn denied_network_policy_message_requires_deny_decision() { + let blocked = BlockedRequest { + host: "example.com".to_string(), + reason: "not_allowed".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + attempt_id: Some("attempt-1".to_string()), + decision: Some("ask".to_string()), + source: Some("decider".to_string()), + port: Some(80), + timestamp: 0, + }; + assert_eq!(denied_network_policy_message(&blocked), None); + } + + #[test] + fn denied_network_policy_message_for_denylist_block_is_explicit() { + let blocked = BlockedRequest { + host: "example.com".to_string(), + reason: "denied".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + attempt_id: Some("attempt-1".to_string()), + decision: Some("deny".to_string()), + source: Some("baseline_policy".to_string()), + port: Some(80), + timestamp: 0, + }; + assert_eq!( + denied_network_policy_message(&blocked), + Some( + "Network access to \"example.com\" was blocked: domain is explicitly denied by policy and cannot be approved from this prompt.".to_string() + ) + ); + } +} diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 9a72b6e24..487278b61 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -46,6 +46,7 @@ pub struct ExecRequest { pub cwd: PathBuf, pub env: HashMap, pub network: Option, + pub network_attempt_id: Option, pub expiration: ExecExpiration, pub sandbox: SandboxType, pub windows_sandbox_level: WindowsSandboxLevel, @@ -214,6 +215,7 @@ impl SandboxManager { cwd: spec.cwd, env, network: network.cloned(), + network_attempt_id: None, expiration: spec.expiration, sandbox, windows_sandbox_level, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 3aec0e535..6f640131f 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -12,6 +12,7 @@ use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::skills::SkillsManager; use crate::state_db::StateDbHandle; +use crate::tools::network_approval::NetworkApprovalService; use crate::tools::sandboxing::ApprovalStore; use crate::unified_exec::UnifiedExecProcessManager; use codex_hooks::Hooks; @@ -40,6 +41,7 @@ pub(crate) struct SessionServices { pub(crate) file_watcher: Arc, pub(crate) agent_control: AgentControl, pub(crate) network_proxy: Option, + pub(crate) network_approval: Arc, pub(crate) state_db: Option, /// Session-scoped model client shared across turns. pub(crate) model_client: ModelClient, diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 43950a8da..35088efce 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -150,6 +150,7 @@ pub(crate) async fn execute_user_shell_command( Some(session.conversation_id), ), network: turn_context.network.clone(), + network_attempt_id: None, // TODO(zhao-oai): Now that we have ExecExpiration::Cancellation, we // should use that instead of an "arbitrarily large" timeout here. expiration: USER_SHELL_TIMEOUT_MS.into(), diff --git a/codex-rs/core/src/tools/events.rs b/codex-rs/core/src/tools/events.rs index 65972f5a5..2bacd188d 100644 --- a/codex-rs/core/src/tools/events.rs +++ b/codex-rs/core/src/tools/events.rs @@ -314,7 +314,7 @@ impl ToolEmitter { (event, result) } Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Timeout { output }))) - | Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => { + | Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, .. }))) => { let response = self.format_exec_output_for_model(&output, ctx); let event = ToolEventStage::Failure(ToolEventFailure::Output(*output)); let result = Err(FunctionCallError::RespondToModel(response)); diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index aced569ce..49c679e52 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -143,10 +143,12 @@ impl ToolHandler for ApplyPatchHandler { turn: turn.as_ref(), call_id: call_id.clone(), tool_name: tool_name.to_string(), + network_attempt_id: None, }; let out = orchestrator .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) - .await; + .await + .map(|result| result.output); let event_ctx = ToolEventCtx::new( session.as_ref(), turn.as_ref(), @@ -232,10 +234,12 @@ pub(crate) async fn intercept_apply_patch( turn, call_id: call_id.to_string(), tool_name: tool_name.to_string(), + network_attempt_id: None, }; let out = orchestrator .run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy) - .await; + .await + .map(|result| result.output); let event_ctx = ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied()); let content = emitter.finish(event_ctx, out).await?; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b9e2a97d6..67f4c0416 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -54,6 +54,7 @@ impl ShellHandler { expiration: params.timeout_ms.into(), env: create_env(&turn_context.shell_environment_policy, Some(thread_id)), network: turn_context.network.clone(), + network_attempt_id: None, sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, justification: params.justification.clone(), @@ -83,6 +84,7 @@ impl ShellCommandHandler { expiration: params.timeout_ms.into(), env: create_env(&turn_context.shell_environment_policy, Some(thread_id)), network: turn_context.network.clone(), + network_attempt_id: None, sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), windows_sandbox_level: turn_context.windows_sandbox_level, justification: params.justification.clone(), @@ -325,10 +327,12 @@ impl ShellHandler { turn: turn.as_ref(), call_id: call_id.clone(), tool_name, + network_attempt_id: None, }; let out = orchestrator .run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy) - .await; + .await + .map(|result| result.output); let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None); let content = emitter.finish(event_ctx, out).await?; Ok(ToolOutput::Function { diff --git a/codex-rs/core/src/tools/mod.rs b/codex-rs/core/src/tools/mod.rs index 3307d6531..27fe51512 100644 --- a/codex-rs/core/src/tools/mod.rs +++ b/codex-rs/core/src/tools/mod.rs @@ -2,6 +2,7 @@ pub mod context; pub mod events; pub(crate) mod handlers; pub mod js_repl; +pub(crate) mod network_approval; pub mod orchestrator; pub mod parallel; pub mod registry; diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs new file mode 100644 index 000000000..51e6e3c98 --- /dev/null +++ b/codex-rs/core/src/tools/network_approval.rs @@ -0,0 +1,675 @@ +use crate::codex::Session; +use crate::network_policy_decision::denied_network_policy_message; +use crate::tools::sandboxing::ToolError; +use codex_network_proxy::BlockedRequest; +use codex_network_proxy::BlockedRequestObserver; +use codex_network_proxy::NetworkDecision; +use codex_network_proxy::NetworkPolicyDecider; +use codex_network_proxy::NetworkPolicyRequest; +use codex_network_proxy::NetworkProtocol; +use codex_network_proxy::NetworkProxy; +use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol; +use codex_protocol::protocol::ReviewDecision; +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::PathBuf; +use std::sync::Arc; +use tokio::sync::Mutex; +use tokio::sync::RwLock; +use uuid::Uuid; + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(crate) enum NetworkApprovalMode { + Immediate, + Deferred, +} + +#[derive(Clone, Debug)] +pub(crate) struct NetworkApprovalSpec { + pub command: Vec, + pub cwd: PathBuf, + pub network: Option, + pub mode: NetworkApprovalMode, +} + +#[derive(Clone, Debug)] +pub(crate) struct DeferredNetworkApproval { + attempt_id: String, +} + +impl DeferredNetworkApproval { + pub(crate) fn attempt_id(&self) -> &str { + &self.attempt_id + } +} + +#[derive(Debug)] +pub(crate) struct ActiveNetworkApproval { + attempt_id: Option, + mode: NetworkApprovalMode, +} + +impl ActiveNetworkApproval { + pub(crate) fn attempt_id(&self) -> Option<&str> { + self.attempt_id.as_deref() + } + + pub(crate) fn mode(&self) -> NetworkApprovalMode { + self.mode + } + + pub(crate) fn into_deferred(self) -> Option { + match (self.mode, self.attempt_id) { + (NetworkApprovalMode::Deferred, Some(attempt_id)) => { + Some(DeferredNetworkApproval { attempt_id }) + } + _ => None, + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) enum NetworkApprovalOutcome { + DeniedByUser, + DeniedByPolicy(String), +} + +struct NetworkApprovalAttempt { + turn_id: String, + call_id: String, + command: Vec, + cwd: PathBuf, + approved_hosts: Mutex>, + outcome: Mutex>, +} + +pub(crate) struct NetworkApprovalService { + attempts: Mutex>>, + session_approved_hosts: Mutex>, +} + +impl Default for NetworkApprovalService { + fn default() -> Self { + Self { + attempts: Mutex::new(HashMap::new()), + session_approved_hosts: Mutex::new(HashSet::new()), + } + } +} + +impl NetworkApprovalService { + pub(crate) async fn register_attempt( + &self, + attempt_id: String, + turn_id: String, + call_id: String, + command: Vec, + cwd: PathBuf, + ) { + let mut attempts = self.attempts.lock().await; + attempts.insert( + attempt_id, + Arc::new(NetworkApprovalAttempt { + turn_id, + call_id, + command, + cwd, + approved_hosts: Mutex::new(HashSet::new()), + outcome: Mutex::new(None), + }), + ); + } + + pub(crate) async fn unregister_attempt(&self, attempt_id: &str) { + let mut attempts = self.attempts.lock().await; + attempts.remove(attempt_id); + } + + pub(crate) async fn take_outcome(&self, attempt_id: &str) -> Option { + let attempt = { + let attempts = self.attempts.lock().await; + attempts.get(attempt_id).cloned() + }?; + let mut outcome = attempt.outcome.lock().await; + outcome.take() + } + + pub(crate) async fn take_user_denial_outcome(&self, attempt_id: &str) -> bool { + let attempt = { + let attempts = self.attempts.lock().await; + attempts.get(attempt_id).cloned() + }; + let Some(attempt) = attempt else { + return false; + }; + let mut outcome = attempt.outcome.lock().await; + if matches!(outcome.as_ref(), Some(NetworkApprovalOutcome::DeniedByUser)) { + outcome.take(); + return true; + } + false + } + + async fn resolve_attempt_for_request( + &self, + request: &NetworkPolicyRequest, + ) -> Option> { + let attempts = self.attempts.lock().await; + + if let Some(attempt_id) = request.attempt_id.as_deref() { + if let Some(attempt) = attempts.get(attempt_id).cloned() { + return Some(attempt); + } + return None; + } + + if attempts.len() == 1 { + return attempts.values().next().cloned(); + } + + None + } + + async fn resolve_attempt_for_blocked_request( + &self, + blocked: &BlockedRequest, + ) -> Option> { + let attempts = self.attempts.lock().await; + + if let Some(attempt_id) = blocked.attempt_id.as_deref() { + if let Some(attempt) = attempts.get(attempt_id).cloned() { + return Some(attempt); + } + return None; + } + + if attempts.len() == 1 { + return attempts.values().next().cloned(); + } + + None + } + + pub(crate) async fn record_blocked_request(&self, blocked: BlockedRequest) { + let Some(message) = denied_network_policy_message(&blocked) else { + return; + }; + + let Some(attempt) = self.resolve_attempt_for_blocked_request(&blocked).await else { + return; + }; + + let mut outcome = attempt.outcome.lock().await; + if matches!(outcome.as_ref(), Some(NetworkApprovalOutcome::DeniedByUser)) { + return; + } + *outcome = Some(NetworkApprovalOutcome::DeniedByPolicy(message)); + } + + pub(crate) async fn handle_inline_policy_request( + &self, + session: &Session, + request: NetworkPolicyRequest, + ) -> NetworkDecision { + const REASON_NOT_ALLOWED: &str = "not_allowed"; + + { + let approved_hosts = self.session_approved_hosts.lock().await; + if approved_hosts.contains(request.host.as_str()) { + return NetworkDecision::Allow; + } + } + + let Some(attempt) = self.resolve_attempt_for_request(&request).await else { + return NetworkDecision::deny(REASON_NOT_ALLOWED); + }; + + { + let approved_hosts = attempt.approved_hosts.lock().await; + if approved_hosts.contains(request.host.as_str()) { + return NetworkDecision::Allow; + } + } + + let protocol = match request.protocol { + NetworkProtocol::Http => NetworkApprovalProtocol::Http, + NetworkProtocol::HttpsConnect => NetworkApprovalProtocol::Https, + NetworkProtocol::Socks5Tcp => NetworkApprovalProtocol::Socks5Tcp, + NetworkProtocol::Socks5Udp => NetworkApprovalProtocol::Socks5Udp, + }; + + let Some(turn_context) = session.turn_context_for_sub_id(&attempt.turn_id).await else { + return NetworkDecision::deny(REASON_NOT_ALLOWED); + }; + + let approval_decision = session + .request_command_approval( + turn_context.as_ref(), + attempt.call_id.clone(), + attempt.command.clone(), + attempt.cwd.clone(), + Some(format!( + "Network access to \"{}\" is blocked by policy.", + request.host + )), + Some(NetworkApprovalContext { + host: request.host.clone(), + protocol, + }), + None, + ) + .await; + + match approval_decision { + ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { + let mut approved_hosts = attempt.approved_hosts.lock().await; + approved_hosts.insert(request.host); + NetworkDecision::Allow + } + ReviewDecision::ApprovedForSession => { + let mut approved_hosts = self.session_approved_hosts.lock().await; + approved_hosts.insert(request.host); + NetworkDecision::Allow + } + ReviewDecision::Denied | ReviewDecision::Abort => { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByUser); + NetworkDecision::deny(REASON_NOT_ALLOWED) + } + } + } +} + +pub(crate) fn build_blocked_request_observer( + network_approval: Arc, +) -> Arc { + Arc::new(move |blocked: BlockedRequest| { + let network_approval = Arc::clone(&network_approval); + async move { + network_approval.record_blocked_request(blocked).await; + } + }) +} + +pub(crate) fn build_network_policy_decider( + network_approval: Arc, + network_policy_decider_session: Arc>>, +) -> Arc { + Arc::new(move |request: NetworkPolicyRequest| { + let network_approval = Arc::clone(&network_approval); + let network_policy_decider_session = Arc::clone(&network_policy_decider_session); + async move { + let Some(session) = network_policy_decider_session.read().await.upgrade() else { + return NetworkDecision::ask("not_allowed"); + }; + network_approval + .handle_inline_policy_request(session.as_ref(), request) + .await + } + }) +} + +pub(crate) async fn begin_network_approval( + session: &Session, + turn_id: &str, + call_id: &str, + has_managed_network_requirements: bool, + spec: Option, +) -> Option { + let spec = spec?; + if !has_managed_network_requirements || spec.network.is_none() { + return None; + } + + let attempt_id = Uuid::new_v4().to_string(); + session + .services + .network_approval + .register_attempt( + attempt_id.clone(), + turn_id.to_string(), + call_id.to_string(), + spec.command, + spec.cwd, + ) + .await; + + Some(ActiveNetworkApproval { + attempt_id: Some(attempt_id), + mode: spec.mode, + }) +} + +pub(crate) async fn finish_immediate_network_approval( + session: &Session, + active: ActiveNetworkApproval, +) -> Result<(), ToolError> { + let Some(attempt_id) = active.attempt_id.as_deref() else { + return Ok(()); + }; + + let approval_outcome = session + .services + .network_approval + .take_outcome(attempt_id) + .await; + + session + .services + .network_approval + .unregister_attempt(attempt_id) + .await; + + match approval_outcome { + Some(NetworkApprovalOutcome::DeniedByUser) => { + Err(ToolError::Rejected("rejected by user".to_string())) + } + Some(NetworkApprovalOutcome::DeniedByPolicy(message)) => Err(ToolError::Rejected(message)), + None => Ok(()), + } +} + +pub(crate) async fn deferred_rejection_message( + session: &Session, + deferred: &DeferredNetworkApproval, +) -> Option { + match session + .services + .network_approval + .take_outcome(deferred.attempt_id()) + .await + { + Some(NetworkApprovalOutcome::DeniedByUser) => Some("rejected by user".to_string()), + Some(NetworkApprovalOutcome::DeniedByPolicy(message)) => Some(message), + None => None, + } +} + +pub(crate) async fn finish_deferred_network_approval( + session: &Session, + deferred: Option, +) { + let Some(deferred) = deferred else { + return; + }; + session + .services + .network_approval + .unregister_attempt(deferred.attempt_id()) + .await; +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_network_proxy::BlockedRequestArgs; + use codex_network_proxy::NetworkPolicyRequestArgs; + use pretty_assertions::assert_eq; + + fn http_request(host: &str, attempt_id: Option<&str>) -> NetworkPolicyRequest { + NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Http, + host: host.to_string(), + port: 80, + client_addr: None, + method: Some("GET".to_string()), + command: None, + exec_policy_hint: None, + attempt_id: attempt_id.map(ToString::to_string), + }) + } + + #[tokio::test] + async fn resolve_attempt_for_request_falls_back_to_single_active_attempt() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let resolved = service + .resolve_attempt_for_request(&http_request("example.com", None)) + .await + .expect("single active attempt should be used as fallback"); + assert_eq!(resolved.call_id, "call-1"); + } + + #[tokio::test] + async fn resolve_attempt_for_request_returns_exact_attempt_match() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + service + .register_attempt( + "attempt-2".to_string(), + "turn-2".to_string(), + "call-2".to_string(), + vec!["curl".to_string(), "openai.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let resolved = service + .resolve_attempt_for_request(&http_request("openai.com", Some("attempt-2"))) + .await + .expect("attempt-2 should resolve"); + assert_eq!(resolved.call_id, "call-2"); + } + + #[tokio::test] + async fn resolve_attempt_for_request_returns_none_for_unknown_attempt_id() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let resolved = service + .resolve_attempt_for_request(&http_request("example.com", Some("attempt-unknown"))) + .await; + assert!(resolved.is_none()); + } + + #[tokio::test] + async fn resolve_attempt_for_request_returns_none_when_ambiguous() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + service + .register_attempt( + "attempt-2".to_string(), + "turn-2".to_string(), + "call-2".to_string(), + vec!["curl".to_string(), "robinhood.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let resolved = service + .resolve_attempt_for_request(&http_request("example.com", None)) + .await; + assert!(resolved.is_none()); + } + + #[tokio::test] + async fn take_outcome_clears_stored_value() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let attempt = { + let attempts = service.attempts.lock().await; + attempts + .get("attempt-1") + .cloned() + .expect("attempt should exist") + }; + { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByUser); + } + + assert_eq!( + service.take_outcome("attempt-1").await, + Some(NetworkApprovalOutcome::DeniedByUser) + ); + assert_eq!(service.take_outcome("attempt-1").await, None); + } + + #[tokio::test] + async fn take_user_denial_outcome_preserves_policy_denial() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let attempt = { + let attempts = service.attempts.lock().await; + attempts + .get("attempt-1") + .cloned() + .expect("attempt should exist") + }; + { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByPolicy( + "policy denied".to_string(), + )); + } + + assert!(!service.take_user_denial_outcome("attempt-1").await); + assert_eq!( + service.take_outcome("attempt-1").await, + Some(NetworkApprovalOutcome::DeniedByPolicy( + "policy denied".to_string(), + )) + ); + } + + #[tokio::test] + async fn record_blocked_request_stores_policy_denial_outcome() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + service + .record_blocked_request(BlockedRequest::new(BlockedRequestArgs { + host: "example.com".to_string(), + reason: "denied".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + attempt_id: Some("attempt-1".to_string()), + decision: Some("deny".to_string()), + source: Some("baseline_policy".to_string()), + port: Some(80), + })) + .await; + + let outcome = service + .take_outcome("attempt-1") + .await + .expect("outcome should be recorded"); + match outcome { + NetworkApprovalOutcome::DeniedByPolicy(message) => { + assert_eq!( + message, + "Network access to \"example.com\" was blocked: domain is explicitly denied by policy and cannot be approved from this prompt.".to_string() + ); + } + NetworkApprovalOutcome::DeniedByUser => panic!("expected policy denial"), + } + } + + #[tokio::test] + async fn record_blocked_request_does_not_override_user_denial() { + let service = NetworkApprovalService::default(); + service + .register_attempt( + "attempt-1".to_string(), + "turn-1".to_string(), + "call-1".to_string(), + vec!["curl".to_string(), "example.com".to_string()], + std::env::temp_dir(), + ) + .await; + + let attempt = { + let attempts = service.attempts.lock().await; + attempts + .get("attempt-1") + .cloned() + .expect("attempt should exist") + }; + { + let mut outcome = attempt.outcome.lock().await; + *outcome = Some(NetworkApprovalOutcome::DeniedByUser); + } + + service + .record_blocked_request(BlockedRequest::new(BlockedRequestArgs { + host: "example.com".to_string(), + reason: "denied".to_string(), + client: None, + method: Some("GET".to_string()), + mode: None, + protocol: "http".to_string(), + attempt_id: Some("attempt-1".to_string()), + decision: Some("deny".to_string()), + source: Some("baseline_policy".to_string()), + port: Some(80), + })) + .await; + + assert_eq!( + service.take_outcome("attempt-1").await, + Some(NetworkApprovalOutcome::DeniedByUser) + ); + } +} diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 3f410e1b2..cdbaac025 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -10,7 +10,13 @@ use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecToolCallOutput; use crate::features::Feature; +use crate::network_policy_decision::network_approval_context_from_payload; use crate::sandboxing::SandboxManager; +use crate::tools::network_approval::DeferredNetworkApproval; +use crate::tools::network_approval::NetworkApprovalMode; +use crate::tools::network_approval::begin_network_approval; +use crate::tools::network_approval::finish_deferred_network_approval; +use crate::tools::network_approval::finish_immediate_network_approval; use crate::tools::sandboxing::ApprovalCtx; use crate::tools::sandboxing::ExecApprovalRequirement; use crate::tools::sandboxing::SandboxAttempt; @@ -27,6 +33,11 @@ pub(crate) struct ToolOrchestrator { sandbox: SandboxManager, } +pub(crate) struct OrchestratorRunResult { + pub output: Out, + pub deferred_network_approval: Option, +} + impl ToolOrchestrator { pub fn new() -> Self { Self { @@ -34,6 +45,60 @@ impl ToolOrchestrator { } } + async fn run_attempt( + tool: &mut T, + req: &Rq, + tool_ctx: &ToolCtx<'_>, + attempt: &SandboxAttempt<'_>, + has_managed_network_requirements: bool, + ) -> (Result, Option) + where + T: ToolRuntime, + { + let network_approval = begin_network_approval( + tool_ctx.session, + &tool_ctx.turn.sub_id, + &tool_ctx.call_id, + has_managed_network_requirements, + tool.network_approval_spec(req, tool_ctx), + ) + .await; + + let attempt_tool_ctx = ToolCtx { + session: tool_ctx.session, + turn: tool_ctx.turn, + call_id: tool_ctx.call_id.clone(), + tool_name: tool_ctx.tool_name.clone(), + network_attempt_id: network_approval.as_ref().and_then(|network_approval| { + network_approval.attempt_id().map(ToString::to_string) + }), + }; + let run_result = tool.run(req, attempt, &attempt_tool_ctx).await; + + let Some(network_approval) = network_approval else { + return (run_result, None); + }; + + match network_approval.mode() { + NetworkApprovalMode::Immediate => { + let finalize_result = + finish_immediate_network_approval(tool_ctx.session, network_approval).await; + if let Err(err) = finalize_result { + return (Err(err), None); + } + (run_result, None) + } + NetworkApprovalMode::Deferred => { + let deferred = network_approval.into_deferred(); + if run_result.is_err() { + finish_deferred_network_approval(tool_ctx.session, deferred).await; + return (run_result, None); + } + (run_result, deferred) + } + } + } + pub async fn run( &mut self, tool: &mut T, @@ -41,7 +106,7 @@ impl ToolOrchestrator { tool_ctx: &ToolCtx<'_>, turn_ctx: &crate::codex::TurnContext, approval_policy: AskForApproval, - ) -> Result + ) -> Result, ToolError> where T: ToolRuntime, { @@ -70,6 +135,7 @@ impl ToolOrchestrator { turn: turn_ctx, call_id: &tool_ctx.call_id, retry_reason: reason, + network_approval_context: None, }; let decision = tool.start_approval_async(req, approval_ctx).await; @@ -118,33 +184,86 @@ impl ToolOrchestrator { windows_sandbox_level: turn_ctx.windows_sandbox_level, }; - match tool.run(req, &initial_attempt, tool_ctx).await { + let (first_result, first_deferred_network_approval) = Self::run_attempt( + tool, + req, + tool_ctx, + &initial_attempt, + has_managed_network_requirements, + ) + .await; + match first_result { Ok(out) => { // We have a successful initial result - Ok(out) + Ok(OrchestratorRunResult { + output: out, + deferred_network_approval: first_deferred_network_approval, + }) } - Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output }))) => { + Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }))) => { + let network_approval_context = if has_managed_network_requirements { + network_policy_decision + .as_ref() + .and_then(network_approval_context_from_payload) + } else { + None + }; + if network_policy_decision.is_some() && network_approval_context.is_none() { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }))); + } if !tool.escalate_on_failure() { return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output, + network_policy_decision, }))); } // Under `Never` or `OnRequest`, do not retry without sandbox; surface a concise // sandbox denial that preserves the original output. if !tool.wants_no_sandbox_approval(approval_policy) { - return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { - output, - }))); + let allow_on_request_network_prompt = + matches!(approval_policy, AskForApproval::OnRequest) + && network_approval_context.is_some() + && matches!( + default_exec_approval_requirement( + approval_policy, + &turn_ctx.sandbox_policy + ), + ExecApprovalRequirement::NeedsApproval { .. } + ); + if !allow_on_request_network_prompt { + return Err(ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { + output, + network_policy_decision, + }))); + } } + let retry_reason = + if let Some(network_approval_context) = network_approval_context.as_ref() { + format!( + "Network access to \"{}\" is blocked by policy.", + network_approval_context.host + ) + } else { + build_denial_reason_from_output(output.as_ref()) + }; // Ask for approval before retrying with the escalated sandbox. - if !tool.should_bypass_approval(approval_policy, already_approved) { - let reason_msg = build_denial_reason_from_output(output.as_ref()); + let bypass_retry_approval = tool + .should_bypass_approval(approval_policy, already_approved) + && network_approval_context.is_none(); + if !bypass_retry_approval { let approval_ctx = ApprovalCtx { session: tool_ctx.session, turn: turn_ctx, call_id: &tool_ctx.call_id, - retry_reason: Some(reason_msg), + retry_reason: Some(retry_reason), + network_approval_context: network_approval_context.clone(), }; let decision = tool.start_approval_async(req, approval_ctx).await; @@ -172,9 +291,20 @@ impl ToolOrchestrator { }; // Second attempt. - (*tool).run(req, &escalated_attempt, tool_ctx).await + let (retry_result, retry_deferred_network_approval) = Self::run_attempt( + tool, + req, + tool_ctx, + &escalated_attempt, + has_managed_network_requirements, + ) + .await; + retry_result.map(|output| OrchestratorRunResult { + output, + deferred_network_approval: retry_deferred_network_approval, + }) } - other => other, + Err(err) => Err(err), } } } diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index fd076f005..3b7f970d7 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -11,6 +11,8 @@ use crate::powershell::prefix_powershell_script_with_utf8; use crate::sandboxing::SandboxPermissions; use crate::sandboxing::execute_env; use crate::shell::ShellType; +use crate::tools::network_approval::NetworkApprovalMode; +use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; use crate::tools::sandboxing::Approvable; @@ -109,6 +111,7 @@ impl Approvable for ShellRuntime { command, cwd, reason, + ctx.network_approval_context.clone(), req.exec_approval_requirement .proposed_execpolicy_amendment() .cloned(), @@ -141,6 +144,20 @@ impl Approvable for ShellRuntime { } impl ToolRuntime for ShellRuntime { + fn network_approval_spec( + &self, + req: &ShellRequest, + _ctx: &ToolCtx<'_>, + ) -> Option { + req.network.as_ref()?; + Some(NetworkApprovalSpec { + command: req.command.clone(), + cwd: req.cwd.clone(), + network: req.network.clone(), + mode: NetworkApprovalMode::Immediate, + }) + } + async fn run( &mut self, req: &ShellRequest, @@ -167,9 +184,10 @@ impl ToolRuntime for ShellRuntime { req.sandbox_permissions, req.justification.clone(), )?; - let env = attempt + let mut env = attempt .env_for(spec, req.network.as_ref()) .map_err(|err| ToolError::Codex(err.into()))?; + env.network_attempt_id = ctx.network_attempt_id.clone(); let out = execute_env(env, attempt.policy, Self::stdout_stream(ctx)) .await .map_err(ToolError::Codex)?; diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index f3bd2e64b..4c6c61107 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -12,6 +12,8 @@ use crate::features::Feature; use crate::powershell::prefix_powershell_script_with_utf8; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; +use crate::tools::network_approval::NetworkApprovalMode; +use crate::tools::network_approval::NetworkApprovalSpec; use crate::tools::runtimes::build_command_spec; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; use crate::tools::sandboxing::Approvable; @@ -110,6 +112,7 @@ impl Approvable for UnifiedExecRuntime<'_> { command, cwd, reason, + ctx.network_approval_context.clone(), req.exec_approval_requirement .proposed_execpolicy_amendment() .cloned(), @@ -145,6 +148,20 @@ impl Approvable for UnifiedExecRuntime<'_> { } impl<'a> ToolRuntime for UnifiedExecRuntime<'a> { + fn network_approval_spec( + &self, + req: &UnifiedExecRequest, + _ctx: &ToolCtx<'_>, + ) -> Option { + req.network.as_ref()?; + Some(NetworkApprovalSpec { + command: req.command.clone(), + cwd: req.cwd.clone(), + network: req.network.clone(), + mode: NetworkApprovalMode::Deferred, + }) + } + async fn run( &mut self, req: &UnifiedExecRequest, @@ -165,7 +182,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt let mut env = req.env.clone(); if let Some(network) = req.network.as_ref() { - network.apply_to_env(&mut env); + network.apply_to_env_for_attempt(&mut env, ctx.network_attempt_id.as_deref()); } let spec = build_command_spec( &command, @@ -186,6 +203,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt UnifiedExecError::SandboxDenied { output, .. } => { ToolError::Codex(CodexErr::Sandbox(SandboxErr::Denied { output: Box::new(output), + network_policy_decision: None, })) } other => ToolError::Rejected(other.to_string()), diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 367176084..04dc10c3e 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -12,8 +12,10 @@ use crate::sandboxing::CommandSpec; use crate::sandboxing::SandboxManager; use crate::sandboxing::SandboxTransformError; use crate::state::SessionServices; +use crate::tools::network_approval::NetworkApprovalSpec; use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::ExecPolicyAmendment; +use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; use std::collections::HashMap; @@ -110,6 +112,7 @@ pub(crate) struct ApprovalCtx<'a> { pub turn: &'a TurnContext, pub call_id: &'a str, pub retry_reason: Option, + pub network_approval_context: Option, } // Specifies what tool orchestrator should do with a given tool call. @@ -252,6 +255,7 @@ pub(crate) struct ToolCtx<'a> { pub turn: &'a TurnContext, pub call_id: String, pub tool_name: String, + pub network_attempt_id: Option, } #[derive(Debug)] @@ -261,6 +265,10 @@ pub(crate) enum ToolError { } pub(crate) trait ToolRuntime: Approvable + Sandboxable { + fn network_approval_spec(&self, _req: &Req, _ctx: &ToolCtx<'_>) -> Option { + None + } + async fn run( &mut self, req: &Req, diff --git a/codex-rs/core/src/unified_exec/async_watcher.rs b/codex-rs/core/src/unified_exec/async_watcher.rs index 1fbb1e8f6..772baab00 100644 --- a/codex-rs/core/src/unified_exec/async_watcher.rs +++ b/codex-rs/core/src/unified_exec/async_watcher.rs @@ -139,6 +139,43 @@ pub(crate) fn spawn_exit_watcher( }); } +pub(crate) fn spawn_network_denial_watcher( + process: Arc, + session: Arc, + process_id: String, + network_attempt_id: String, +) { + let exit_token = process.cancellation_token(); + tokio::spawn(async move { + let mut poll = tokio::time::interval(Duration::from_millis(100)); + poll.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + + loop { + tokio::select! { + _ = exit_token.cancelled() => { + break; + } + _ = poll.tick() => { + if session + .services + .network_approval + .take_user_denial_outcome(&network_attempt_id) + .await + { + process.terminate(); + session + .services + .unified_exec_manager + .release_process_id(&process_id) + .await; + break; + } + } + } + } + }); +} + async fn process_chunk( pending: &mut Vec, transcript: &Arc>, diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 84785270f..a907f348f 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -25,6 +25,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; use std::sync::Arc; +use std::sync::Weak; use std::time::Duration; use codex_network_proxy::NetworkProxy; @@ -144,6 +145,8 @@ struct ProcessEntry { process_id: String, command: Vec, tty: bool, + network_attempt_id: Option, + session: Weak, last_used: tokio::time::Instant, } diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 251a4910b..197cda479 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -19,6 +19,9 @@ use crate::sandboxing::ExecRequest; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; use crate::tools::events::ToolEventStage; +use crate::tools::network_approval::DeferredNetworkApproval; +use crate::tools::network_approval::deferred_rejection_message; +use crate::tools::network_approval::finish_deferred_network_approval; use crate::tools::orchestrator::ToolOrchestrator; use crate::tools::runtimes::unified_exec::UnifiedExecRequest as UnifiedExecToolRequest; use crate::tools::runtimes::unified_exec::UnifiedExecRuntime; @@ -40,6 +43,7 @@ use crate::unified_exec::WARNING_UNIFIED_EXEC_PROCESSES; use crate::unified_exec::WriteStdinRequest; use crate::unified_exec::async_watcher::emit_exec_end_for_unified_exec; use crate::unified_exec::async_watcher::spawn_exit_watcher; +use crate::unified_exec::async_watcher::spawn_network_denial_watcher; use crate::unified_exec::async_watcher::start_streaming_output; use crate::unified_exec::clamp_yield_time; use crate::unified_exec::generate_chunk_id; @@ -130,8 +134,25 @@ impl UnifiedExecProcessManager { } pub(crate) async fn release_process_id(&self, process_id: &str) { - let mut store = self.process_store.lock().await; - store.remove(process_id); + let removed = { + let mut store = self.process_store.lock().await; + store.remove(process_id) + }; + if let Some(entry) = removed { + Self::unregister_network_attempt_for_entry(&entry).await; + } + } + + async fn unregister_network_attempt_for_entry(entry: &ProcessEntry) { + if let Some(attempt_id) = entry.network_attempt_id.as_deref() + && let Some(session) = entry.session.upgrade() + { + session + .services + .network_approval + .unregister_attempt(attempt_id) + .await; + } } pub(crate) async fn exec_command( @@ -143,13 +164,14 @@ impl UnifiedExecProcessManager { .workdir .clone() .unwrap_or_else(|| context.turn.cwd.clone()); - let process = self .open_session_with_sandbox(&request, cwd.clone(), context) .await; - let process = match process { - Ok(process) => Arc::new(process), + let (process, mut deferred_network_approval) = match process { + Ok((process, deferred_network_approval)) => { + (Arc::new(process), deferred_network_approval) + } Err(err) => { self.release_process_id(&request.process_id).await; return Err(err); @@ -172,7 +194,6 @@ impl UnifiedExecProcessManager { emitter.emit(event_ctx, ToolEventStage::Begin).await; start_streaming_output(&process, context, Arc::clone(&transcript)); - let max_tokens = resolve_max_tokens(request.max_output_tokens); let yield_time_ms = clamp_yield_time(request.yield_time_ms); @@ -205,6 +226,7 @@ impl UnifiedExecProcessManager { let has_exited = process.has_exited() || exit_code.is_some(); let chunk_id = generate_chunk_id(); let process_id = request.process_id.clone(); + if has_exited { // Short‑lived command: emit ExecCommandEnd immediately using the // same helper as the background watcher, so all end events share @@ -215,7 +237,7 @@ impl UnifiedExecProcessManager { Arc::clone(&context.turn), context.call_id.clone(), request.command.clone(), - cwd, + cwd.clone(), Some(process_id), Arc::clone(&transcript), output.clone(), @@ -225,12 +247,45 @@ impl UnifiedExecProcessManager { .await; self.release_process_id(&request.process_id).await; + if let Some(deferred) = deferred_network_approval.as_ref() + && let Some(message) = + deferred_rejection_message(context.session.as_ref(), deferred).await + { + finish_deferred_network_approval( + context.session.as_ref(), + deferred_network_approval.take(), + ) + .await; + return Err(UnifiedExecError::create_process(message)); + } + finish_deferred_network_approval( + context.session.as_ref(), + deferred_network_approval.take(), + ) + .await; process.check_for_sandbox_denial_with_text(&text).await?; } else { + if let Some(deferred) = deferred_network_approval.as_ref() + && let Some(message) = + deferred_rejection_message(context.session.as_ref(), deferred).await + { + process.terminate(); + finish_deferred_network_approval( + context.session.as_ref(), + deferred_network_approval.take(), + ) + .await; + self.release_process_id(&request.process_id).await; + return Err(UnifiedExecError::create_process(message)); + } + // Long‑lived command: persist the process so write_stdin can reuse // it, and register a background watcher that will emit // ExecCommandEnd when the PTY eventually exits (even if no further // tool calls are made). + let network_attempt_id = deferred_network_approval + .as_ref() + .map(|deferred| deferred.attempt_id().to_string()); self.store_process( Arc::clone(&process), context, @@ -239,6 +294,7 @@ impl UnifiedExecProcessManager { start, process_id, request.tty, + network_attempt_id, Arc::clone(&transcript), ) .await; @@ -358,29 +414,35 @@ impl UnifiedExecProcessManager { } async fn refresh_process_state(&self, process_id: &str) -> ProcessStatus { - let mut store = self.process_store.lock().await; - let Some(entry) = store.processes.get(process_id) else { - return ProcessStatus::Unknown; - }; - - let exit_code = entry.process.exit_code(); - let process_id = entry.process_id.clone(); - - if entry.process.has_exited() { - let Some(entry) = store.remove(&process_id) else { + let status = { + let mut store = self.process_store.lock().await; + let Some(entry) = store.processes.get(process_id) else { return ProcessStatus::Unknown; }; - ProcessStatus::Exited { - exit_code, - entry: Box::new(entry), - } - } else { - ProcessStatus::Alive { - exit_code, - call_id: entry.call_id.clone(), - process_id, + + let exit_code = entry.process.exit_code(); + let process_id = entry.process_id.clone(); + + if entry.process.has_exited() { + let Some(entry) = store.remove(&process_id) else { + return ProcessStatus::Unknown; + }; + ProcessStatus::Exited { + exit_code, + entry: Box::new(entry), + } + } else { + ProcessStatus::Alive { + exit_code, + call_id: entry.call_id.clone(), + process_id, + } } + }; + if let ProcessStatus::Exited { entry, .. } = &status { + Self::unregister_network_attempt_for_entry(entry).await; } + status } async fn prepare_process_handles( @@ -437,22 +499,32 @@ impl UnifiedExecProcessManager { started_at: Instant, process_id: String, tty: bool, + network_attempt_id: Option, transcript: Arc>, ) { + let network_attempt_id_for_watcher = network_attempt_id.clone(); let entry = ProcessEntry { process: Arc::clone(&process), call_id: context.call_id.clone(), process_id: process_id.clone(), command: command.to_vec(), tty, + network_attempt_id, + session: Arc::downgrade(&context.session), last_used: started_at, }; - let number_processes = { + let (number_processes, pruned_entry) = { let mut store = self.process_store.lock().await; - Self::prune_processes_if_needed(&mut store); + let pruned_entry = Self::prune_processes_if_needed(&mut store); store.processes.insert(process_id.clone(), entry); - store.processes.len() + (store.processes.len(), pruned_entry) }; + // prune_processes_if_needed runs while holding process_store; do async + // network-approval cleanup only after dropping that lock. + if let Some(pruned_entry) = pruned_entry { + Self::unregister_network_attempt_for_entry(&pruned_entry).await; + pruned_entry.process.terminate(); + } if number_processes >= WARNING_UNIFIED_EXEC_PROCESSES { context @@ -471,10 +543,21 @@ impl UnifiedExecProcessManager { context.call_id.clone(), command.to_vec(), cwd, - process_id, + process_id.clone(), transcript, started_at, ); + + if context.turn.config.managed_network_requirements_enabled() + && let Some(network_attempt_id) = network_attempt_id_for_watcher + { + spawn_network_denial_watcher( + Arc::clone(&process), + Arc::clone(&context.session), + process_id, + network_attempt_id, + ); + } } pub(crate) async fn open_session_with_exec_env( @@ -516,7 +599,7 @@ impl UnifiedExecProcessManager { request: &ExecCommandRequest, cwd: PathBuf, context: &UnifiedExecContext, - ) -> Result { + ) -> Result<(UnifiedExecProcess, Option), UnifiedExecError> { let env = apply_unified_exec_env(create_env( &context.turn.shell_environment_policy, Some(context.session.conversation_id), @@ -550,6 +633,7 @@ impl UnifiedExecProcessManager { turn: context.turn.as_ref(), call_id: context.call_id.clone(), tool_name: "exec_command".to_string(), + network_attempt_id: None, }; orchestrator .run( @@ -560,6 +644,7 @@ impl UnifiedExecProcessManager { context.turn.approval_policy, ) .await + .map(|result| (result.output, result.deferred_network_approval)) .map_err(|e| UnifiedExecError::create_process(format!("{e:?}"))) } @@ -643,9 +728,9 @@ impl UnifiedExecProcessManager { collected } - fn prune_processes_if_needed(store: &mut ProcessStore) -> bool { + fn prune_processes_if_needed(store: &mut ProcessStore) -> Option { if store.processes.len() < MAX_UNIFIED_EXEC_PROCESSES { - return false; + return None; } let meta: Vec<(String, Instant, bool)> = store @@ -655,13 +740,10 @@ impl UnifiedExecProcessManager { .collect(); if let Some(process_id) = Self::process_id_to_prune_from_meta(&meta) { - if let Some(entry) = store.remove(&process_id) { - entry.process.terminate(); - } - return true; + return store.remove(&process_id); } - false + None } // Centralized pruning policy so we can easily swap strategies later. @@ -706,6 +788,7 @@ impl UnifiedExecProcessManager { }; for entry in entries { + Self::unregister_network_attempt_for_entry(&entry).await; entry.process.terminate(); } } diff --git a/codex-rs/core/tests/suite/exec.rs b/codex-rs/core/tests/suite/exec.rs index c3cb15875..c857affd2 100644 --- a/codex-rs/core/tests/suite/exec.rs +++ b/codex-rs/core/tests/suite/exec.rs @@ -37,6 +37,7 @@ async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>) -> Result bool { .await { Ok(output) => is_bwrap_unavailable_output(&output), - Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => { + Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => { is_bwrap_unavailable_output(&output) } // Probe timeouts are not actionable for the bwrap-specific assertions below; @@ -142,7 +143,7 @@ fn expect_denied( assert_ne!(output.exit_code, 0, "{context}: expected nonzero exit code"); output } - Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => *output, + Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output, Err(err) => panic!("{context}: {err:?}"), } } @@ -236,6 +237,7 @@ async fn assert_network_blocked(cmd: &[&str]) { expiration: NETWORK_TIMEOUT_MS.into(), env: create_env_from_core_vars(), network: None, + network_attempt_id: None, sandbox_permissions: SandboxPermissions::UseDefault, windows_sandbox_level: WindowsSandboxLevel::Disabled, justification: None, @@ -257,7 +259,7 @@ async fn assert_network_blocked(cmd: &[&str]) { let output = match result { Ok(output) => output, - Err(CodexErr::Sandbox(SandboxErr::Denied { output })) => *output, + Err(CodexErr::Sandbox(SandboxErr::Denied { output, .. })) => *output, _ => { panic!("expected sandbox denied error, got: {result:?}"); } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 6c5b3c25a..5af2191fc 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -221,6 +221,7 @@ async fn run_codex_tool_session_inner( reason: _, proposed_execpolicy_amendment: _, parsed_cmd, + network_approval_context: _, }) => { handle_exec_approval_request( command, diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 0983b07dc..536dd4bf6 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -18,7 +18,9 @@ pub use config::NetworkMode; pub use config::NetworkProxyConfig; pub use config::host_and_port_from_network_addr; pub use network_policy::NetworkDecision; +pub use network_policy::NetworkDecisionSource; pub use network_policy::NetworkPolicyDecider; +pub use network_policy::NetworkPolicyDecision; pub use network_policy::NetworkPolicyRequest; pub use network_policy::NetworkPolicyRequestArgs; pub use network_policy::NetworkProtocol; @@ -34,6 +36,8 @@ pub use proxy::PROXY_URL_ENV_KEYS; pub use proxy::has_proxy_url_env_vars; pub use proxy::proxy_url_env_value; pub use runtime::BlockedRequest; +pub use runtime::BlockedRequestArgs; +pub use runtime::BlockedRequestObserver; pub use runtime::ConfigReloader; pub use runtime::ConfigState; pub use runtime::NetworkProxyState; diff --git a/codex-rs/network-proxy/src/network_policy.rs b/codex-rs/network-proxy/src/network_policy.rs index 201a62cda..fdbbe9281 100644 --- a/codex-rs/network-proxy/src/network_policy.rs +++ b/codex-rs/network-proxy/src/network_policy.rs @@ -26,7 +26,8 @@ impl NetworkProtocol { } } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize, PartialEq, Eq)] +#[serde(rename_all = "lowercase")] pub enum NetworkPolicyDecision { Deny, Ask, @@ -41,7 +42,8 @@ impl NetworkPolicyDecision { } } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[derive(Clone, Copy, Debug, serde::Deserialize, serde::Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] pub enum NetworkDecisionSource { BaselinePolicy, ModeGuard, diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 42f1637c2..55a0eaa16 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -3,7 +3,7 @@ use crate::config; use crate::http_proxy; use crate::metadata::proxy_username_for_attempt_id; use crate::network_policy::NetworkPolicyDecider; -use crate::runtime::BlockedRequest; +use crate::runtime::BlockedRequestObserver; use crate::runtime::unix_socket_permissions_supported; use crate::socks5; use crate::state::NetworkProxyState; @@ -72,6 +72,7 @@ pub struct NetworkProxyBuilder { admin_addr: Option, managed_by_codex: bool, policy_decider: Option>, + blocked_request_observer: Option>, } impl Default for NetworkProxyBuilder { @@ -83,6 +84,7 @@ impl Default for NetworkProxyBuilder { admin_addr: None, managed_by_codex: true, policy_decider: None, + blocked_request_observer: None, } } } @@ -126,12 +128,31 @@ impl NetworkProxyBuilder { self } + pub fn blocked_request_observer(mut self, observer: O) -> Self + where + O: BlockedRequestObserver, + { + self.blocked_request_observer = Some(Arc::new(observer)); + self + } + + pub fn blocked_request_observer_arc( + mut self, + observer: Arc, + ) -> Self { + self.blocked_request_observer = Some(observer); + self + } + pub async fn build(self) -> Result { let state = self.state.ok_or_else(|| { anyhow::anyhow!( "NetworkProxyBuilder requires a state; supply one via builder.state(...)" ) })?; + state + .set_blocked_request_observer(self.blocked_request_observer.clone()) + .await; let current_cfg = state.current_cfg().await?; let (requested_http_addr, requested_socks_addr, requested_admin_addr, reserved_listeners) = if self.managed_by_codex { @@ -399,13 +420,6 @@ impl NetworkProxy { self.admin_addr } - pub async fn latest_blocked_request_for_attempt( - &self, - attempt_id: &str, - ) -> Result> { - self.state.latest_blocked_for_attempt(attempt_id).await - } - pub fn apply_to_env(&self, env: &mut HashMap) { self.apply_to_env_for_attempt(env, None); } diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 737a8dd1d..7917747a8 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -20,6 +20,7 @@ use globset::GlobSet; use serde::Serialize; use std::collections::HashSet; use std::collections::VecDeque; +use std::future::Future; use std::net::IpAddr; use std::path::Path; use std::sync::Arc; @@ -162,9 +163,33 @@ pub trait ConfigReloader: Send + Sync { async fn reload_now(&self) -> Result; } +#[async_trait] +pub trait BlockedRequestObserver: Send + Sync + 'static { + async fn on_blocked_request(&self, request: BlockedRequest); +} + +#[async_trait] +impl BlockedRequestObserver for Arc { + async fn on_blocked_request(&self, request: BlockedRequest) { + (**self).on_blocked_request(request).await + } +} + +#[async_trait] +impl BlockedRequestObserver for F +where + F: Fn(BlockedRequest) -> Fut + Send + Sync + 'static, + Fut: Future + Send, +{ + async fn on_blocked_request(&self, request: BlockedRequest) { + (self)(request).await + } +} + pub struct NetworkProxyState { state: Arc>, reloader: Arc, + blocked_request_observer: Arc>>>, } impl std::fmt::Debug for NetworkProxyState { @@ -180,18 +205,36 @@ impl Clone for NetworkProxyState { Self { state: self.state.clone(), reloader: self.reloader.clone(), + blocked_request_observer: self.blocked_request_observer.clone(), } } } impl NetworkProxyState { pub fn with_reloader(state: ConfigState, reloader: Arc) -> Self { + Self::with_reloader_and_blocked_observer(state, reloader, None) + } + + pub fn with_reloader_and_blocked_observer( + state: ConfigState, + reloader: Arc, + blocked_request_observer: Option>, + ) -> Self { Self { state: Arc::new(RwLock::new(state)), reloader, + blocked_request_observer: Arc::new(RwLock::new(blocked_request_observer)), } } + pub async fn set_blocked_request_observer( + &self, + blocked_request_observer: Option>, + ) { + let mut observer = self.blocked_request_observer.write().await; + *observer = blocked_request_observer; + } + pub async fn current_cfg(&self) -> Result { // Callers treat `NetworkProxyState` as a live view of policy. We reload-on-demand so edits to // `config.toml` (including Codex-managed writes) take effect without a restart. @@ -312,6 +355,8 @@ impl NetworkProxyState { pub async fn record_blocked(&self, entry: BlockedRequest) -> Result<()> { self.reload_if_needed().await?; + let blocked_for_observer = entry.clone(); + let blocked_request_observer = self.blocked_request_observer.read().await.clone(); let violation_line = blocked_request_violation_log_line(&entry); let mut guard = self.state.write().await; let host = entry.host.clone(); @@ -340,6 +385,11 @@ impl NetworkProxyState { guard.blocked.len() ); debug!("{violation_line}"); + drop(guard); + + if let Some(observer) = blocked_request_observer { + observer.on_blocked_request(blocked_for_observer).await; + } Ok(()) } @@ -351,20 +401,6 @@ impl NetworkProxyState { Ok(guard.blocked.iter().cloned().collect()) } - pub async fn latest_blocked_for_attempt( - &self, - attempt_id: &str, - ) -> Result> { - self.reload_if_needed().await?; - let guard = self.state.read().await; - Ok(guard - .blocked - .iter() - .rev() - .find(|entry| entry.attempt_id.as_deref() == Some(attempt_id)) - .cloned()) - } - /// Drain and return the buffered blocked-request entries in FIFO order. pub async fn drain_blocked(&self) -> Result> { self.reload_if_needed().await?; @@ -692,64 +728,6 @@ mod tests { assert_eq!(drained[0].port, snapshot[0].port); } - #[tokio::test] - async fn latest_blocked_for_attempt_returns_latest_matching_entry() { - let state = network_proxy_state_for_policy(NetworkProxySettings::default()); - - state - .record_blocked(BlockedRequest::new(BlockedRequestArgs { - host: "one.example.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: Some("attempt-1".to_string()), - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(80), - })) - .await - .expect("entry should be recorded"); - state - .record_blocked(BlockedRequest::new(BlockedRequestArgs { - host: "two.example.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: Some("attempt-2".to_string()), - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(80), - })) - .await - .expect("entry should be recorded"); - state - .record_blocked(BlockedRequest::new(BlockedRequestArgs { - host: "three.example.com".to_string(), - reason: "not_allowed".to_string(), - client: None, - method: Some("GET".to_string()), - mode: None, - protocol: "http".to_string(), - attempt_id: Some("attempt-1".to_string()), - decision: Some("ask".to_string()), - source: Some("decider".to_string()), - port: Some(80), - })) - .await - .expect("entry should be recorded"); - - let latest = state - .latest_blocked_for_attempt("attempt-1") - .await - .expect("lookup should succeed") - .expect("attempt should have a blocked entry"); - assert_eq!(latest.host, "three.example.com"); - } - #[tokio::test] async fn drain_blocked_returns_buffered_window() { let state = network_proxy_state_for_policy(NetworkProxySettings::default()); diff --git a/codex-rs/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 635cd5223..4ee7bd8b3 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -37,6 +37,24 @@ impl From> for ExecPolicyAmendment { } } +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum NetworkApprovalProtocol { + // TODO(viyatb): Add websocket protocol variants when managed proxy policy + // decisions expose websocket traffic as a distinct approval context. + Http, + #[serde(alias = "https_connect", alias = "http-connect")] + Https, + Socks5Tcp, + Socks5Udp, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +pub struct NetworkApprovalContext { + pub host: String, + pub protocol: NetworkApprovalProtocol, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ExecApprovalRequestEvent { /// Identifier for the associated exec call, if available. @@ -52,6 +70,10 @@ pub struct ExecApprovalRequestEvent { /// Optional human-readable reason for the approval (e.g. retry without sandbox). #[serde(skip_serializing_if = "Option::is_none")] pub reason: Option, + /// Optional network context for a blocked request that can be approved. + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub network_approval_context: Option, /// Proposed execpolicy amendment that can be applied to allow future runs. #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index e44bf4c1d..8a7fa9c38 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -54,6 +54,8 @@ pub use crate::approvals::ApplyPatchApprovalRequestEvent; pub use crate::approvals::ElicitationAction; pub use crate::approvals::ExecApprovalRequestEvent; pub use crate::approvals::ExecPolicyAmendment; +pub use crate::approvals::NetworkApprovalContext; +pub use crate::approvals::NetworkApprovalProtocol; pub use crate::request_user_input::RequestUserInputEvent; /// Open/close tags for special user-input blocks. Used across crates to avoid diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 5435913bb..6881459b9 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2476,6 +2476,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + network_approval_context: None, proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; @@ -2520,6 +2521,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + network_approval_context: None, proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; @@ -2570,6 +2572,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { command: vec!["bash".into(), "-lc".into(), long], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, + network_approval_context: None, proposed_execpolicy_amendment: None, parsed_cmd: vec![], }; @@ -5450,6 +5453,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".into(), "hello".into(), @@ -5506,6 +5510,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, + network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".into(), "hello".into(), @@ -5553,6 +5558,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() command: command.clone(), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, + network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), parsed_cmd: vec![], }; @@ -5912,6 +5918,7 @@ async fn status_widget_and_approval_modal_snapshot() { reason: Some( "this is a test reason such as one that would be produced by the model".into(), ), + network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".into(), "hello world".into(),