From 2d73bac45f9b7a5a3304c17a2e67b9a6fc6ff10c Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Wed, 22 Apr 2026 14:00:53 -0700 Subject: [PATCH] feat: add guardian network approval trigger context (#18197) ## Summary Give guardian network-access reviews the command context that triggered a managed-network approval. The prompt JSON now includes the originating tool call id, tool name, command argv, cwd, sandbox permissions, additional permissions, justification, and tty state when a single active tool call can be attributed. The implementation keeps the trigger shape canonical by serializing `GuardianNetworkAccessTrigger` directly and lets each runtime build that trigger from its `ToolCtx`. Non-guardian approval prompts avoid cloning the full trigger payload. ## UX changes Guardian network-access reviews now include a `trigger` object that explains what command caused the network approval. Instead of seeing only the requested host, the guardian reviewer can also see the originating tool call, argv, working directory, sandbox mode, justification, and tty state. Example payload the guardian reviewer can see: ```json { "tool": "network_access", "target": "https://api.github.com:443", "host": "api.github.com", "protocol": "https", "port": 443, "trigger": { "callId": "call_abc123", "toolName": "shell", "command": ["gh", "api", "/repos/openai/codex/pulls/18197"], "cwd": "/workspace/codex", "sandboxPermissions": "require_escalated", "justification": "Fetch PR metadata from GitHub.", "tty": false } } ``` The network review itself remains scoped to the network decision: `target_item_id` stays `null`. `trigger.callId` is attribution context only, so clients can still distinguish network reviews from item-targeted command reviews. ## Verification - Added coverage for serializing network trigger context in guardian approval JSON. - Added regression coverage that network guardian reviews do not reuse `trigger.callId` as `target_item_id`. --------- Co-authored-by: Codex --- .../core/src/guardian/approval_request.rs | 50 +++++++++--- codex-rs/core/src/guardian/mod.rs | 1 + codex-rs/core/src/guardian/tests.rs | 70 ++++++++++++++++ codex-rs/core/src/tools/network_approval.rs | 33 ++++++-- .../core/src/tools/network_approval_tests.rs | 80 +++++++++++++------ codex-rs/core/src/tools/runtimes/shell.rs | 13 ++- .../core/src/tools/runtimes/unified_exec.rs | 13 ++- 7 files changed, 220 insertions(+), 40 deletions(-) diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index 989c6ee11..2afc5e080 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -54,6 +54,7 @@ pub(crate) enum GuardianApprovalRequest { host: String, protocol: NetworkApprovalProtocol, port: u16, + trigger: Option, }, McpToolCall { id: String, @@ -75,6 +76,22 @@ pub(crate) enum GuardianApprovalRequest { }, } +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub(crate) struct GuardianNetworkAccessTrigger { + pub(crate) call_id: String, + pub(crate) tool_name: String, + pub(crate) command: Vec, + pub(crate) cwd: AbsolutePathBuf, + pub(crate) sandbox_permissions: crate::sandboxing::SandboxPermissions, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) additional_permissions: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) justification: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) tty: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub(crate) struct GuardianMcpAnnotations { #[serde(skip_serializing_if = "Option::is_none")] @@ -131,6 +148,18 @@ struct McpToolCallApprovalAction<'a> { annotations: Option<&'a GuardianMcpAnnotations>, } +#[derive(Serialize)] +#[serde(rename_all = "camelCase")] +struct NetworkAccessApprovalAction<'a> { + tool: &'static str, + target: &'a str, + host: &'a str, + protocol: NetworkApprovalProtocol, + port: u16, + #[serde(skip_serializing_if = "Option::is_none")] + trigger: Option<&'a GuardianNetworkAccessTrigger>, +} + #[derive(Serialize)] struct RequestPermissionsApprovalAction<'a> { tool: &'static str, @@ -297,13 +326,15 @@ pub(crate) fn guardian_approval_request_to_json( host, protocol, port, - } => Ok(serde_json::json!({ - "tool": "network_access", - "target": target, - "host": host, - "protocol": protocol, - "port": port, - })), + trigger, + } => serialize_guardian_action(NetworkAccessApprovalAction { + tool: "network_access", + target, + host, + protocol: *protocol, + port: *port, + trigger: trigger.as_ref(), + }), GuardianApprovalRequest::McpToolCall { id: _, server, @@ -371,12 +402,13 @@ pub(crate) fn guardian_assessment_action( } } GuardianApprovalRequest::NetworkAccess { - id: _, - turn_id: _, + id: _id, + turn_id: _turn_id, target, host, protocol, port, + trigger: _trigger, } => GuardianAssessmentAction::NetworkAccess { target: target.clone(), host: host.clone(), diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 155db6f1f..531815ed7 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -25,6 +25,7 @@ use serde::Serialize; pub(crate) use approval_request::GuardianApprovalRequest; pub(crate) use approval_request::GuardianMcpAnnotations; +pub(crate) use approval_request::GuardianNetworkAccessTrigger; pub(crate) use approval_request::guardian_approval_request_to_json; pub(crate) use review::guardian_rejection_message; pub(crate) use review::guardian_timeout_message; diff --git a/codex-rs/core/src/guardian/tests.rs b/codex-rs/core/src/guardian/tests.rs index cf29c5411..41e33accf 100644 --- a/codex-rs/core/src/guardian/tests.rs +++ b/codex-rs/core/src/guardian/tests.rs @@ -12,6 +12,7 @@ use crate::config_loader::NetworkDomainPermissionToml; use crate::config_loader::NetworkDomainPermissionsToml; use crate::config_loader::RequirementSource; use crate::config_loader::Sourced; +use crate::guardian::approval_request::guardian_request_target_item_id; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::test_support; @@ -727,6 +728,50 @@ fn guardian_approval_request_to_json_renders_mcp_tool_call_shape() -> serde_json Ok(()) } +#[test] +fn guardian_approval_request_to_json_renders_network_access_trigger() -> serde_json::Result<()> { + let cwd = test_path_buf("/repo").abs(); + let action = GuardianApprovalRequest::NetworkAccess { + id: "network-1".to_string(), + turn_id: "turn-1".to_string(), + target: "https://example.com:443".to_string(), + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + port: 443, + trigger: Some(GuardianNetworkAccessTrigger { + call_id: "call-1".to_string(), + tool_name: "shell".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + cwd: cwd.clone(), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("Fetch the release metadata.".to_string()), + tty: None, + }), + }; + + assert_eq!( + guardian_approval_request_to_json(&action)?, + serde_json::json!({ + "tool": "network_access", + "target": "https://example.com:443", + "host": "example.com", + "protocol": "https", + "port": 443, + "trigger": { + "callId": "call-1", + "toolName": "shell", + "command": ["curl", "https://example.com"], + "cwd": cwd.to_string_lossy().to_string(), + "sandboxPermissions": "use_default", + "justification": "Fetch the release metadata.", + }, + }) + ); + + Ok(()) +} + #[test] fn guardian_assessment_action_redacts_apply_patch_patch_text() { let cwd = test_path_buf("/tmp").abs(); @@ -758,6 +803,7 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() { host: "example.com".to_string(), protocol: NetworkApprovalProtocol::Https, port: 443, + trigger: None, }; let apply_patch = GuardianApprovalRequest::ApplyPatch { id: "patch-1".to_string(), @@ -777,6 +823,30 @@ fn guardian_request_turn_id_prefers_network_access_owner_turn() { ); } +#[test] +fn guardian_request_target_item_id_omits_network_access_trigger_call_id() { + let network_access = GuardianApprovalRequest::NetworkAccess { + id: "network-1".to_string(), + turn_id: "owner-turn".to_string(), + target: "https://example.com:443".to_string(), + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + port: 443, + trigger: Some(GuardianNetworkAccessTrigger { + call_id: "call-1".to_string(), + tool_name: "shell".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + cwd: test_path_buf("/repo").abs(), + sandbox_permissions: crate::sandboxing::SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + tty: None, + }), + }; + + assert_eq!(guardian_request_target_item_id(&network_access), None); +} + #[tokio::test] async fn cancelled_guardian_review_emits_terminal_abort_without_warning() { let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await; diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 4be18bad8..d43859e62 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -1,4 +1,5 @@ use crate::guardian::GuardianApprovalRequest; +use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::guardian_rejection_message; use crate::guardian::guardian_timeout_message; use crate::guardian::new_guardian_review_id; @@ -47,6 +48,7 @@ pub(crate) enum NetworkApprovalMode { pub(crate) struct NetworkApprovalSpec { pub network: Option, pub mode: NetworkApprovalMode, + pub trigger: GuardianNetworkAccessTrigger, pub command: String, } @@ -177,6 +179,7 @@ impl PendingHostApproval { struct ActiveNetworkApprovalCall { registration_id: String, turn_id: String, + trigger: GuardianNetworkAccessTrigger, command: String, } @@ -210,7 +213,13 @@ impl NetworkApprovalService { other_approved_hosts.extend(approved_hosts.iter().cloned()); } - async fn register_call(&self, registration_id: String, turn_id: String, command: String) { + async fn register_call( + &self, + registration_id: String, + turn_id: String, + trigger: GuardianNetworkAccessTrigger, + command: String, + ) { let mut active_calls = self.active_calls.lock().await; let key = registration_id.clone(); active_calls.insert( @@ -218,6 +227,7 @@ impl NetworkApprovalService { Arc::new(ActiveNetworkApprovalCall { registration_id, turn_id, + trigger, command, }), ); @@ -369,11 +379,11 @@ impl NetworkApprovalService { return NetworkDecision::deny(REASON_NOT_ALLOWED); } + let owner_call = self.resolve_single_active_call().await; let network_approval_context = NetworkApprovalContext { host: request.host.clone(), protocol, }; - let owner_call = self.resolve_single_active_call().await; let guardian_approval_id = Self::approval_id_for_key(&key); let prompt_command = vec!["network-access".to_string(), target.clone()]; let command = owner_call @@ -431,6 +441,7 @@ impl NetworkApprovalService { host: request.host, protocol, port: key.port, + trigger: owner_call.as_ref().map(|call| call.trigger.clone()), }, Some(policy_denial_message.clone()), ) @@ -623,8 +634,13 @@ pub(crate) async fn begin_network_approval( managed_network_active: bool, spec: Option, ) -> Option { - let spec = spec?; - if !managed_network_active || spec.network.is_none() { + let NetworkApprovalSpec { + network, + mode, + trigger, + command, + } = spec?; + if !managed_network_active || network.is_none() { return None; } @@ -632,12 +648,17 @@ pub(crate) async fn begin_network_approval( session .services .network_approval - .register_call(registration_id.clone(), turn_id.to_string(), spec.command) + .register_call( + registration_id.clone(), + turn_id.to_string(), + trigger, + command, + ) .await; Some(ActiveNetworkApproval { registration_id: Some(registration_id), - mode: spec.mode, + mode, }) } diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 888d0bc35..ac0046228 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -1,7 +1,10 @@ use super::*; +use crate::sandboxing::SandboxPermissions; use codex_network_proxy::BlockedRequestArgs; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; +use core_test_support::PathBufExt; +use core_test_support::test_path_buf; use pretty_assertions::assert_eq; #[tokio::test] @@ -207,17 +210,66 @@ fn denied_blocked_request(host: &str) -> BlockedRequest { }) } +async fn register_call_with_default_shell_trigger( + service: &NetworkApprovalService, + registration_id: &str, +) { + service + .register_call( + registration_id.to_string(), + "turn-1".to_string(), + GuardianNetworkAccessTrigger { + call_id: "call-1".to_string(), + tool_name: "shell".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + cwd: test_path_buf("/tmp").abs(), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: None, + tty: None, + }, + "curl https://example.com".to_string(), + ) + .await; +} + #[tokio::test] -async fn record_blocked_request_sets_policy_outcome_for_owner_call() { +async fn active_call_preserves_triggering_command_context() { let service = NetworkApprovalService::default(); + let expected = GuardianNetworkAccessTrigger { + call_id: "call-1".to_string(), + tool_name: "shell".to_string(), + command: vec!["curl".to_string(), "https://example.com".to_string()], + cwd: test_path_buf("/repo").abs(), + sandbox_permissions: SandboxPermissions::UseDefault, + additional_permissions: None, + justification: Some("fetch release metadata".to_string()), + tty: None, + }; + service .register_call( "registration-1".to_string(), "turn-1".to_string(), - "curl http://example.com".to_string(), + expected.clone(), + "curl https://example.com".to_string(), ) .await; + let call = service + .resolve_single_active_call() + .await + .expect("single active call should resolve"); + + assert_eq!(&call.trigger, &expected); + assert_eq!(call.command, "curl https://example.com"); +} + +#[tokio::test] +async fn record_blocked_request_sets_policy_outcome_for_owner_call() { + let service = NetworkApprovalService::default(); + register_call_with_default_shell_trigger(&service, "registration-1").await; + service .record_blocked_request(denied_blocked_request("example.com")) .await; @@ -233,13 +285,7 @@ async fn record_blocked_request_sets_policy_outcome_for_owner_call() { #[tokio::test] async fn blocked_request_policy_does_not_override_user_denial_outcome() { let service = NetworkApprovalService::default(); - service - .register_call( - "registration-1".to_string(), - "turn-1".to_string(), - "curl http://example.com".to_string(), - ) - .await; + register_call_with_default_shell_trigger(&service, "registration-1").await; service .record_call_outcome("registration-1", NetworkApprovalOutcome::DeniedByUser) @@ -257,20 +303,8 @@ async fn blocked_request_policy_does_not_override_user_denial_outcome() { #[tokio::test] async fn record_blocked_request_ignores_ambiguous_unattributed_blocked_requests() { let service = NetworkApprovalService::default(); - service - .register_call( - "registration-1".to_string(), - "turn-1".to_string(), - "curl http://example.com".to_string(), - ) - .await; - service - .register_call( - "registration-2".to_string(), - "turn-1".to_string(), - "gh api /foo".to_string(), - ) - .await; + register_call_with_default_shell_trigger(&service, "registration-1").await; + register_call_with_default_shell_trigger(&service, "registration-2").await; service .record_blocked_request(denied_blocked_request("example.com")) diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 34292421a..4bbbe774a 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -11,6 +11,7 @@ pub(crate) mod zsh_fork_backend; use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecCapturePolicy; use crate::guardian::GuardianApprovalRequest; +use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::review_approval_request; use crate::sandboxing::ExecOptions; use crate::sandboxing::SandboxPermissions; @@ -217,12 +218,22 @@ impl ToolRuntime for ShellRuntime { fn network_approval_spec( &self, req: &ShellRequest, - _ctx: &ToolCtx, + ctx: &ToolCtx, ) -> Option { req.network.as_ref()?; Some(NetworkApprovalSpec { network: req.network.clone(), mode: NetworkApprovalMode::Immediate, + trigger: GuardianNetworkAccessTrigger { + call_id: ctx.call_id.clone(), + tool_name: ctx.tool_name.clone(), + command: req.command.clone(), + cwd: req.cwd.clone(), + sandbox_permissions: req.sandbox_permissions, + additional_permissions: req.additional_permissions.clone(), + justification: req.justification.clone(), + tty: None, + }, command: req.hook_command.clone(), }) } diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 955f7d830..1a4132da6 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -8,6 +8,7 @@ use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecCapturePolicy; use crate::exec::ExecExpiration; use crate::guardian::GuardianApprovalRequest; +use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::review_approval_request; use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecServerEnvConfig; @@ -202,12 +203,22 @@ impl<'a> ToolRuntime for UnifiedExecRunt fn network_approval_spec( &self, req: &UnifiedExecRequest, - _ctx: &ToolCtx, + ctx: &ToolCtx, ) -> Option { req.network.as_ref()?; Some(NetworkApprovalSpec { network: req.network.clone(), mode: NetworkApprovalMode::Deferred, + trigger: GuardianNetworkAccessTrigger { + call_id: ctx.call_id.clone(), + tool_name: ctx.tool_name.clone(), + command: req.command.clone(), + cwd: req.cwd.clone(), + sandbox_permissions: req.sandbox_permissions, + additional_permissions: req.additional_permissions.clone(), + justification: req.justification.clone(), + tty: Some(req.tty), + }, command: req.hook_command.clone(), }) }