diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4ae2d33c4b..395a8ae04d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2600,6 +2600,7 @@ dependencies = [ "codex-app-server-protocol", "codex-apply-patch", "codex-async-utils", + "codex-backend-client", "codex-code-mode", "codex-config", "codex-connectors", diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index 52275eb4be..9cbd6570c2 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -1,6 +1,8 @@ use crate::types::AccountsCheckResponse; use crate::types::CodeTaskDetailsResponse; use crate::types::ConfigBundleResponse; +use crate::types::DeterministicGuardianReviewRequest; +use crate::types::DeterministicGuardianReviewResponse; use crate::types::PaginatedListTaskListItem; use crate::types::RateLimitReachedKind as BackendRateLimitReachedKind; use crate::types::RateLimitStatusPayload; @@ -314,6 +316,31 @@ impl Client { self.decode_json(&url, &ct, &body) } + pub async fn deterministic_guardian_review_network_access( + &self, + url: &str, + method: &str, + ) -> Result { + let endpoint_url = match self.path_style { + PathStyle::CodexApi => { + format!("{}/api/codex/guardian/deterministic-review", self.base_url) + } + PathStyle::ChatGptApi => { + format!("{}/wham/guardian/deterministic-review", self.base_url) + } + }; + let req = self + .http + .post(&endpoint_url) + .headers(self.headers()) + .header(CONTENT_TYPE, HeaderValue::from_static("application/json")) + .json(&DeterministicGuardianReviewRequest::network_access( + url, method, + )); + let (body, ct) = self.exec_request(req, "POST", &endpoint_url).await?; + self.decode_json(&endpoint_url, &ct, &body) + } + pub async fn get_token_usage_profile(&self) -> Result { let url = self.token_usage_profile_url(); let req = self.http.get(&url).headers(self.headers()); diff --git a/codex-rs/backend-client/src/lib.rs b/codex-rs/backend-client/src/lib.rs index e50fd8db40..1e6578b744 100644 --- a/codex-rs/backend-client/src/lib.rs +++ b/codex-rs/backend-client/src/lib.rs @@ -12,6 +12,7 @@ pub use types::ConfigBundleResponse; pub use types::DeliveredConfigToml; pub use types::DeliveredRequirementsToml; pub use types::DeliveredTomlFragment; +pub use types::DeterministicGuardianReviewResponse; pub use types::PaginatedListTaskListItem; pub use types::TaskListItem; pub use types::TokenUsageProfile; diff --git a/codex-rs/backend-client/src/types.rs b/codex-rs/backend-client/src/types.rs index 3ccacbc8c9..4fdf551d4b 100644 --- a/codex-rs/backend-client/src/types.rs +++ b/codex-rs/backend-client/src/types.rs @@ -13,6 +13,7 @@ pub use codex_backend_openapi_models::models::SpendControlLimitDetails; pub use codex_backend_openapi_models::models::TaskListItem; use serde::Deserialize; +use serde::Serialize; use serde::de::Deserializer; use serde_json::Value; use std::collections::HashMap; @@ -24,6 +25,32 @@ pub struct AccountsCheckResponse { pub default_account_id: Option, } +#[derive(Clone, Debug, Serialize)] +pub struct DeterministicGuardianReviewRequest<'a> { + pub action: DeterministicGuardianReviewAction<'a>, +} + +impl<'a> DeterministicGuardianReviewRequest<'a> { + pub fn network_access(url: &'a str, method: &'a str) -> Self { + Self { + action: DeterministicGuardianReviewAction::NetworkAccess { url, method }, + } + } +} + +#[derive(Clone, Debug, Serialize)] +#[serde(rename_all = "snake_case", tag = "type")] +pub enum DeterministicGuardianReviewAction<'a> { + NetworkAccess { url: &'a str, method: &'a str }, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct DeterministicGuardianReviewResponse { + pub safe: bool, + #[serde(default)] + pub rationale: Option, +} + #[derive(Clone, Debug, Deserialize)] pub struct AccountEntry { pub id: String, diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index d21245c63e..4a109b4c6b 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -28,6 +28,7 @@ codex-api = { workspace = true } codex-app-server-protocol = { workspace = true } codex-apply-patch = { workspace = true } codex-async-utils = { workspace = true } +codex-backend-client = { workspace = true } codex-code-mode = { workspace = true } codex-connectors = { workspace = true } codex-context-fragments = { workspace = true } diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index eddc9bc13e..ee1c82348f 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::GuardianAssessment; use crate::guardian::GuardianNetworkAccessTrigger; use crate::guardian::GuardianReviewMode; use crate::guardian::guardian_rejection_message; @@ -11,6 +12,7 @@ use crate::network_policy_decision::denied_network_policy_message; use crate::session::session::Session; use crate::tools::sandboxing::PermissionRequestPayload; use crate::tools::sandboxing::ToolError; +use codex_backend_client::Client as BackendClient; use codex_hooks::PermissionRequestDecision; use codex_network_proxy::BlockedRequest; use codex_network_proxy::BlockedRequestObserver; @@ -26,16 +28,22 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::GuardianAssessmentDecisionSource; +use codex_protocol::protocol::GuardianAssessmentOutcome; +use codex_protocol::protocol::GuardianRiskLevel; +use codex_protocol::protocol::GuardianUserAuthorization; use codex_protocol::protocol::ReviewDecision; use codex_protocol::protocol::WarningEvent; use indexmap::IndexMap; use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; +use std::time::Duration; use tokio::sync::Mutex; use tokio::sync::Notify; use tokio::sync::OnceCell; use tokio::sync::RwLock; +use tokio::time::timeout; use tokio_util::sync::CancellationToken; use tracing::warn; use uuid::Uuid; @@ -124,6 +132,7 @@ struct HostApprovalKey { host: String, protocol: &'static str, port: u16, + url_fingerprint: Option, } impl HostApprovalKey { @@ -132,8 +141,13 @@ impl HostApprovalKey { host: request.host.to_ascii_lowercase(), protocol: protocol_key_label(protocol), port: request.port, + url_fingerprint: request.url.as_deref().map(Self::url_fingerprint), } } + + fn url_fingerprint(url: &str) -> Uuid { + Uuid::new_v5(&Uuid::NAMESPACE_URL, url.as_bytes()) + } } fn protocol_key_label(protocol: NetworkApprovalProtocol) -> &'static str { @@ -253,6 +267,8 @@ impl Default for NetworkApprovalService { } impl NetworkApprovalService { + const GUARDIAN_DETERMINISTIC_REVIEW_TIMEOUT: Duration = Duration::from_secs(2); + /// Replace the target session's approval cache with the source session's /// currently approved hosts. pub(crate) async fn sync_session_approved_hosts_to(&self, other: &Self) { @@ -385,7 +401,68 @@ impl NetworkApprovalService { } fn approval_id_for_key(key: &HostApprovalKey) -> String { - format!("network#{}#{}#{}", key.protocol, key.host, key.port) + match key.url_fingerprint { + Some(url_fingerprint) => format!( + "network#{}#{}#{}#{}", + key.protocol, key.host, key.port, url_fingerprint + ), + None => format!("network#{}#{}#{}", key.protocol, key.host, key.port), + } + } + + async fn guardian_review_mode_for_network_access( + session: &Session, + turn_context: &crate::session::turn_context::TurnContext, + request: &NetworkPolicyRequest, + ) -> GuardianReviewMode { + let Some(url) = request.url.as_deref() else { + return GuardianReviewMode::Agent; + }; + let Some(method) = request.method.as_deref() else { + return GuardianReviewMode::Agent; + }; + let auth = session.services.auth_manager.auth().await; + let Some(auth) = auth.as_ref().filter(|auth| auth.uses_codex_backend()) else { + return GuardianReviewMode::Agent; + }; + let client = + match BackendClient::from_auth(turn_context.config.chatgpt_base_url.clone(), auth) { + Ok(client) => client, + Err(err) => { + warn!("failed to create deterministic Guardian review client: {err:#}"); + return GuardianReviewMode::Agent; + } + }; + let response = match timeout( + Self::GUARDIAN_DETERMINISTIC_REVIEW_TIMEOUT, + client.deterministic_guardian_review_network_access(url, method), + ) + .await + { + Ok(Ok(response)) => response, + Ok(Err(err)) => { + warn!("deterministic Guardian network review request failed: {err:#}"); + return GuardianReviewMode::Agent; + } + Err(_) => { + warn!("deterministic Guardian network review request timed out"); + return GuardianReviewMode::Agent; + } + }; + if !response.safe { + return GuardianReviewMode::Agent; + } + + GuardianReviewMode::Deterministic(GuardianAssessment { + risk_level: GuardianRiskLevel::Low, + user_authorization: GuardianUserAuthorization::High, + outcome: GuardianAssessmentOutcome::Allow, + rationale: response + .rationale + .filter(|rationale| !rationale.trim().is_empty()) + .unwrap_or_else(|| "Trusted deterministic Guardian review approved.".to_string()), + source: GuardianAssessmentDecisionSource::Deterministic, + }) } pub(crate) async fn handle_inline_policy_request( @@ -397,6 +474,7 @@ impl NetworkApprovalService { let protocol = match request.protocol { NetworkProtocol::Http => NetworkApprovalProtocol::Http, + NetworkProtocol::Https => NetworkApprovalProtocol::Https, NetworkProtocol::HttpsConnect => NetworkApprovalProtocol::Https, NetworkProtocol::Socks5Tcp => NetworkApprovalProtocol::Socks5Tcp, NetworkProtocol::Socks5Udp => NetworkApprovalProtocol::Socks5Udp, @@ -422,7 +500,12 @@ impl NetworkApprovalService { return pending.wait_for_decision().await.to_network_decision(); } - let target = Self::format_network_target(key.protocol, request.host.as_str(), key.port); + let network_target = + Self::format_network_target(key.protocol, request.host.as_str(), key.port); + let target = request + .url + .clone() + .unwrap_or_else(|| network_target.clone()); let policy_denial_message = format!("Network access to \"{target}\" was blocked by policy."); let prompt_reason = format!("{} is not in the allowed_domains", request.host); @@ -462,14 +545,18 @@ impl NetworkApprovalService { }; let guardian_approval_id = Self::approval_id_for_key(&key); let prompt_command = vec!["network-access".to_string(), target.clone()]; + let hook_command = ["network-access".to_string(), network_target.clone()]; let command = owner_call .as_ref() - .map_or_else(|| prompt_command.join(" "), |call| call.command.clone()); + .map_or_else(|| hook_command.join(" "), |call| call.command.clone()); if let Some(permission_request_decision) = run_permission_request_hooks( &session, &turn_context, &guardian_approval_id, - PermissionRequestPayload::bash(command, Some(format!("network-access {target}"))), + PermissionRequestPayload::bash( + command, + Some(format!("network-access {network_target}")), + ), ) .await { @@ -500,6 +587,9 @@ impl NetworkApprovalService { let use_guardian = routes_approval_to_guardian(&turn_context); let guardian_review_id = use_guardian.then(new_guardian_review_id); let approval_decision = if let Some(review_id) = guardian_review_id.clone() { + let guardian_review_mode = + Self::guardian_review_mode_for_network_access(&session, &turn_context, &request) + .await; review_approval_request( &session, &turn_context, @@ -515,7 +605,7 @@ impl NetworkApprovalService { port: key.port, trigger: owner_call.as_ref().map(|call| call.trigger.clone()), }, - GuardianReviewMode::Agent, + guardian_review_mode, Some(policy_denial_message.clone()), ) .await diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 25bcfe255d..1460d2a28c 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -1,13 +1,26 @@ use super::*; use crate::sandboxing::SandboxPermissions; +use crate::session::tests::make_session_and_context; +use crate::test_support::auth_manager_from_auth; +use codex_login::CodexAuth; use codex_network_proxy::BlockedRequestArgs; +use codex_network_proxy::NetworkPolicyRequestArgs; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; +use codex_protocol::protocol::GuardianAssessmentDecisionSource; +use codex_protocol::protocol::GuardianAssessmentOutcome; use core_test_support::PathBufExt; +use core_test_support::responses::start_mock_server; use core_test_support::test_path_buf; use pretty_assertions::assert_eq; +use std::sync::Arc; use tokio_util::sync::CancellationToken; +use wiremock::Mock; +use wiremock::ResponseTemplate; +use wiremock::matchers::body_json; +use wiremock::matchers::method; +use wiremock::matchers::path; #[tokio::test] async fn pending_approvals_are_deduped_per_host_protocol_and_port() { @@ -16,6 +29,7 @@ async fn pending_approvals_are_deduped_per_host_protocol_and_port() { host: "example.com".to_string(), protocol: "http", port: 443, + url_fingerprint: None, }; let (first, first_is_owner) = service.get_or_create_pending_approval(key.clone()).await; @@ -33,11 +47,13 @@ async fn pending_approvals_do_not_dedupe_across_ports() { host: "example.com".to_string(), protocol: "https", port: 443, + url_fingerprint: None, }; let second_key = HostApprovalKey { host: "example.com".to_string(), protocol: "https", port: 8443, + url_fingerprint: None, }; let (first, first_is_owner) = service.get_or_create_pending_approval(first_key).await; @@ -58,16 +74,19 @@ async fn session_approved_hosts_preserve_protocol_and_port_scope() { host: "example.com".to_string(), protocol: "https", port: 443, + url_fingerprint: None, }, HostApprovalKey { host: "example.com".to_string(), protocol: "https", port: 8443, + url_fingerprint: None, }, HostApprovalKey { host: "example.com".to_string(), protocol: "http", port: 80, + url_fingerprint: None, }, ]); } @@ -91,16 +110,19 @@ async fn session_approved_hosts_preserve_protocol_and_port_scope() { host: "example.com".to_string(), protocol: "http", port: 80, + url_fingerprint: None, }, HostApprovalKey { host: "example.com".to_string(), protocol: "https", port: 443, + url_fingerprint: None, }, HostApprovalKey { host: "example.com".to_string(), protocol: "https", port: 8443, + url_fingerprint: None, }, ] ); @@ -115,6 +137,7 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { host: "source.example.com".to_string(), protocol: "https", port: 443, + url_fingerprint: None, }); } @@ -125,6 +148,7 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { host: "stale.example.com".to_string(), protocol: "https", port: 8443, + url_fingerprint: None, }); } @@ -144,10 +168,93 @@ async fn sync_session_approved_hosts_to_replaces_existing_target_hosts() { host: "source.example.com".to_string(), protocol: "https", port: 443, + url_fingerprint: None, }] ); } +#[tokio::test] +async fn pending_approvals_do_not_dedupe_across_urls() { + let service = NetworkApprovalService::default(); + let first_key = HostApprovalKey { + host: "example.com".to_string(), + protocol: "https", + port: 443, + url_fingerprint: Some(HostApprovalKey::url_fingerprint("https://example.com/safe")), + }; + let second_key = HostApprovalKey { + host: "example.com".to_string(), + protocol: "https", + port: 443, + url_fingerprint: Some(HostApprovalKey::url_fingerprint( + "https://example.com/unsafe", + )), + }; + + let (first, first_is_owner) = service.get_or_create_pending_approval(first_key).await; + let (second, second_is_owner) = service.get_or_create_pending_approval(second_key).await; + + assert!(first_is_owner); + assert!(second_is_owner); + assert!(!Arc::ptr_eq(&first, &second)); +} + +#[tokio::test] +async fn deterministic_network_review_uses_safe_url_backend_result() { + let server = start_mock_server().await; + Mock::given(method("POST")) + .and(path("/api/codex/guardian/deterministic-review")) + .and(body_json(serde_json::json!({ + "action": { + "type": "network_access", + "url": "https://example.com/from-index", + "method": "GET", + } + }))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "safe": true, + "rationale": "safe url", + }))) + .expect(1) + .mount(&server) + .await; + + let (mut session, mut turn_context) = make_session_and_context().await; + let auth_manager = auth_manager_from_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + session.services.auth_manager = Arc::clone(&auth_manager); + turn_context.auth_manager = Some(auth_manager); + let mut config = (*turn_context.config).clone(); + config.chatgpt_base_url = server.uri(); + turn_context.config = Arc::new(config); + let request = NetworkPolicyRequest::new(NetworkPolicyRequestArgs { + protocol: NetworkProtocol::Https, + host: "example.com".to_string(), + port: 443, + client_addr: None, + method: Some("GET".to_string()), + url: Some("https://example.com/from-index".to_string()), + command: None, + exec_policy_hint: None, + }); + + let review_mode = NetworkApprovalService::guardian_review_mode_for_network_access( + &session, + &turn_context, + &request, + ) + .await; + + let GuardianReviewMode::Deterministic(assessment) = review_mode else { + panic!("safe URL should use deterministic Guardian review"); + }; + assert_eq!(assessment.outcome, GuardianAssessmentOutcome::Allow); + assert_eq!(assessment.rationale, "safe url"); + assert_eq!( + assessment.source, + GuardianAssessmentDecisionSource::Deterministic + ); +} + #[tokio::test] async fn pending_waiters_receive_owner_decision() { let pending = Arc::new(PendingHostApproval::new()); diff --git a/codex-rs/network-proxy/src/http_proxy.rs b/codex-rs/network-proxy/src/http_proxy.rs index f20c01b906..3b429d7c28 100644 --- a/codex-rs/network-proxy/src/http_proxy.rs +++ b/codex-rs/network-proxy/src/http_proxy.rs @@ -202,6 +202,7 @@ async fn http_connect_accept( port: authority.port, client_addr: client.clone(), method: Some("CONNECT".to_string()), + url: None, command: None, exec_policy_hint: None, }); @@ -685,6 +686,7 @@ async fn http_plain_proxy( port, client_addr: client.clone(), method: Some(req.method().as_str().to_string()), + url: Some(req.uri().to_string()), command: None, exec_policy_hint: None, }); diff --git a/codex-rs/network-proxy/src/network_policy.rs b/codex-rs/network-proxy/src/network_policy.rs index 18bca2c916..a24ddaa869 100644 --- a/codex-rs/network-proxy/src/network_policy.rs +++ b/codex-rs/network-proxy/src/network_policy.rs @@ -22,6 +22,7 @@ const DEFAULT_CLIENT_ADDRESS: &str = "unknown"; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum NetworkProtocol { Http, + Https, HttpsConnect, Socks5Tcp, Socks5Udp, @@ -31,6 +32,7 @@ impl NetworkProtocol { pub const fn as_policy_protocol(self) -> &'static str { match self { Self::Http => "http", + Self::Https => "https", Self::HttpsConnect => "https_connect", Self::Socks5Tcp => "socks5_tcp", Self::Socks5Udp => "socks5_udp", @@ -81,6 +83,7 @@ pub struct NetworkPolicyRequest { pub port: u16, pub client_addr: Option, pub method: Option, + pub url: Option, pub command: Option, pub exec_policy_hint: Option, } @@ -91,6 +94,7 @@ pub struct NetworkPolicyRequestArgs { pub port: u16, pub client_addr: Option, pub method: Option, + pub url: Option, pub command: Option, pub exec_policy_hint: Option, } @@ -103,6 +107,7 @@ impl NetworkPolicyRequest { port, client_addr, method, + url, command, exec_policy_hint, } = args; @@ -112,6 +117,7 @@ impl NetworkPolicyRequest { port, client_addr, method, + url, command, exec_policy_hint, } @@ -627,6 +633,7 @@ mod tests { port: 80, client_addr: None, method: None, + url: None, command: None, exec_policy_hint: None, }); @@ -688,6 +695,7 @@ mod tests { port: 80, client_addr: Some("127.0.0.1:1234".to_string()), method: Some("GET".to_string()), + url: None, command: None, exec_policy_hint: None, }); @@ -731,6 +739,7 @@ mod tests { port: 80, client_addr: None, method: Some("GET".to_string()), + url: None, command: None, exec_policy_hint: None, }); @@ -781,6 +790,7 @@ mod tests { port: 80, client_addr: None, method: Some("GET".to_string()), + url: None, command: None, exec_policy_hint: None, }); @@ -867,6 +877,7 @@ mod tests { port: 80, client_addr: None, method: Some("GET".to_string()), + url: None, command: None, exec_policy_hint: None, }); diff --git a/codex-rs/network-proxy/src/socks5.rs b/codex-rs/network-proxy/src/socks5.rs index 6e600d2025..d02cf817fa 100644 --- a/codex-rs/network-proxy/src/socks5.rs +++ b/codex-rs/network-proxy/src/socks5.rs @@ -270,6 +270,7 @@ async fn handle_socks5_tcp( port, client_addr: client.clone(), method: None, + url: None, command: None, exec_policy_hint: None, }); @@ -626,6 +627,7 @@ async fn inspect_socks5_udp( port, client_addr: client.clone(), method: None, + url: None, command: None, exec_policy_hint: None, });