diff --git a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json index b00ac1184e7..37380f59883 100644 --- a/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ApplyPatchApprovalResponse.json @@ -1,6 +1,28 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" + }, "ReviewDecision": { "description": "User's decision in response to an ExecApprovalRequest.", "oneOf": [ @@ -43,6 +65,28 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.", + "properties": { + "network_policy_amendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "network_policy_amendment" + ], + "title": "NetworkPolicyAmendmentReviewDecision", + "type": "object" + }, { "description": "User has denied this command and the agent should not execute it, but it should continue the session and try something else.", "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 81140db5e92..ea55a572dcd 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -1662,6 +1662,16 @@ "null" ] }, + "proposed_network_policy_amendments": { + "description": "Proposed network policy amendments (for example allow/deny this host in future).", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional human-readable reason for the approval (e.g. retry without sandbox).", "type": [ @@ -3637,6 +3647,28 @@ ], "type": "string" }, + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { @@ -6907,6 +6939,16 @@ "null" ] }, + "proposed_network_policy_amendments": { + "description": "Proposed network policy amendments (for example allow/deny this host in future).", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional human-readable reason for the approval (e.g. retry without sandbox).", "type": [ diff --git a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json index 1f278291a25..ac15f23658e 100644 --- a/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json +++ b/codex-rs/app-server-protocol/schema/json/ExecCommandApprovalResponse.json @@ -1,6 +1,28 @@ { "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" + }, "ReviewDecision": { "description": "User's decision in response to an ExecApprovalRequest.", "oneOf": [ @@ -43,6 +65,28 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.", + "properties": { + "network_policy_amendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "network_policy_amendment" + ], + "title": "NetworkPolicyAmendmentReviewDecision", + "type": "object" + }, { "description": "User has denied this command and the agent should not execute it, but it should continue the session and try something else.", "enum": [ 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 d478bb2c78e..fb9019aebb8 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 @@ -2723,6 +2723,16 @@ "null" ] }, + "proposed_network_policy_amendments": { + "description": "Proposed network policy amendments (for example allow/deny this host in future).", + "items": { + "$ref": "#/definitions/NetworkPolicyAmendment" + }, + "type": [ + "array", + "null" + ] + }, "reason": { "description": "Optional human-readable reason for the approval (e.g. retry without sandbox).", "type": [ @@ -4898,6 +4908,28 @@ ], "type": "string" }, + "NetworkPolicyAmendment": { + "properties": { + "action": { + "$ref": "#/definitions/NetworkPolicyRuleAction" + }, + "host": { + "type": "string" + } + }, + "required": [ + "action", + "host" + ], + "type": "object" + }, + "NetworkPolicyRuleAction": { + "enum": [ + "allow", + "deny" + ], + "type": "string" + }, "ParsedCommand": { "oneOf": [ { @@ -5310,6 +5342,28 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "User chose to persist a network policy rule (allow/deny) for future requests to the same host.", + "properties": { + "network_policy_amendment": { + "properties": { + "network_policy_amendment": { + "$ref": "#/definitions/NetworkPolicyAmendment" + } + }, + "required": [ + "network_policy_amendment" + ], + "type": "object" + } + }, + "required": [ + "network_policy_amendment" + ], + "title": "NetworkPolicyAmendmentReviewDecision", + "type": "object" + }, { "description": "User has denied this command and the agent should not execute it, but it should continue the session and try something else.", "enum": [ diff --git a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts index d0a7fef5cb2..71b28576073 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ExecApprovalRequestEvent.ts @@ -3,6 +3,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 { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; import type { ParsedCommand } from "./ParsedCommand"; export type ExecApprovalRequestEvent = { @@ -41,4 +42,8 @@ network_approval_context?: NetworkApprovalContext, /** * Proposed execpolicy amendment that can be applied to allow future runs. */ -proposed_execpolicy_amendment?: ExecPolicyAmendment, parsed_cmd: Array, }; +proposed_execpolicy_amendment?: ExecPolicyAmendment, +/** + * Proposed network policy amendments (for example allow/deny this host in future). + */ +proposed_network_policy_amendments?: Array, parsed_cmd: Array, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyAmendment.ts b/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyAmendment.ts new file mode 100644 index 00000000000..4e5092e4d0c --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyAmendment.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 { NetworkPolicyRuleAction } from "./NetworkPolicyRuleAction"; + +export type NetworkPolicyAmendment = { host: string, action: NetworkPolicyRuleAction, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyRuleAction.ts b/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyRuleAction.ts new file mode 100644 index 00000000000..55ec70032a6 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/NetworkPolicyRuleAction.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 NetworkPolicyRuleAction = "allow" | "deny"; diff --git a/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts b/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts index 662fae625a7..b5193785d86 100644 --- a/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts +++ b/codex-rs/app-server-protocol/schema/typescript/ReviewDecision.ts @@ -2,8 +2,9 @@ // 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 { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; /** * User's decision in response to an ExecApprovalRequest. */ -export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | "denied" | "abort"; +export type ReviewDecision = "approved" | { "approved_execpolicy_amendment": { proposed_execpolicy_amendment: ExecPolicyAmendment, } } | "approved_for_session" | { "network_policy_amendment": { network_policy_amendment: NetworkPolicyAmendment, } } | "denied" | "abort"; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 30197f89229..e38f97df4a9 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -132,6 +132,8 @@ export type { ModelRerouteReason } from "./ModelRerouteReason"; export type { NetworkAccess } from "./NetworkAccess"; export type { NetworkApprovalContext } from "./NetworkApprovalContext"; export type { NetworkApprovalProtocol } from "./NetworkApprovalProtocol"; +export type { NetworkPolicyAmendment } from "./NetworkPolicyAmendment"; +export type { NetworkPolicyRuleAction } from "./NetworkPolicyRuleAction"; export type { NewConversationParams } from "./NewConversationParams"; export type { NewConversationResponse } from "./NewConversationResponse"; export type { ParsedCommand } from "./ParsedCommand"; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 249cb99776d..fb614f12cf3 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -55,8 +55,11 @@ use codex_hooks::HookResult; use codex_hooks::Hooks; use codex_hooks::HooksConfig; use codex_network_proxy::NetworkProxy; +use codex_network_proxy::normalize_host; use codex_protocol::ThreadId; use codex_protocol::approvals::ExecPolicyAmendment; +use codex_protocol::approvals::NetworkPolicyAmendment; +use codex_protocol::approvals::NetworkPolicyRuleAction; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; use codex_protocol::config_types::WebSearchMode; @@ -165,6 +168,7 @@ use crate::mentions::build_connector_slug_counts; use crate::mentions::build_skill_name_counts; use crate::mentions::collect_explicit_app_ids; use crate::mentions::collect_tool_mentions_from_messages; +use crate::network_policy_decision::execpolicy_network_rule_amendment; use crate::project_doc::get_user_instructions; use crate::proposed_plan_parser::ProposedPlanParser; use crate::proposed_plan_parser::ProposedPlanSegment; @@ -2377,6 +2381,103 @@ impl Session { } } + pub(crate) async fn persist_network_policy_amendment( + &self, + amendment: &NetworkPolicyAmendment, + network_approval_context: &NetworkApprovalContext, + ) -> anyhow::Result<()> { + let host = + Self::validated_network_policy_amendment_host(amendment, network_approval_context)?; + let codex_home = self + .state + .lock() + .await + .session_configuration + .codex_home() + .clone(); + let execpolicy_amendment = + execpolicy_network_rule_amendment(amendment, network_approval_context, &host); + + if let Some(started_network_proxy) = self.services.network_proxy.as_ref() { + let proxy = started_network_proxy.proxy(); + match amendment.action { + NetworkPolicyRuleAction::Allow => proxy + .add_allowed_domain(&host) + .await + .map_err(|err| anyhow::anyhow!("failed to update runtime allowlist: {err}"))?, + NetworkPolicyRuleAction::Deny => proxy + .add_denied_domain(&host) + .await + .map_err(|err| anyhow::anyhow!("failed to update runtime denylist: {err}"))?, + } + } + + self.services + .exec_policy + .append_network_rule_and_update( + &codex_home, + &host, + execpolicy_amendment.protocol, + execpolicy_amendment.decision, + Some(execpolicy_amendment.justification), + ) + .await + .map_err(|err| { + anyhow::anyhow!("failed to persist network policy amendment to execpolicy: {err}") + })?; + + Ok(()) + } + + fn validated_network_policy_amendment_host( + amendment: &NetworkPolicyAmendment, + network_approval_context: &NetworkApprovalContext, + ) -> anyhow::Result { + let approved_host = normalize_host(&network_approval_context.host); + let amendment_host = normalize_host(&amendment.host); + if amendment_host != approved_host { + return Err(anyhow::anyhow!( + "network policy amendment host '{}' does not match approved host '{}'", + amendment.host, + network_approval_context.host + )); + } + Ok(approved_host) + } + + pub(crate) async fn record_network_policy_amendment_message( + &self, + sub_id: &str, + amendment: &NetworkPolicyAmendment, + ) { + let (action, list_name) = match amendment.action { + NetworkPolicyRuleAction::Allow => ("Allowed", "allowlist"), + NetworkPolicyRuleAction::Deny => ("Denied", "denylist"), + }; + let text = format!( + "{action} network rule saved in execpolicy ({list_name}): {}", + amendment.host + ); + let message: ResponseItem = DeveloperInstructions::new(text.clone()).into(); + + if let Some(turn_context) = self.turn_context_for_sub_id(sub_id).await { + self.record_conversation_items(&turn_context, std::slice::from_ref(&message)) + .await; + return; + } + + if self + .inject_response_items(vec![ResponseInputItem::Message { + role: "developer".to_string(), + content: vec![ContentItem::InputText { text }], + }]) + .await + .is_err() + { + warn!("no active turn found to record network policy amendment message for {sub_id}"); + } + } + /// Emit an exec approval request event and await the user's decision. /// /// The request is keyed by `call_id` + `approval_id` so matching responses are delivered @@ -2414,6 +2515,18 @@ impl Session { } let parsed_cmd = parse_command(&command); + let proposed_network_policy_amendments = network_approval_context.as_ref().map(|context| { + vec![ + NetworkPolicyAmendment { + host: context.host.clone(), + action: NetworkPolicyRuleAction::Allow, + }, + NetworkPolicyAmendment { + host: context.host.clone(), + action: NetworkPolicyRuleAction::Deny, + }, + ] + }); let event = EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent { call_id, approval_id, @@ -2423,6 +2536,7 @@ impl Session { reason, network_approval_context, proposed_execpolicy_amendment, + proposed_network_policy_amendments, parsed_cmd, }); self.send_event(turn_context, event).await; @@ -6120,6 +6234,7 @@ mod tests { use crate::protocol::CompactedItem; use crate::protocol::CreditsSnapshot; use crate::protocol::InitialHistory; + use crate::protocol::NetworkApprovalProtocol; use crate::protocol::RateLimitSnapshot; use crate::protocol::RateLimitWindow; use crate::protocol::ResumedHistory; @@ -6246,6 +6361,41 @@ mod tests { }) } + #[test] + fn validated_network_policy_amendment_host_allows_normalized_match() { + let amendment = NetworkPolicyAmendment { + host: "ExAmPlE.Com.:443".to_string(), + action: NetworkPolicyRuleAction::Allow, + }; + let context = NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }; + + let host = Session::validated_network_policy_amendment_host(&amendment, &context) + .expect("normalized hosts should match"); + + assert_eq!(host, "example.com"); + } + + #[test] + fn validated_network_policy_amendment_host_rejects_mismatch() { + let amendment = NetworkPolicyAmendment { + host: "evil.example.com".to_string(), + action: NetworkPolicyRuleAction::Deny, + }; + let context = NetworkApprovalContext { + host: "api.example.com".to_string(), + protocol: NetworkApprovalProtocol::Https, + }; + + let err = Session::validated_network_policy_amendment_host(&amendment, &context) + .expect_err("mismatched hosts should be rejected"); + + let message = err.to_string(); + assert!(message.contains("does not match approved host")); + } + #[tokio::test] async fn get_base_instructions_no_user_content() { let prompt_with_apply_patch_instructions = diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 52329a255fd..d89961aeccf 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -13,10 +13,12 @@ use codex_execpolicy::AmendError; use codex_execpolicy::Decision; use codex_execpolicy::Error as ExecPolicyRuleError; use codex_execpolicy::Evaluation; +use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; use codex_execpolicy::blocking_append_allow_prefix_rule; +use codex_execpolicy::blocking_append_network_rule; use codex_protocol::approvals::ExecPolicyAmendment; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; @@ -293,6 +295,43 @@ impl ExecPolicyManager { self.policy.store(Arc::new(updated_policy)); Ok(()) } + + pub(crate) async fn append_network_rule_and_update( + &self, + codex_home: &Path, + host: &str, + protocol: NetworkRuleProtocol, + decision: Decision, + justification: Option, + ) -> Result<(), ExecPolicyUpdateError> { + let policy_path = default_policy_path(codex_home); + let host = host.to_string(); + spawn_blocking({ + let policy_path = policy_path.clone(); + let host = host.clone(); + let justification = justification.clone(); + move || { + blocking_append_network_rule( + &policy_path, + &host, + protocol, + decision, + justification.as_deref(), + ) + } + }) + .await + .map_err(|source| ExecPolicyUpdateError::JoinBlockingTask { source })? + .map_err(|source| ExecPolicyUpdateError::AppendRule { + path: policy_path, + source, + })?; + + let mut updated_policy = self.current().as_ref().clone(); + updated_policy.add_network_rule(&host, protocol, decision, justification)?; + self.policy.store(Arc::new(updated_policy)); + Ok(()) + } } impl Default for ExecPolicyManager { @@ -440,7 +479,10 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result anyhow::Result<()> { + let temp_dir = tempdir()?; + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy.add_network_rule( + "blocked.example.com", + codex_execpolicy::NetworkRuleProtocol::Https, + Decision::Forbidden, + None, + )?; + + let requirements = ConfigRequirements { + exec_policy: Some(codex_config::Sourced::new( + codex_config::RequirementsExecPolicy::new(requirements_exec_policy), + codex_config::RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path())?; + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + let config_stack = + ConfigLayerStack::new(vec![layer], requirements, ConfigRequirementsToml::default())?; + + let policy = load_exec_policy(&config_stack).await?; + let (allowed, denied) = policy.compiled_network_domains(); + + assert!(allowed.is_empty()); + assert_eq!(denied, vec!["blocked.example.com".to_string()]); + Ok(()) + } + #[tokio::test] async fn ignores_policies_outside_policy_dir() { let temp_dir = tempdir().expect("create temp dir"); diff --git a/codex-rs/core/src/network_policy_decision.rs b/codex-rs/core/src/network_policy_decision.rs index 81ba7db82d1..e40ae854c48 100644 --- a/codex-rs/core/src/network_policy_decision.rs +++ b/codex-rs/core/src/network_policy_decision.rs @@ -1,8 +1,12 @@ +use codex_execpolicy::Decision as ExecPolicyDecision; +use codex_execpolicy::NetworkRuleProtocol as ExecPolicyNetworkRuleProtocol; 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 codex_protocol::approvals::NetworkPolicyAmendment; +use codex_protocol::approvals::NetworkPolicyRuleAction; use serde::Deserialize; #[derive(Debug, Clone, Deserialize, PartialEq, Eq)] @@ -17,6 +21,13 @@ pub struct NetworkPolicyDecisionPayload { pub port: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct ExecPolicyNetworkRuleAmendment { + pub protocol: ExecPolicyNetworkRuleProtocol, + pub decision: ExecPolicyDecision, + pub justification: String, +} + impl NetworkPolicyDecisionPayload { pub(crate) fn is_ask_from_decider(&self) -> bool { self.decision == NetworkPolicyDecision::Ask && self.source == NetworkDecisionSource::Decider @@ -79,10 +90,42 @@ pub(crate) fn denied_network_policy_message(blocked: &BlockedRequest) -> Option< )) } +pub(crate) fn execpolicy_network_rule_amendment( + amendment: &NetworkPolicyAmendment, + network_approval_context: &NetworkApprovalContext, + host: &str, +) -> ExecPolicyNetworkRuleAmendment { + let protocol = match network_approval_context.protocol { + NetworkApprovalProtocol::Http => ExecPolicyNetworkRuleProtocol::Http, + NetworkApprovalProtocol::Https => ExecPolicyNetworkRuleProtocol::Https, + NetworkApprovalProtocol::Socks5Tcp => ExecPolicyNetworkRuleProtocol::Socks5Tcp, + NetworkApprovalProtocol::Socks5Udp => ExecPolicyNetworkRuleProtocol::Socks5Udp, + }; + let (decision, action_verb) = match amendment.action { + NetworkPolicyRuleAction::Allow => (ExecPolicyDecision::Allow, "Allow"), + NetworkPolicyRuleAction::Deny => (ExecPolicyDecision::Forbidden, "Deny"), + }; + let protocol_label = match network_approval_context.protocol { + NetworkApprovalProtocol::Http => "http", + NetworkApprovalProtocol::Https => "https_connect", + NetworkApprovalProtocol::Socks5Tcp => "socks5_tcp", + NetworkApprovalProtocol::Socks5Udp => "socks5_udp", + }; + let justification = format!("{action_verb} {protocol_label} access to {host}"); + + ExecPolicyNetworkRuleAmendment { + protocol, + decision, + justification, + } +} + #[cfg(test)] mod tests { use super::*; use codex_network_proxy::BlockedRequest; + use codex_protocol::approvals::NetworkPolicyAmendment; + use codex_protocol::approvals::NetworkPolicyRuleAction; use pretty_assertions::assert_eq; #[test] @@ -211,6 +254,27 @@ mod tests { assert_eq!(payload.protocol, Some(NetworkApprovalProtocol::Https)); } + #[test] + fn execpolicy_network_rule_amendment_maps_protocol_action_and_justification() { + let amendment = NetworkPolicyAmendment { + action: NetworkPolicyRuleAction::Deny, + host: "example.com".to_string(), + }; + let context = NetworkApprovalContext { + host: "example.com".to_string(), + protocol: NetworkApprovalProtocol::Socks5Udp, + }; + + assert_eq!( + execpolicy_network_rule_amendment(&amendment, &context, "example.com"), + ExecPolicyNetworkRuleAmendment { + protocol: ExecPolicyNetworkRuleProtocol::Socks5Udp, + decision: ExecPolicyDecision::Forbidden, + justification: "Deny socks5_udp access to example.com".to_string(), + } + ); + } + #[test] fn denied_network_policy_message_requires_deny_decision() { let blocked = BlockedRequest { diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 15d4e41664e..ce865756eeb 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -6,6 +6,9 @@ use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; +use crate::exec_policy::ExecPolicyError; +use crate::exec_policy::format_exec_policy_error_with_source; +use crate::exec_policy::load_exec_policy; use anyhow::Context; use anyhow::Result; use async_trait::async_trait; @@ -18,6 +21,7 @@ use codex_network_proxy::NetworkProxyConstraintError; use codex_network_proxy::NetworkProxyConstraints; use codex_network_proxy::NetworkProxyState; use codex_network_proxy::build_config_state; +use codex_network_proxy::normalize_host; use codex_network_proxy::validate_policy_against_constraints; use serde::Deserialize; use std::path::PathBuf; @@ -49,7 +53,21 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec (policy, None), + Err(err @ ExecPolicyError::ParsePolicy { .. }) => { + (codex_execpolicy::Policy::empty(), Some(err)) + } + Err(err) => return Err(err.into()), + }; + if let Some(err) = warning.as_ref() { + tracing::warn!( + "failed to parse execpolicy while building network proxy state: {}", + format_exec_policy_error_with_source(err) + ); + } + + let config = config_from_layers(&config_layer_stack, &exec_policy)?; let constraints = enforce_trusted_constraints(&config_layer_stack, &config)?; let layer_mtimes = collect_layer_mtimes(&config_layer_stack); @@ -175,15 +193,46 @@ fn apply_network_tables(config: &mut NetworkProxyConfig, parsed: NetworkTablesTo } } -fn config_from_layers(layers: &ConfigLayerStack) -> Result { +fn config_from_layers( + layers: &ConfigLayerStack, + exec_policy: &codex_execpolicy::Policy, +) -> Result { let mut config = NetworkProxyConfig::default(); for layer in layers.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false) { let parsed = network_tables_from_toml(&layer.config)?; apply_network_tables(&mut config, parsed); } + apply_exec_policy_network_rules(&mut config, exec_policy); Ok(config) } +fn apply_exec_policy_network_rules( + config: &mut NetworkProxyConfig, + exec_policy: &codex_execpolicy::Policy, +) { + let (allowed_domains, denied_domains) = exec_policy.compiled_network_domains(); + for host in allowed_domains { + upsert_network_domain( + &mut config.network.allowed_domains, + &mut config.network.denied_domains, + host, + ); + } + for host in denied_domains { + upsert_network_domain( + &mut config.network.denied_domains, + &mut config.network.allowed_domains, + host, + ); + } +} + +fn upsert_network_domain(target: &mut Vec, opposite: &mut Vec, host: String) { + opposite.retain(|entry| normalize_host(entry) != host); + target.retain(|entry| normalize_host(entry) != host); + target.push(host); +} + fn is_user_controlled_layer(layer: &ConfigLayerSource) -> bool { matches!( layer, @@ -260,6 +309,9 @@ impl ConfigReloader for MtimeConfigReloader { mod tests { use super::*; + use codex_execpolicy::Decision; + use codex_execpolicy::NetworkRuleProtocol; + use codex_execpolicy::Policy; use pretty_assertions::assert_eq; #[test] @@ -292,6 +344,45 @@ allowed_domains = ["higher.example.com"] assert_eq!(config.network.allowed_domains, vec!["higher.example.com"]); } + #[test] + fn execpolicy_network_rules_overlay_network_lists() { + let mut config = NetworkProxyConfig::default(); + config.network.allowed_domains = vec!["config.example.com".to_string()]; + config.network.denied_domains = vec!["blocked.example.com".to_string()]; + + let mut exec_policy = Policy::empty(); + exec_policy + .add_network_rule( + "blocked.example.com", + NetworkRuleProtocol::Https, + Decision::Allow, + None, + ) + .expect("allow rule should be valid"); + exec_policy + .add_network_rule( + "api.example.com", + NetworkRuleProtocol::Http, + Decision::Forbidden, + None, + ) + .expect("deny rule should be valid"); + + apply_exec_policy_network_rules(&mut config, &exec_policy); + + assert_eq!( + config.network.allowed_domains, + vec![ + "config.example.com".to_string(), + "blocked.example.com".to_string() + ] + ); + assert_eq!( + config.network.denied_domains, + vec!["api.example.com".to_string()] + ); + } + #[test] fn apply_network_constraints_includes_allow_all_unix_sockets_flag() { let config: toml::Value = toml::from_str( diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index 0d4f2f30782..d6a71514516 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -10,8 +10,12 @@ use codex_network_proxy::NetworkProtocol; use codex_network_proxy::NetworkProxy; use codex_protocol::approvals::NetworkApprovalContext; use codex_protocol::approvals::NetworkApprovalProtocol; +use codex_protocol::approvals::NetworkPolicyRuleAction; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::Event; +use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ReviewDecision; +use codex_protocol::protocol::WarningEvent; use indexmap::IndexMap; use std::collections::HashMap; use std::collections::HashSet; @@ -19,6 +23,7 @@ use std::sync::Arc; use tokio::sync::Mutex; use tokio::sync::Notify; use tokio::sync::RwLock; +use tracing::warn; use uuid::Uuid; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -158,6 +163,7 @@ pub(crate) struct NetworkApprovalService { call_outcomes: Mutex>, pending_host_approvals: Mutex>>, session_approved_hosts: Mutex>, + session_denied_hosts: Mutex>, } impl Default for NetworkApprovalService { @@ -167,6 +173,7 @@ impl Default for NetworkApprovalService { call_outcomes: Mutex::new(HashMap::new()), pending_host_approvals: Mutex::new(HashMap::new()), session_approved_hosts: Mutex::new(HashSet::new()), + session_denied_hosts: Mutex::new(HashSet::new()), } } } @@ -272,6 +279,13 @@ impl NetworkApprovalService { }; let key = HostApprovalKey::from_request(&request, protocol); + { + let denied_hosts = self.session_denied_hosts.lock().await; + if denied_hosts.contains(&key) { + return NetworkDecision::deny(REASON_NOT_ALLOWED); + } + } + { let approved_hosts = self.session_approved_hosts.lock().await; if approved_hosts.contains(&key) { @@ -312,6 +326,10 @@ impl NetworkApprovalService { let approval_id = Self::approval_id_for_key(&key); let prompt_command = vec!["network-access".to_string(), target.clone()]; + let network_approval_context = NetworkApprovalContext { + host: request.host.clone(), + protocol, + }; let approval_decision = session .request_command_approval( @@ -321,19 +339,86 @@ impl NetworkApprovalService { prompt_command, turn_context.cwd.clone(), Some(prompt_reason), - Some(NetworkApprovalContext { - host: request.host.clone(), - protocol, - }), + Some(network_approval_context.clone()), None, ) .await; + let mut cache_session_deny = false; let resolved = match approval_decision { ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { PendingApprovalDecision::AllowOnce } ReviewDecision::ApprovedForSession => PendingApprovalDecision::AllowForSession, + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => { + match session + .persist_network_policy_amendment( + &network_policy_amendment, + &network_approval_context, + ) + .await + { + Ok(()) => { + session + .record_network_policy_amendment_message( + &turn_context.sub_id, + &network_policy_amendment, + ) + .await; + } + Err(err) => { + let message = + format!("Failed to apply network policy amendment: {err}"); + warn!("{message}"); + session + .send_event_raw(Event { + id: turn_context.sub_id.clone(), + msg: EventMsg::Warning(WarningEvent { message }), + }) + .await; + } + } + PendingApprovalDecision::AllowForSession + } + NetworkPolicyRuleAction::Deny => { + match session + .persist_network_policy_amendment( + &network_policy_amendment, + &network_approval_context, + ) + .await + { + Ok(()) => { + session + .record_network_policy_amendment_message( + &turn_context.sub_id, + &network_policy_amendment, + ) + .await; + } + Err(err) => { + let message = + format!("Failed to apply network policy amendment: {err}"); + warn!("{message}"); + session + .send_event_raw(Event { + id: turn_context.sub_id.clone(), + msg: EventMsg::Warning(WarningEvent { message }), + }) + .await; + } + } + self.record_outcome_for_single_active_call( + NetworkApprovalOutcome::DeniedByUser, + ) + .await; + cache_session_deny = true; + PendingApprovalDecision::Deny + } + }, ReviewDecision::Denied | ReviewDecision::Abort => { self.record_outcome_for_single_active_call(NetworkApprovalOutcome::DeniedByUser) .await; @@ -342,10 +427,23 @@ impl NetworkApprovalService { }; if matches!(resolved, PendingApprovalDecision::AllowForSession) { + { + let mut denied_hosts = self.session_denied_hosts.lock().await; + denied_hosts.remove(&key); + } let mut approved_hosts = self.session_approved_hosts.lock().await; approved_hosts.insert(key.clone()); } + if cache_session_deny { + { + let mut approved_hosts = self.session_approved_hosts.lock().await; + approved_hosts.remove(&key); + } + let mut denied_hosts = self.session_denied_hosts.lock().await; + denied_hosts.insert(key.clone()); + } + pending.set_decision(resolved).await; let mut pending_approvals = self.pending_host_approvals.lock().await; pending_approvals.remove(&key); diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index db28c931310..f66c79bbb02 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -27,6 +27,7 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::default_exec_approval_requirement; use codex_otel::ToolDecisionSource; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::NetworkPolicyRuleAction; use codex_protocol::protocol::ReviewDecision; pub(crate) struct ToolOrchestrator { @@ -145,6 +146,14 @@ impl ToolOrchestrator { ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => {} + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => {} + NetworkPolicyRuleAction::Deny => { + return Err(ToolError::Rejected("rejected by user".to_string())); + } + }, } already_approved = true; } @@ -273,6 +282,14 @@ impl ToolOrchestrator { ReviewDecision::Approved | ReviewDecision::ApprovedExecpolicyAmendment { .. } | ReviewDecision::ApprovedForSession => {} + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => {} + NetworkPolicyRuleAction::Deny => { + return Err(ToolError::Rejected("rejected by user".to_string())); + } + }, } } diff --git a/codex-rs/core/src/zsh_exec_bridge/mod.rs b/codex-rs/core/src/zsh_exec_bridge/mod.rs index 8094a4b786c..932bdc02bb4 100644 --- a/codex-rs/core/src/zsh_exec_bridge/mod.rs +++ b/codex-rs/core/src/zsh_exec_bridge/mod.rs @@ -15,6 +15,8 @@ use crate::protocol::ExecCommandOutputDeltaEvent; #[cfg(unix)] use crate::protocol::ExecOutputStream; #[cfg(unix)] +use crate::protocol::NetworkPolicyRuleAction; +#[cfg(unix)] use crate::protocol::ReviewDecision; #[cfg(unix)] use anyhow::Context as _; @@ -373,6 +375,16 @@ impl ZshExecBridge { | ReviewDecision::ApprovedExecpolicyAmendment { .. } => { (WrapperExecAction::Run, None, false) } + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => (WrapperExecAction::Run, None, false), + NetworkPolicyRuleAction::Deny => ( + WrapperExecAction::Deny, + Some("command denied by host approval policy".to_string()), + true, + ), + }, ReviewDecision::Denied => ( WrapperExecAction::Deny, Some("command denied by host approval policy".to_string()), diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 38ae062ab2d..9444791991d 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -2,8 +2,17 @@ use anyhow::Result; use codex_core::config::Constrained; +use codex_core::config_loader::ConfigLayerStack; +use codex_core::config_loader::ConfigLayerStackOrdering; +use codex_core::config_loader::NetworkConstraints; +use codex_core::config_loader::NetworkRequirementsToml; +use codex_core::config_loader::RequirementSource; +use codex_core::config_loader::Sourced; use codex_core::features::Feature; use codex_core::sandboxing::SandboxPermissions; +use codex_protocol::approvals::NetworkApprovalProtocol; +use codex_protocol::approvals::NetworkPolicyAmendment; +use codex_protocol::approvals::NetworkPolicyRuleAction; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::protocol::ApplyPatchApprovalRequestEvent; use codex_protocol::protocol::AskForApproval; @@ -26,6 +35,7 @@ use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; +use core_test_support::wait_for_event_with_timeout; use pretty_assertions::assert_eq; use regex_lite::Regex; use serde_json::Value; @@ -33,6 +43,8 @@ use serde_json::json; use std::env; use std::fs; use std::path::PathBuf; +use std::sync::Arc; +use tempfile::TempDir; use wiremock::Mock; use wiremock::MockServer; use wiremock::ResponseTemplate; @@ -2106,6 +2118,305 @@ async fn approving_fallback_rule_for_compound_command_works() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "current_thread")] +async fn denying_network_policy_amendment_persists_policy_and_skips_future_network_prompt() +-> Result<()> { + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + fs::write( + home.path().join("config.toml"), + r#"[permissions.network] +enabled = true +mode = "limited" +allow_local_binding = true +"#, + )?; + let approval_policy = AskForApproval::OnFailure; + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: true, + exclude_tmpdir_env_var: false, + exclude_slash_tmp: false, + }; + let sandbox_policy_for_config = sandbox_policy.clone(); + let mut builder = test_codex().with_home(home).with_config(move |config| { + config.permissions.approval_policy = Constrained::allow_any(approval_policy); + config.permissions.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config); + let layers = config + .config_layer_stack + .get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, true) + .into_iter() + .cloned() + .collect(); + let mut requirements = config.config_layer_stack.requirements().clone(); + requirements.network = Some(Sourced::new( + NetworkConstraints { + enabled: Some(true), + allow_local_binding: Some(true), + ..Default::default() + }, + RequirementSource::CloudRequirements, + )); + let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); + requirements_toml.network = Some(NetworkRequirementsToml { + enabled: Some(true), + allow_local_binding: Some(true), + ..Default::default() + }); + config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml) + .expect("rebuild config layer stack with network requirements"); + }); + let test = builder.build(&server).await?; + assert!( + test.config.managed_network_requirements_enabled(), + "expected managed network requirements to be enabled" + ); + assert!( + test.config.permissions.network.is_some(), + "expected managed network proxy config to be present" + ); + let runtime_proxy = test + .session_configured + .network_proxy + .as_ref() + .expect("expected runtime managed network proxy addresses"); + let proxy_addr = runtime_proxy.http_addr.as_str(); + + let call_id_first = "allow-network-first"; + // Use the same urllib-based pattern as the other network integration tests, + // but point it at the runtime proxy directly so the blocked host reliably + // produces a network approval request without relying on curl. + let fetch_command = format!( + "python3 -c \"import urllib.request; proxy = urllib.request.ProxyHandler({{'http': 'http://{proxy_addr}'}}); opener = urllib.request.build_opener(proxy); print('OK:' + opener.open('http://codex-network-test.invalid', timeout=30).read().decode(errors='replace'))\"" + ); + let first_event = shell_event( + call_id_first, + &fetch_command, + 30_000, + SandboxPermissions::UseDefault, + )?; + + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-allow-network-1"), + first_event, + ev_completed("resp-allow-network-1"), + ]), + ) + .await; + let first_results = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-allow-network-1", "done"), + ev_completed("resp-allow-network-2"), + ]), + ) + .await; + + submit_turn( + &test, + "allow-network-first", + approval_policy, + sandbox_policy.clone(), + ) + .await?; + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + let approval = loop { + let remaining = deadline + .checked_duration_since(std::time::Instant::now()) + .expect("timed out waiting for network approval request"); + let event = wait_for_event_with_timeout( + &test.codex, + |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }, + remaining, + ) + .await; + match event { + EventMsg::ExecApprovalRequest(approval) => { + if approval.command.first().map(std::string::String::as_str) + == Some("network-access") + { + break approval; + } + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + } + EventMsg::TurnComplete(_) => { + panic!("expected network approval request before completion"); + } + other => panic!("unexpected event: {other:?}"), + } + }; + let network_context = approval + .network_approval_context + .clone() + .expect("expected network approval context"); + assert_eq!(network_context.protocol, NetworkApprovalProtocol::Http); + let expected_network_amendments = vec![ + NetworkPolicyAmendment { + host: network_context.host.clone(), + action: NetworkPolicyRuleAction::Allow, + }, + NetworkPolicyAmendment { + host: network_context.host.clone(), + action: NetworkPolicyRuleAction::Deny, + }, + ]; + assert_eq!( + approval.proposed_network_policy_amendments, + Some(expected_network_amendments.clone()) + ); + let deny_network_amendment = expected_network_amendments + .into_iter() + .find(|amendment| amendment.action == NetworkPolicyRuleAction::Deny) + .expect("expected deny network policy amendment"); + + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment: deny_network_amendment.clone(), + }, + }) + .await?; + wait_for_completion(&test).await; + + let policy_path = test.home.path().join("rules").join("default.rules"); + let policy_contents = fs::read_to_string(&policy_path)?; + let expected_rule = format!( + r#"network_rule(host="{}", protocol="{}", decision="deny", justification="Deny {} access to {}")"#, + deny_network_amendment.host, + match network_context.protocol { + NetworkApprovalProtocol::Http => "http", + NetworkApprovalProtocol::Https => "https_connect", + NetworkApprovalProtocol::Socks5Tcp => "socks5_tcp", + NetworkApprovalProtocol::Socks5Udp => "socks5_udp", + }, + match network_context.protocol { + NetworkApprovalProtocol::Http => "http", + NetworkApprovalProtocol::Https => "https_connect", + NetworkApprovalProtocol::Socks5Tcp => "socks5_tcp", + NetworkApprovalProtocol::Socks5Udp => "socks5_udp", + }, + deny_network_amendment.host + ); + assert!( + policy_contents.contains(&expected_rule), + "unexpected policy contents: {policy_contents}" + ); + + let first_output = parse_result( + &first_results + .single_request() + .function_call_output(call_id_first), + ); + Expectation::CommandFailure { + output_contains: "", + } + .verify(&test, &first_output)?; + + let call_id_second = "allow-network-second"; + let second_event = shell_event( + call_id_second, + &fetch_command, + 30_000, + SandboxPermissions::UseDefault, + )?; + + let _ = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-allow-network-3"), + second_event, + ev_completed("resp-allow-network-3"), + ]), + ) + .await; + let second_results = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-allow-network-2", "done"), + ev_completed("resp-allow-network-4"), + ]), + ) + .await; + + submit_turn( + &test, + "allow-network-second", + approval_policy, + sandbox_policy.clone(), + ) + .await?; + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + let remaining = deadline + .checked_duration_since(std::time::Instant::now()) + .expect("timed out waiting for second turn completion"); + let event = wait_for_event_with_timeout( + &test.codex, + |event| { + matches!( + event, + EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_) + ) + }, + remaining, + ) + .await; + match event { + EventMsg::ExecApprovalRequest(approval) => { + if approval.command.first().map(std::string::String::as_str) + == Some("network-access") + { + panic!( + "unexpected network approval request: {:?}", + approval.command + ); + } + test.codex + .submit(Op::ExecApproval { + id: approval.effective_approval_id(), + turn_id: None, + decision: ReviewDecision::Approved, + }) + .await?; + } + EventMsg::TurnComplete(_) => break, + other => panic!("unexpected event: {other:?}"), + } + } + + let second_output = parse_result( + &second_results + .single_request() + .function_call_output(call_id_second), + ); + Expectation::CommandFailure { + output_contains: "", + } + .verify(&test, &second_output)?; + + Ok(()) +} + // todo(dylan) add ScenarioSpec support for rules #[tokio::test(flavor = "current_thread")] #[cfg(unix)] diff --git a/codex-rs/execpolicy/src/amend.rs b/codex-rs/execpolicy/src/amend.rs index 9809a46fe2e..2a6115c2fac 100644 --- a/codex-rs/execpolicy/src/amend.rs +++ b/codex-rs/execpolicy/src/amend.rs @@ -6,6 +6,9 @@ use std::io::Write; use std::path::Path; use std::path::PathBuf; +use crate::decision::Decision; +use crate::rule::NetworkRuleProtocol; +use crate::rule::normalize_network_rule_host; use serde_json; use thiserror::Error; @@ -13,6 +16,8 @@ use thiserror::Error; pub enum AmendError { #[error("prefix rule requires at least one token")] EmptyPrefix, + #[error("invalid network rule: {0}")] + InvalidNetworkRule(String), #[error("policy path has no parent: {path}")] MissingParent { path: PathBuf }, #[error("failed to create policy directory {dir}: {source}")] @@ -22,6 +27,8 @@ pub enum AmendError { }, #[error("failed to format prefix tokens: {source}")] SerializePrefix { source: serde_json::Error }, + #[error("failed to serialize network rule field: {source}")] + SerializeNetworkRule { source: serde_json::Error }, #[error("failed to open policy file {path}: {source}")] OpenPolicyFile { path: PathBuf, @@ -71,7 +78,54 @@ pub fn blocking_append_allow_prefix_rule( .map_err(|source| AmendError::SerializePrefix { source })?; let pattern = format!("[{}]", tokens.join(", ")); let rule = format!(r#"prefix_rule(pattern={pattern}, decision="allow")"#); + append_rule_line(policy_path, &rule) +} + +/// Note this function uses advisory file locking and performs blocking I/O, so it should be used +/// with [`tokio::task::spawn_blocking`] when called from an async context. +pub fn blocking_append_network_rule( + policy_path: &Path, + host: &str, + protocol: NetworkRuleProtocol, + decision: Decision, + justification: Option<&str>, +) -> Result<(), AmendError> { + let host = normalize_network_rule_host(host) + .map_err(|err| AmendError::InvalidNetworkRule(err.to_string()))?; + if let Some(raw) = justification + && raw.trim().is_empty() + { + return Err(AmendError::InvalidNetworkRule( + "justification cannot be empty".to_string(), + )); + } + + let host = serde_json::to_string(&host) + .map_err(|source| AmendError::SerializeNetworkRule { source })?; + let protocol = serde_json::to_string(protocol.as_policy_string()) + .map_err(|source| AmendError::SerializeNetworkRule { source })?; + let decision = serde_json::to_string(match decision { + Decision::Allow => "allow", + Decision::Prompt => "prompt", + Decision::Forbidden => "deny", + }) + .map_err(|source| AmendError::SerializeNetworkRule { source })?; + let mut args = vec![ + format!("host={host}"), + format!("protocol={protocol}"), + format!("decision={decision}"), + ]; + if let Some(justification) = justification { + let justification = serde_json::to_string(justification) + .map_err(|source| AmendError::SerializeNetworkRule { source })?; + args.push(format!("justification={justification}")); + } + let rule = format!("network_rule({})", args.join(", ")); + append_rule_line(policy_path, &rule) +} + +fn append_rule_line(policy_path: &Path, rule: &str) -> Result<(), AmendError> { let dir = policy_path .parent() .ok_or_else(|| AmendError::MissingParent { @@ -87,7 +141,8 @@ pub fn blocking_append_allow_prefix_rule( }); } } - append_locked_line(policy_path, &rule) + + append_locked_line(policy_path, rule) } fn append_locked_line(policy_path: &Path, line: &str) -> Result<(), AmendError> { @@ -215,4 +270,69 @@ prefix_rule(pattern=["echo", "Hello, world!"], decision="allow") "# ); } + + #[test] + fn appends_network_rule() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + + blocking_append_network_rule( + &policy_path, + "Api.GitHub.com", + NetworkRuleProtocol::Https, + Decision::Allow, + Some("Allow https_connect access to api.github.com"), + ) + .expect("append network rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + r#"network_rule(host="api.github.com", protocol="https", decision="allow", justification="Allow https_connect access to api.github.com") +"# + ); + } + + #[test] + fn appends_prefix_and_network_rules() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + + blocking_append_allow_prefix_rule(&policy_path, &[String::from("curl")]) + .expect("append prefix rule"); + blocking_append_network_rule( + &policy_path, + "api.github.com", + NetworkRuleProtocol::Https, + Decision::Allow, + Some("Allow https_connect access to api.github.com"), + ) + .expect("append network rule"); + + let contents = std::fs::read_to_string(&policy_path).expect("read policy"); + assert_eq!( + contents, + r#"prefix_rule(pattern=["curl"], decision="allow") +network_rule(host="api.github.com", protocol="https", decision="allow", justification="Allow https_connect access to api.github.com") +"# + ); + } + + #[test] + fn rejects_wildcard_network_rule_host() { + let tmp = tempdir().expect("create temp dir"); + let policy_path = tmp.path().join("rules").join("default.rules"); + let err = blocking_append_network_rule( + &policy_path, + "*.example.com", + NetworkRuleProtocol::Https, + Decision::Allow, + None, + ) + .expect_err("wildcards should be rejected"); + assert_eq!( + err.to_string(), + "invalid network rule: invalid rule: network_rule host must be a specific host; wildcards are not allowed" + ); + } } diff --git a/codex-rs/execpolicy/src/lib.rs b/codex-rs/execpolicy/src/lib.rs index a9453f28d3d..a9e9faa56de 100644 --- a/codex-rs/execpolicy/src/lib.rs +++ b/codex-rs/execpolicy/src/lib.rs @@ -8,6 +8,7 @@ pub mod rule; pub use amend::AmendError; pub use amend::blocking_append_allow_prefix_rule; +pub use amend::blocking_append_network_rule; pub use decision::Decision; pub use error::Error; pub use error::ErrorLocation; @@ -18,6 +19,7 @@ pub use execpolicycheck::ExecPolicyCheckCommand; pub use parser::PolicyParser; pub use policy::Evaluation; pub use policy::Policy; +pub use rule::NetworkRuleProtocol; pub use rule::Rule; pub use rule::RuleMatch; pub use rule::RuleRef; diff --git a/codex-rs/execpolicy/src/parser.rs b/codex-rs/execpolicy/src/parser.rs index 0ff0f4b34a7..8490c4848c7 100644 --- a/codex-rs/execpolicy/src/parser.rs +++ b/codex-rs/execpolicy/src/parser.rs @@ -18,6 +18,8 @@ use std::sync::Arc; use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use crate::rule::NetworkRule; +use crate::rule::NetworkRuleProtocol; use crate::rule::PatternToken; use crate::rule::PrefixPattern; use crate::rule::PrefixRule; @@ -71,12 +73,14 @@ impl PolicyParser { #[derive(Debug, ProvidesStaticType)] struct PolicyBuilder { rules_by_program: MultiMap, + network_rules: Vec, } impl PolicyBuilder { fn new() -> Self { Self { rules_by_program: MultiMap::new(), + network_rules: Vec::new(), } } @@ -85,8 +89,12 @@ impl PolicyBuilder { .insert(rule.program().to_string(), rule); } + fn add_network_rule(&mut self, rule: NetworkRule) { + self.network_rules.push(rule); + } + fn build(self) -> crate::policy::Policy { - crate::policy::Policy::new(self.rules_by_program) + crate::policy::Policy::from_parts(self.rules_by_program, self.network_rules) } } @@ -142,6 +150,13 @@ fn parse_examples<'v>(examples: UnpackList>) -> Result examples.items.into_iter().map(parse_example).collect() } +fn parse_network_rule_decision(raw: &str) -> Result { + match raw { + "deny" => Ok(Decision::Forbidden), + other => Decision::parse(other), + } +} + fn parse_example<'v>(value: Value<'v>) -> Result> { if let Some(raw) = value.unpack_str() { parse_string_example(raw) @@ -266,4 +281,31 @@ fn policy_builtins(builder: &mut GlobalsBuilder) { rules.into_iter().for_each(|rule| builder.add_rule(rule)); Ok(NoneType) } + + fn network_rule<'v>( + host: &'v str, + protocol: &'v str, + decision: &'v str, + justification: Option<&'v str>, + eval: &mut Evaluator<'v, '_, '_>, + ) -> anyhow::Result { + let protocol = NetworkRuleProtocol::parse(protocol)?; + let decision = parse_network_rule_decision(decision)?; + let justification = match justification { + Some(raw) if raw.trim().is_empty() => { + return Err(Error::InvalidRule("justification cannot be empty".to_string()).into()); + } + Some(raw) => Some(raw.to_string()), + None => None, + }; + + let mut builder = policy_builder(eval); + builder.add_network_rule(NetworkRule { + host: crate::rule::normalize_network_rule_host(host)?, + protocol, + decision, + justification, + }); + Ok(NoneType) + } } diff --git a/codex-rs/execpolicy/src/policy.rs b/codex-rs/execpolicy/src/policy.rs index 0da0332d0b8..faa74cc517c 100644 --- a/codex-rs/execpolicy/src/policy.rs +++ b/codex-rs/execpolicy/src/policy.rs @@ -1,11 +1,14 @@ use crate::decision::Decision; use crate::error::Error; use crate::error::Result; +use crate::rule::NetworkRule; +use crate::rule::NetworkRuleProtocol; use crate::rule::PatternToken; use crate::rule::PrefixPattern; use crate::rule::PrefixRule; use crate::rule::RuleMatch; use crate::rule::RuleRef; +use crate::rule::normalize_network_rule_host; use multimap::MultiMap; use serde::Deserialize; use serde::Serialize; @@ -16,11 +19,22 @@ type HeuristicsFallback<'a> = Option<&'a dyn Fn(&[String]) -> Decision>; #[derive(Clone, Debug)] pub struct Policy { rules_by_program: MultiMap, + network_rules: Vec, } impl Policy { pub fn new(rules_by_program: MultiMap) -> Self { - Self { rules_by_program } + Self::from_parts(rules_by_program, Vec::new()) + } + + pub fn from_parts( + rules_by_program: MultiMap, + network_rules: Vec, + ) -> Self { + Self { + rules_by_program, + network_rules, + } } pub fn empty() -> Self { @@ -31,6 +45,10 @@ impl Policy { &self.rules_by_program } + pub fn network_rules(&self) -> &[NetworkRule] { + &self.network_rules + } + pub fn get_allowed_prefixes(&self) -> Vec> { let mut prefixes = Vec::new(); @@ -77,6 +95,51 @@ impl Policy { Ok(()) } + pub fn add_network_rule( + &mut self, + host: &str, + protocol: NetworkRuleProtocol, + decision: Decision, + justification: Option, + ) -> Result<()> { + let host = normalize_network_rule_host(host)?; + if let Some(raw) = justification.as_deref() + && raw.trim().is_empty() + { + return Err(Error::InvalidRule( + "justification cannot be empty".to_string(), + )); + } + self.network_rules.push(NetworkRule { + host, + protocol, + decision, + justification, + }); + Ok(()) + } + + pub fn compiled_network_domains(&self) -> (Vec, Vec) { + let mut allowed = Vec::new(); + let mut denied = Vec::new(); + + for rule in &self.network_rules { + match rule.decision { + Decision::Allow => { + denied.retain(|entry| entry != &rule.host); + upsert_domain(&mut allowed, &rule.host); + } + Decision::Forbidden => { + allowed.retain(|entry| entry != &rule.host); + upsert_domain(&mut denied, &rule.host); + } + Decision::Prompt => {} + } + } + + (allowed, denied) + } + pub fn check(&self, cmd: &[String], heuristics_fallback: &F) -> Evaluation where F: Fn(&[String]) -> Decision, @@ -140,6 +203,11 @@ impl Policy { } } +fn upsert_domain(entries: &mut Vec, host: &str) { + entries.retain(|entry| entry != host); + entries.push(host.to_string()); +} + fn render_pattern_token(token: &PatternToken) -> String { match token { PatternToken::Single(value) => value.clone(), diff --git a/codex-rs/execpolicy/src/rule.rs b/codex-rs/execpolicy/src/rule.rs index b7c1a7cfde2..0d8bfc243a9 100644 --- a/codex-rs/execpolicy/src/rule.rs +++ b/codex-rs/execpolicy/src/rule.rs @@ -92,6 +92,103 @@ pub struct PrefixRule { pub justification: Option, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum NetworkRuleProtocol { + Http, + Https, + Socks5Tcp, + Socks5Udp, +} + +impl NetworkRuleProtocol { + pub fn parse(raw: &str) -> Result { + match raw { + "http" => Ok(Self::Http), + "https" | "https_connect" | "http-connect" => Ok(Self::Https), + "socks5_tcp" => Ok(Self::Socks5Tcp), + "socks5_udp" => Ok(Self::Socks5Udp), + other => Err(Error::InvalidRule(format!( + "network_rule protocol must be one of http, https, socks5_tcp, socks5_udp (got {other})" + ))), + } + } + + pub fn as_policy_string(self) -> &'static str { + match self { + Self::Http => "http", + Self::Https => "https", + Self::Socks5Tcp => "socks5_tcp", + Self::Socks5Udp => "socks5_udp", + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct NetworkRule { + pub host: String, + pub protocol: NetworkRuleProtocol, + pub decision: Decision, + pub justification: Option, +} + +pub(crate) fn normalize_network_rule_host(raw: &str) -> Result { + let mut host = raw.trim(); + if host.is_empty() { + return Err(Error::InvalidRule( + "network_rule host cannot be empty".to_string(), + )); + } + if host.contains("://") || host.contains('/') || host.contains('?') || host.contains('#') { + return Err(Error::InvalidRule( + "network_rule host must be a hostname or IP literal (without scheme or path)" + .to_string(), + )); + } + + if let Some(stripped) = host.strip_prefix('[') { + let Some((inside, rest)) = stripped.split_once(']') else { + return Err(Error::InvalidRule( + "network_rule host has an invalid bracketed IPv6 literal".to_string(), + )); + }; + let port_ok = rest + .strip_prefix(':') + .is_some_and(|port| !port.is_empty() && port.chars().all(|c| c.is_ascii_digit())); + if !rest.is_empty() && !port_ok { + return Err(Error::InvalidRule(format!( + "network_rule host contains an unsupported suffix: {raw}" + ))); + } + host = inside; + } else if host.matches(':').count() == 1 + && let Some((candidate, port)) = host.rsplit_once(':') + && !candidate.is_empty() + && !port.is_empty() + && port.chars().all(|c| c.is_ascii_digit()) + { + host = candidate; + } + + let normalized = host.trim_end_matches('.').trim().to_ascii_lowercase(); + if normalized.is_empty() { + return Err(Error::InvalidRule( + "network_rule host cannot be empty".to_string(), + )); + } + if normalized.contains('*') { + return Err(Error::InvalidRule( + "network_rule host must be a specific host; wildcards are not allowed".to_string(), + )); + } + if normalized.chars().any(char::is_whitespace) { + return Err(Error::InvalidRule( + "network_rule host cannot contain whitespace".to_string(), + )); + } + + Ok(normalized) +} + pub trait Rule: Any + Debug + Send + Sync { fn program(&self) -> &str; diff --git a/codex-rs/execpolicy/tests/basic.rs b/codex-rs/execpolicy/tests/basic.rs index 040509f115e..eec7cec651a 100644 --- a/codex-rs/execpolicy/tests/basic.rs +++ b/codex-rs/execpolicy/tests/basic.rs @@ -7,6 +7,7 @@ use anyhow::Result; use codex_execpolicy::Decision; use codex_execpolicy::Error; use codex_execpolicy::Evaluation; +use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use codex_execpolicy::PolicyParser; use codex_execpolicy::RuleMatch; @@ -67,6 +68,45 @@ fn append_allow_prefix_rule_dedupes_existing_rule() -> Result<()> { Ok(()) } +#[test] +fn network_rules_compile_into_domain_lists() -> Result<()> { + let policy_src = r#" +network_rule(host = "google.com", protocol = "http", decision = "allow") +network_rule(host = "api.github.com", protocol = "https", decision = "allow") +network_rule(host = "blocked.example.com", protocol = "https", decision = "deny") +network_rule(host = "prompt-only.example.com", protocol = "https", decision = "prompt") + "#; + let mut parser = PolicyParser::new(); + parser.parse("network.rules", policy_src)?; + let policy = parser.build(); + + assert_eq!(policy.network_rules().len(), 4); + assert_eq!( + policy.network_rules()[1].protocol, + NetworkRuleProtocol::Https + ); + + let (allowed, denied) = policy.compiled_network_domains(); + assert_eq!( + allowed, + vec!["google.com".to_string(), "api.github.com".to_string()] + ); + assert_eq!(denied, vec!["blocked.example.com".to_string()]); + Ok(()) +} + +#[test] +fn network_rule_rejects_wildcard_hosts() { + let mut parser = PolicyParser::new(); + let err = parser + .parse( + "network.rules", + r#"network_rule(host="*", protocol="http", decision="allow")"#, + ) + .expect_err("wildcard network_rule host should fail"); + assert!(err.to_string().contains("wildcards are not allowed")); +} + #[test] fn basic_match() -> Result<()> { let policy_src = r#" diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index b720a1b258b..f1dedb11cc1 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -223,6 +223,7 @@ async fn run_codex_tool_session_inner( approval_id: _, reason: _, proposed_execpolicy_amendment: _, + proposed_network_policy_amendments: _, parsed_cmd, network_approval_context: _, } = ev; diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index f0cb3ae471c..d1cb3312602 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -23,6 +23,7 @@ pub use network_policy::NetworkPolicyDecision; pub use network_policy::NetworkPolicyRequest; pub use network_policy::NetworkPolicyRequestArgs; pub use network_policy::NetworkProtocol; +pub use policy::normalize_host; pub use proxy::ALL_PROXY_ENV_KEYS; pub use proxy::ALLOW_LOCAL_BINDING_ENV_KEY; pub use proxy::Args; diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index f30006958fa..0e4a246fe8d 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -425,6 +425,14 @@ impl NetworkProxy { self.admin_addr } + pub async fn add_allowed_domain(&self, host: &str) -> Result<()> { + self.state.add_allowed_domain(host).await + } + + pub async fn add_denied_domain(&self, host: &str) -> Result<()> { + self.state.add_denied_domain(host).await + } + pub fn allow_local_binding(&self) -> bool { self.allow_local_binding } diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 583ecd7eda0..8f8daadc252 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -10,7 +10,6 @@ use crate::reasons::REASON_NOT_ALLOWED; use crate::reasons::REASON_NOT_ALLOWED_LOCAL; use crate::state::NetworkProxyConstraintError; use crate::state::NetworkProxyConstraints; -#[cfg(test)] use crate::state::build_config_state; use crate::state::validate_policy_against_constraints; use anyhow::Context; @@ -500,6 +499,70 @@ impl NetworkProxyState { } } + pub async fn add_allowed_domain(&self, host: &str) -> Result<()> { + self.update_domain_list(host, DomainListKind::Allow).await + } + + pub async fn add_denied_domain(&self, host: &str) -> Result<()> { + self.update_domain_list(host, DomainListKind::Deny).await + } + + async fn update_domain_list(&self, host: &str, target: DomainListKind) -> Result<()> { + let host = Host::parse(host).context("invalid network host")?; + let normalized_host = host.as_str().to_string(); + let list_name = target.list_name(); + let constraint_field = target.constraint_field(); + + loop { + self.reload_if_needed().await?; + let (previous_cfg, constraints, blocked, blocked_total) = { + let guard = self.state.read().await; + ( + guard.config.clone(), + guard.constraints.clone(), + guard.blocked.clone(), + guard.blocked_total, + ) + }; + + let mut candidate = previous_cfg.clone(); + let (target_entries, opposite_entries) = candidate.split_domain_lists_mut(target); + let target_contains = target_entries + .iter() + .any(|entry| normalize_host(entry) == normalized_host); + let opposite_contains = opposite_entries + .iter() + .any(|entry| normalize_host(entry) == normalized_host); + if target_contains && !opposite_contains { + return Ok(()); + } + + target_entries.retain(|entry| normalize_host(entry) != normalized_host); + target_entries.push(normalized_host.clone()); + opposite_entries.retain(|entry| normalize_host(entry) != normalized_host); + + validate_policy_against_constraints(&candidate, &constraints) + .map_err(NetworkProxyConstraintError::into_anyhow) + .with_context(|| format!("{constraint_field} constrained by managed config"))?; + + let mut new_state = build_config_state(candidate.clone(), constraints.clone()) + .with_context(|| format!("failed to compile updated network {list_name}"))?; + new_state.blocked = blocked; + new_state.blocked_total = blocked_total; + + let mut guard = self.state.write().await; + if guard.constraints != constraints || guard.config != previous_cfg { + drop(guard); + continue; + } + + log_policy_changes(&guard.config, &candidate); + *guard = new_state; + info!("updated network {list_name} with {normalized_host}"); + return Ok(()); + } + } + async fn reload_if_needed(&self) -> Result<()> { match self.reloader.maybe_reload().await? { None => Ok(()), @@ -527,6 +590,46 @@ impl NetworkProxyState { } } +#[derive(Clone, Copy)] +enum DomainListKind { + Allow, + Deny, +} + +impl DomainListKind { + fn list_name(self) -> &'static str { + match self { + Self::Allow => "allowlist", + Self::Deny => "denylist", + } + } + + fn constraint_field(self) -> &'static str { + match self { + Self::Allow => "network.allowed_domains", + Self::Deny => "network.denied_domains", + } + } +} + +impl NetworkProxyConfig { + fn split_domain_lists_mut( + &mut self, + target: DomainListKind, + ) -> (&mut Vec, &mut Vec) { + match target { + DomainListKind::Allow => ( + &mut self.network.allowed_domains, + &mut self.network.denied_domains, + ), + DomainListKind::Deny => ( + &mut self.network.denied_domains, + &mut self.network.allowed_domains, + ), + } + } +} + pub(crate) fn unix_socket_permissions_supported() -> bool { cfg!(target_os = "macos") } @@ -695,6 +798,42 @@ mod tests { ); } + #[tokio::test] + async fn add_allowed_domain_removes_matching_deny_entry() { + let state = network_proxy_state_for_policy(NetworkProxySettings { + denied_domains: vec!["example.com".to_string()], + ..NetworkProxySettings::default() + }); + + state.add_allowed_domain("ExAmPlE.CoM").await.unwrap(); + + let (allowed, denied) = state.current_patterns().await.unwrap(); + assert_eq!(allowed, vec!["example.com".to_string()]); + assert!(denied.is_empty()); + assert_eq!( + state.host_blocked("example.com", 80).await.unwrap(), + HostBlockDecision::Allowed + ); + } + + #[tokio::test] + async fn add_denied_domain_removes_matching_allow_entry() { + let state = network_proxy_state_for_policy(NetworkProxySettings { + allowed_domains: vec!["example.com".to_string()], + ..NetworkProxySettings::default() + }); + + state.add_denied_domain("EXAMPLE.COM").await.unwrap(); + + let (allowed, denied) = state.current_patterns().await.unwrap(); + assert!(allowed.is_empty()); + assert_eq!(denied, vec!["example.com".to_string()]); + assert_eq!( + state.host_blocked("example.com", 80).await.unwrap(), + HostBlockDecision::Blocked(HostBlockReason::Denied) + ); + } + #[tokio::test] async fn blocked_snapshot_does_not_consume_entries() { 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 f275ca66d76..58b9231a77d 100644 --- a/codex-rs/protocol/src/approvals.rs +++ b/codex-rs/protocol/src/approvals.rs @@ -55,6 +55,19 @@ pub struct NetworkApprovalContext { pub protocol: NetworkApprovalProtocol, } +#[derive(Debug, Clone, Copy, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "snake_case")] +pub enum NetworkPolicyRuleAction { + Allow, + Deny, +} + +#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)] +pub struct NetworkPolicyAmendment { + pub host: String, + pub action: NetworkPolicyRuleAction, +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)] pub struct ExecApprovalRequestEvent { /// Identifier for the associated command execution item. @@ -85,6 +98,10 @@ pub struct ExecApprovalRequestEvent { #[serde(default, skip_serializing_if = "Option::is_none")] #[ts(optional)] pub proposed_execpolicy_amendment: Option, + /// Proposed network policy amendments (for example allow/deny this host in future). + #[serde(default, skip_serializing_if = "Option::is_none")] + #[ts(optional)] + pub proposed_network_policy_amendments: Option>, pub parsed_cmd: Vec, } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index de7c16e6777..da49fe3843e 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -57,6 +57,8 @@ pub use crate::approvals::ExecApprovalRequestEvent; pub use crate::approvals::ExecPolicyAmendment; pub use crate::approvals::NetworkApprovalContext; pub use crate::approvals::NetworkApprovalProtocol; +pub use crate::approvals::NetworkPolicyAmendment; +pub use crate::approvals::NetworkPolicyRuleAction; pub use crate::request_user_input::RequestUserInputEvent; /// Open/close tags for special user-input blocks. Used across crates to avoid @@ -2756,6 +2758,12 @@ pub enum ReviewDecision { /// remainder of the session. ApprovedForSession, + /// User chose to persist a network policy rule (allow/deny) for future + /// requests to the same host. + NetworkPolicyAmendment { + network_policy_amendment: NetworkPolicyAmendment, + }, + /// User has denied this command and the agent should not execute it, but /// it should continue the session and try something else. #[default] @@ -2774,6 +2782,12 @@ impl ReviewDecision { ReviewDecision::Approved => "approved", ReviewDecision::ApprovedExecpolicyAmendment { .. } => "approved_with_amendment", ReviewDecision::ApprovedForSession => "approved_for_session", + ReviewDecision::NetworkPolicyAmendment { + network_policy_amendment, + } => match network_policy_amendment.action { + NetworkPolicyRuleAction::Allow => "approved_with_network_policy_allow", + NetworkPolicyRuleAction::Deny => "denied_with_network_policy_deny", + }, ReviewDecision::Denied => "denied", ReviewDecision::Abort => "abort", } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index cc10484c35d..1c01b680866 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2775,6 +2775,7 @@ async fn exec_approval_emits_proposed_command_and_decision_history() { ), network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2821,6 +2822,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { ), network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -2873,6 +2875,7 @@ async fn exec_approval_decision_truncates_multiline_and_long_commands() { reason: None, network_approval_context: None, proposed_execpolicy_amendment: None, + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6470,6 +6473,7 @@ async fn approval_modal_exec_snapshot() -> anyhow::Result<()> { "hello".into(), "world".into(), ])), + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6528,6 +6532,7 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { "hello".into(), "world".into(), ])), + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6573,6 +6578,7 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() reason: None, network_approval_context: None, proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { @@ -6937,6 +6943,7 @@ async fn status_widget_and_approval_modal_snapshot() { "echo".into(), "hello world".into(), ])), + proposed_network_policy_amendments: None, parsed_cmd: vec![], }; chat.handle_codex_event(Event { diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 5ec09c25ff4..5faa1ed9ad0 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -828,6 +828,31 @@ pub fn new_approval_decision_cell( ], ) } + NetworkPolicyAmendment { + network_policy_amendment, + } => { + let host = Span::from(network_policy_amendment.host).dim(); + match network_policy_amendment.action { + codex_protocol::protocol::NetworkPolicyRuleAction::Allow => ( + "✔ ".green(), + vec![ + "You ".into(), + "approved".bold(), + " future network access to ".into(), + host, + ], + ), + codex_protocol::protocol::NetworkPolicyRuleAction::Deny => ( + "✗ ".red(), + vec![ + "You ".into(), + "blocked".bold(), + " future network access to ".into(), + host, + ], + ), + } + } Denied => { let snippet = Span::from(exec_snippet(&command)).dim(); (