Use deterministic Guardian review for safe network URLs

This commit is contained in:
Winston Howes
2026-06-12 14:40:49 -07:00
parent 90b9508177
commit 8944713d0a
10 changed files with 274 additions and 5 deletions
+1
View File
@@ -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",
+27
View File
@@ -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<DeterministicGuardianReviewResponse> {
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<TokenUsageProfile> {
let url = self.token_usage_profile_url();
let req = self.http.get(&url).headers(self.headers());
+1
View File
@@ -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;
+27
View File
@@ -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<String>,
}
#[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<String>,
}
#[derive(Clone, Debug, Deserialize)]
pub struct AccountEntry {
pub id: String,
+1
View File
@@ -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 }
+95 -5
View File
@@ -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<Uuid>,
}
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
@@ -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());
+2
View File
@@ -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,
});
@@ -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<String>,
pub method: Option<String>,
pub url: Option<String>,
pub command: Option<String>,
pub exec_policy_hint: Option<String>,
}
@@ -91,6 +94,7 @@ pub struct NetworkPolicyRequestArgs {
pub port: u16,
pub client_addr: Option<String>,
pub method: Option<String>,
pub url: Option<String>,
pub command: Option<String>,
pub exec_policy_hint: Option<String>,
}
@@ -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,
});
+2
View File
@@ -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,
});