From ddb4c2b28c01446926f9c1c84a2222e179ee0851 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 9 Feb 2026 13:34:41 -0800 Subject: [PATCH] feat(core): add requirements-gated network approval context and TUI prompt --- .../app-server/src/bespoke_event_handling.rs | 1 + codex-rs/core/src/codex.rs | 3 + codex-rs/core/src/codex_delegate.rs | 2 + codex-rs/core/src/tools/orchestrator.rs | 167 +++++++++++++++++- codex-rs/core/src/tools/runtimes/shell.rs | 1 + .../core/src/tools/runtimes/unified_exec.rs | 1 + codex-rs/core/src/tools/sandboxing.rs | 2 + codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/approvals.rs | 17 ++ codex-rs/protocol/src/protocol.rs | 2 + .../tui/src/bottom_pane/approval_overlay.rs | 107 ++++++++++- codex-rs/tui/src/bottom_pane/mod.rs | 1 + codex-rs/tui/src/chatwidget.rs | 1 + codex-rs/tui/src/chatwidget/tests.rs | 7 + 14 files changed, 304 insertions(+), 9 deletions(-) diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index e0fc1cfa43..b3992651ec 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -205,6 +205,7 @@ pub(crate) async fn apply_bespoke_event_handling( reason, proposed_execpolicy_amendment, parsed_cmd, + network_approval_context: _, }) => match api_version { ApiVersion::V1 => { let params = ExecCommandApprovalParams { diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 21daadb747..f0f5815bcf 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -169,6 +169,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; @@ -1964,6 +1965,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. @@ -1990,6 +1992,7 @@ impl Session { command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, parsed_cmd, }); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 504b876bbf..257ed62b26 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -315,6 +315,7 @@ async fn handle_exec_approval( command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, .. } = event; @@ -326,6 +327,7 @@ async fn handle_exec_approval( command, cwd, reason, + network_approval_context, proposed_execpolicy_amendment, ); let decision = diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 3f410e1b26..5bc6a8bef2 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -20,13 +20,18 @@ use crate::tools::sandboxing::ToolError; use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::default_exec_approval_requirement; use codex_otel::ToolDecisionSource; +use codex_protocol::approvals::NetworkApprovalContext; +use codex_protocol::approvals::NetworkApprovalProtocol; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::ReviewDecision; +use serde::Deserialize; pub(crate) struct ToolOrchestrator { sandbox: SandboxManager, } +const NETWORK_POLICY_DECISION_PREFIX: &str = "CODEX_NETWORK_POLICY_DECISION "; + impl ToolOrchestrator { pub fn new() -> Self { Self { @@ -70,6 +75,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; @@ -139,12 +145,16 @@ impl ToolOrchestrator { // 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 retry_details = build_denial_reason_from_output( + output.as_ref(), + should_prompt_for_network_approval(turn_ctx), + ); 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_details.reason), + network_approval_context: retry_details.network_approval_context, }; let decision = tool.start_approval_async(req, approval_ctx).await; @@ -179,8 +189,153 @@ impl ToolOrchestrator { } } -fn build_denial_reason_from_output(_output: &ExecToolCallOutput) -> String { - // Keep approval reason terse and stable for UX/tests, but accept the - // output so we can evolve heuristics later without touching call sites. - "command failed; retry without sandbox?".to_string() +#[derive(Debug)] +struct RetryApprovalDetails { + reason: String, + network_approval_context: Option, +} + +fn build_denial_reason_from_output( + output: &ExecToolCallOutput, + network_prompting_enabled: bool, +) -> RetryApprovalDetails { + let network_approval_context = if network_prompting_enabled { + extract_network_approval_context(output) + } else { + None + }; + let 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 { + // Keep approval reason terse and stable for UX/tests, but accept the + // output so we can evolve heuristics later without touching call sites. + "command failed; retry without sandbox?".to_string() + }; + RetryApprovalDetails { + reason, + network_approval_context, + } +} + +fn should_prompt_for_network_approval(turn_ctx: &crate::codex::TurnContext) -> bool { + matches!( + turn_ctx + .config + .config_layer_stack + .requirements_toml() + .network + .as_ref() + .and_then(|network| network.enabled), + Some(true) + ) +} + +fn extract_network_approval_context(output: &ExecToolCallOutput) -> Option { + [ + output.stderr.text.as_str(), + output.stdout.text.as_str(), + output.aggregated_output.text.as_str(), + ] + .into_iter() + .find_map(extract_network_approval_context_from_text) +} + +fn extract_network_approval_context_from_text(text: &str) -> Option { + text.lines() + .find_map(|line| line.strip_prefix(NETWORK_POLICY_DECISION_PREFIX)) + .and_then(|payload| serde_json::from_str::(payload).ok()) + .and_then(|payload| { + if payload.decision != "ask" || payload.source != "decider" { + return None; + } + + let protocol = match payload.protocol.as_str() { + "http" => NetworkApprovalProtocol::Http, + "https" | "https_connect" => NetworkApprovalProtocol::Https, + _ => return None, + }; + + let host = payload.host.trim(); + if host.is_empty() { + return None; + } + + Some(NetworkApprovalContext { + host: host.to_string(), + protocol, + }) + }) +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct NetworkPolicyDecisionPayload { + decision: String, + source: String, + protocol: String, + host: String, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::exec::StreamOutput; + use pretty_assertions::assert_eq; + + fn output_with_stderr(stderr: &str) -> ExecToolCallOutput { + ExecToolCallOutput { + stderr: StreamOutput::new(stderr.to_string()), + ..ExecToolCallOutput::default() + } + } + + #[test] + fn build_denial_reason_extracts_network_context_when_enabled() { + let output = output_with_stderr( + "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked", + ); + + let details = build_denial_reason_from_output(&output, true); + + assert_eq!( + details.network_approval_context, + Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }) + ); + assert_eq!( + details.reason, + "Network access to \"example.com\" is blocked by policy." + ); + } + + #[test] + fn build_denial_reason_skips_network_context_when_disabled() { + let output = output_with_stderr( + "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"decider\",\"protocol\":\"https_connect\",\"host\":\"example.com\",\"port\":443}\nblocked", + ); + + let details = build_denial_reason_from_output(&output, false); + + assert_eq!(details.network_approval_context, None); + assert_eq!(details.reason, "command failed; retry without sandbox?"); + } + + #[test] + fn extract_network_approval_context_ignores_non_ask_payloads() { + let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"deny\",\"source\":\"decider\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}"; + + assert_eq!(extract_network_approval_context_from_text(text), None); + } + + #[test] + fn extract_network_approval_context_ignores_non_decider_payloads() { + let text = "CODEX_NETWORK_POLICY_DECISION {\"decision\":\"ask\",\"source\":\"baseline_policy\",\"protocol\":\"http\",\"host\":\"example.com\",\"port\":80}"; + + assert_eq!(extract_network_approval_context_from_text(text), None); + } } diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index fd076f005e..c4047dd750 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -109,6 +109,7 @@ impl Approvable for ShellRuntime { command, cwd, reason, + ctx.network_approval_context.clone(), req.exec_approval_requirement .proposed_execpolicy_amendment() .cloned(), diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index f3bd2e64b1..71593c6685 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -110,6 +110,7 @@ impl Approvable for UnifiedExecRuntime<'_> { command, cwd, reason, + ctx.network_approval_context.clone(), req.exec_approval_requirement .proposed_execpolicy_amendment() .cloned(), diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 56e54e62f1..32a9f0225a 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -14,6 +14,7 @@ use crate::sandboxing::SandboxTransformError; use crate::state::SessionServices; 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 +111,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. diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 6c5b3c25a2..5af2191fcc 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/protocol/src/approvals.rs b/codex-rs/protocol/src/approvals.rs index 635cd5223d..2ed72ed305 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -37,6 +37,19 @@ impl From> for ExecPolicyAmendment { } } +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum NetworkApprovalProtocol { + Http, + Https, +} + +#[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 +65,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 cbf51fc651..fb0eb75fb4 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -53,6 +53,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/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index 42abe174d4..9f00675790 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -20,6 +20,7 @@ use codex_core::features::Features; use codex_core::protocol::ElicitationAction; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::FileChange; +use codex_core::protocol::NetworkApprovalContext; use codex_core::protocol::Op; use codex_core::protocol::ReviewDecision; use codex_protocol::mcp::RequestId; @@ -42,6 +43,7 @@ pub(crate) enum ApprovalRequest { id: String, command: Vec, reason: Option, + network_approval_context: Option, proposed_execpolicy_amendment: Option, }, ApplyPatch { @@ -108,11 +110,23 @@ impl ApprovalOverlay { ) -> (Vec, SelectionViewParams) { let (options, title) = match &variant { ApprovalVariant::Exec { + network_approval_context, proposed_execpolicy_amendment, .. } => ( - exec_options(proposed_execpolicy_amendment.clone()), - "Would you like to run the following command?".to_string(), + exec_options( + proposed_execpolicy_amendment.clone(), + network_approval_context.as_ref(), + ), + network_approval_context.as_ref().map_or_else( + || "Would you like to run the following command?".to_string(), + |network_approval_context| { + format!( + "Do you want to approve access to \"{}\"?", + network_approval_context.host + ) + }, + ), ), ApprovalVariant::ApplyPatch { .. } => ( patch_options(), @@ -342,6 +356,7 @@ impl From for ApprovalRequestState { id, command, reason, + network_approval_context, proposed_execpolicy_amendment, } => { let mut header: Vec> = Vec::new(); @@ -359,6 +374,7 @@ impl From for ApprovalRequestState { variant: ApprovalVariant::Exec { id, command, + network_approval_context, proposed_execpolicy_amendment, }, header: Box::new(Paragraph::new(header).wrap(Wrap { trim: false })), @@ -414,6 +430,7 @@ enum ApprovalVariant { Exec { id: String, command: Vec, + network_approval_context: Option, proposed_execpolicy_amendment: Option, }, ApplyPatch { @@ -447,7 +464,27 @@ impl ApprovalOption { } } -fn exec_options(proposed_execpolicy_amendment: Option) -> Vec { +fn exec_options( + proposed_execpolicy_amendment: Option, + network_approval_context: Option<&NetworkApprovalContext>, +) -> Vec { + if network_approval_context.is_some() { + return vec![ + ApprovalOption { + label: "Yes, proceed".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Approved), + display_shortcut: None, + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))], + }, + ApprovalOption { + label: "No, and tell Codex what to do differently".to_string(), + decision: ApprovalDecision::Review(ReviewDecision::Abort), + display_shortcut: Some(key_hint::plain(KeyCode::Esc)), + additional_shortcuts: vec![key_hint::plain(KeyCode::Char('n'))], + }, + ]; + } + vec![ApprovalOption { label: "Yes, proceed".to_string(), decision: ApprovalDecision::Review(ReviewDecision::Approved), @@ -531,6 +568,7 @@ fn elicitation_options() -> Vec { mod tests { use super::*; use crate::app_event::AppEvent; + use codex_core::protocol::NetworkApprovalProtocol; use pretty_assertions::assert_eq; use tokio::sync::mpsc::unbounded_channel; @@ -539,6 +577,7 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string(), "hi".to_string()], reason: Some("reason".to_string()), + network_approval_context: None, proposed_execpolicy_amendment: None, } } @@ -581,6 +620,7 @@ mod tests { id: "test".to_string(), command: vec!["echo".to_string()], reason: None, + network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ "echo".to_string(), ])), @@ -619,6 +659,7 @@ mod tests { id: "test".into(), command, reason: None, + network_approval_context: None, proposed_execpolicy_amendment: None, }; @@ -641,6 +682,66 @@ mod tests { ); } + #[test] + fn network_exec_options_use_expected_labels_and_hide_execpolicy_amendment() { + let network_context = NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }; + let options = exec_options( + Some(ExecPolicyAmendment::new(vec!["curl".to_string()])), + Some(&network_context), + ); + + let labels: Vec = options.into_iter().map(|option| option.label).collect(); + assert_eq!( + labels, + vec![ + "Yes, proceed".to_string(), + "No, and tell Codex what to do differently".to_string(), + ] + ); + } + + #[test] + fn network_exec_prompt_title_includes_host() { + let (tx, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx); + let exec_request = ApprovalRequest::Exec { + id: "test".into(), + command: vec!["curl".into(), "https://example.com".into()], + reason: Some("network request blocked".into()), + network_approval_context: Some(NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }), + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec!["curl".into()])), + }; + + let view = ApprovalOverlay::new(exec_request, tx, Features::with_defaults()); + let mut buf = Buffer::empty(Rect::new(0, 0, 100, view.desired_height(100))); + view.render(Rect::new(0, 0, 100, view.desired_height(100)), &mut buf); + + let rendered: Vec = (0..buf.area.height) + .map(|row| { + (0..buf.area.width) + .map(|col| buf[(col, row)].symbol().to_string()) + .collect() + }) + .collect(); + + assert!( + rendered + .iter() + .any(|line| line.contains("Do you want to approve access to \"example.com\"?")), + "expected network title to include host, got {rendered:?}" + ); + assert!( + !rendered.iter().any(|line| line.contains("don't ask again")), + "network prompt should not show execpolicy option, got {rendered:?}" + ); + } + #[test] fn exec_history_cell_wraps_with_two_space_indent() { let command = vec![ diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 32029996ae..f60e1bc888 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -989,6 +989,7 @@ mod tests { id: "1".to_string(), command: vec!["echo".into(), "ok".into()], reason: None, + network_approval_context: None, proposed_execpolicy_amendment: None, } } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 81b360cb4f..ce094ed49c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2393,6 +2393,7 @@ impl ChatWidget { id: ev.call_id, command: ev.command, reason: ev.reason, + network_approval_context: ev.network_approval_context, proposed_execpolicy_amendment: ev.proposed_execpolicy_amendment, }; self.bottom_pane diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index f92dee2a97..c91324f22c 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1934,6 +1934,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { "this is a test reason such as one that would be produced by the model".into(), ), proposed_execpolicy_amendment: None, + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -1978,6 +1979,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { "this is a test reason such as one that would be produced by the model".into(), ), proposed_execpolicy_amendment: None, + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2028,6 +2030,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, proposed_execpolicy_amendment: None, + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -4614,6 +4617,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { "hello".into(), "world".into(), ])), + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -4667,6 +4671,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { "hello".into(), "world".into(), ])), + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -4707,6 +4712,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -5066,6 +5072,7 @@ async fn status_widget_and_approval_modal_snapshot() { "echo".into(), "hello world".into(), ])), + network_approval_context: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event {