diff --git a/codex-rs/network-proxy/src/domain_matcher.rs b/codex-rs/network-proxy/src/domain_matcher.rs index 30a4c1aef0..be02d34a58 100644 --- a/codex-rs/network-proxy/src/domain_matcher.rs +++ b/codex-rs/network-proxy/src/domain_matcher.rs @@ -29,41 +29,7 @@ impl DomainPatternSet { let mut restricted_ip_literals = HashSet::new(); let mut seen = HashSet::new(); for pattern in patterns { - if global_wildcard == GlobalWildcard::Reject - && is_global_wildcard_domain_pattern(pattern) - { - bail!( - "unsupported global wildcard domain pattern \"*\"; use exact hosts or scoped wildcards like *.example.com or **.example.com" - ); - } - let raw = pattern.trim(); - ensure!( - !raw.contains(['{', '}', '\\', '/']) && !raw.chars().any(char::is_whitespace), - "unsupported domain pattern syntax: {pattern}" - ); - let host = raw - .strip_prefix("**.") - .or_else(|| raw.strip_prefix("*.")) - .unwrap_or(raw); - ensure!( - !host.contains("**"), - "unsupported repeated wildcard: {pattern}" - ); - if host.contains(['[', ']']) { - let literal = host.strip_prefix('[').and_then(|s| s.split_once(']')); - ensure!( - literal.is_some_and(|(ip, suffix)| { - let normalized = normalize_dns_host_or_ip_literal(ip); - let unscoped = normalized.split('%').next().unwrap_or_default(); - !ip.contains(['[', ']']) - && (normalized == "*" || unscoped.parse::().is_ok()) - && !suffix.contains(['[', ']']) - && (suffix.chars().all(|c| c == '.') || suffix.starts_with(':')) - }), - "brackets are only supported for IPv6 literals or [*]: {pattern}" - ); - } - let pattern = normalize_pattern(pattern); + let pattern = validated_pattern(pattern, global_wildcard)?; // Supported domain patterns: // - "example.com": match the exact host // - "*.example.com": match any subdomain (not the apex) @@ -74,7 +40,6 @@ impl DomainPatternSet { if !seen.insert(candidate.clone()) { continue; } - ensure!(!candidate.is_empty(), "domain pattern is empty"); // Keep exact address equivalence separate from ordinary allowlist // matching. A scoped IP rule must never apply to other interfaces. if !candidate.contains('%') @@ -128,6 +93,92 @@ impl DomainPatternSet { } } +/// Validate and normalize an allow selector using the proxy matcher's syntax. +pub fn canonical_allow_pattern(pattern: &str) -> Result { + let host = canonical_pattern(pattern, GlobalWildcard::Allow)?; + // A wildcard in a stripped port still disqualifies this rule from the local-binding + // exception. Retain a wildcard port in the comparison key for that case. + if pattern.contains(['*', '?']) + && !host.contains(['*', '?']) + && crate::policy::is_local_literal(&host) + { + return Ok(if host.contains(':') { + format!("[{host}]:*") + } else { + format!("{host}:*") + }); + } + Ok(host) +} + +/// Validate and normalize a deny selector, including the restriction on global wildcards. +pub fn canonical_deny_pattern(pattern: &str) -> Result { + canonical_pattern(pattern, GlobalWildcard::Reject).map(restrictive_pattern_identity) +} + +/// Review rules allow global wildcards and, like denials, compare unscoped IPv6 literals by address. +pub fn canonical_review_pattern(pattern: &str) -> Result { + canonical_pattern(pattern, GlobalWildcard::Allow).map(restrictive_pattern_identity) +} + +fn restrictive_pattern_identity(pattern: String) -> String { + match pattern.parse::() { + Ok(ip) => ip.to_string(), + Err(_) => pattern, + } +} + +fn canonical_pattern(pattern: &str, global_wildcard: GlobalWildcard) -> Result { + let pattern = validated_pattern(pattern, global_wildcard)?; + // Subdomains-only selectors compile to one byte glob with this prefix. Treat the + // explicitly written glob as the same selector for list/constraint comparisons. + if let Some(domain) = pattern.strip_prefix("?*.") { + return Ok(format!("*.{domain}")); + } + Ok(pattern) +} + +fn validated_pattern(pattern: &str, global_wildcard: GlobalWildcard) -> Result { + if global_wildcard == GlobalWildcard::Reject && is_global_wildcard_domain_pattern(pattern) { + bail!( + "unsupported global wildcard domain pattern \"*\"; use exact hosts or scoped wildcards like *.example.com or **.example.com" + ); + } + let raw = pattern.trim(); + ensure!( + !raw.contains(['{', '}', '\\', '/']) && !raw.chars().any(char::is_whitespace), + "unsupported domain pattern syntax: {pattern}" + ); + let host = raw + .strip_prefix("**.") + .or_else(|| raw.strip_prefix("*.")) + .unwrap_or(raw); + ensure!( + !host.contains("**"), + "unsupported repeated wildcard: {pattern}" + ); + if host.contains(['[', ']']) { + let literal = host.strip_prefix('[').and_then(|s| s.split_once(']')); + ensure!( + literal.is_some_and(|(ip, suffix)| { + let normalized = normalize_dns_host_or_ip_literal(ip); + let unscoped = normalized.split('%').next().unwrap_or_default(); + !ip.contains(['[', ']']) + && (normalized == "*" || unscoped.parse::().is_ok()) + && !suffix.contains(['[', ']']) + && (suffix.chars().all(|c| c == '.') || suffix.starts_with(':')) + }), + "brackets are only supported for IPv6 literals or [*]: {pattern}" + ); + } + let pattern = normalize_pattern(pattern); + ensure!( + !matches!(pattern.as_str(), "" | "*." | "**."), + "domain pattern is empty" + ); + Ok(pattern) +} + /// Normalize host fragments for policy matching (trim whitespace, strip ports/brackets, lowercase). pub fn normalize_host(host: &str) -> String { let host = host.trim(); diff --git a/codex-rs/network-proxy/src/domain_matcher_tests.rs b/codex-rs/network-proxy/src/domain_matcher_tests.rs index 83c0eec3bf..329def4e24 100644 --- a/codex-rs/network-proxy/src/domain_matcher_tests.rs +++ b/codex-rs/network-proxy/src/domain_matcher_tests.rs @@ -4,6 +4,24 @@ use crate::policy::compile_denylist; use crate::policy::normalize_host; use pretty_assertions::assert_eq; +#[test] +fn comparison_keys_preserve_differences_between_grants_and_restrictions() -> Result<()> { + for (selector, grant, restriction) in [ + ("?*.EXAMPLE.com.", "*.example.com", "*.example.com"), + ("[::1]:*", "[::1]:*", "::1"), + ( + "2001:0db8:0:0:0:0:0:1", + "2001:0db8:0:0:0:0:0:1", + "2001:db8::1", + ), + ] { + assert_eq!(canonical_allow_pattern(selector)?, grant); + assert_eq!(canonical_deny_pattern(selector)?, restriction); + assert_eq!(canonical_review_pattern(selector)?, restriction); + } + Ok(()) +} + #[test] fn unicode_wildcards_preserve_utf8_byte_matching() { for (pattern, host, expected) in [ diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index cba9d70643..31edd05567 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -72,6 +72,9 @@ pub use credential_broker::brokered_credential_value_env_keys; pub use credential_broker::credential_broker_provider_context_env_keys; pub use credential_broker::credential_broker_provider_sources_allowed; pub use credential_broker::is_credential_broker_provider_env_key; +pub use domain_matcher::canonical_allow_pattern; +pub use domain_matcher::canonical_deny_pattern; +pub use domain_matcher::canonical_review_pattern; pub use environment_policy::EnvironmentNetworkPolicy; pub use mitm_hook::InjectedHeaderConfig; pub use mitm_hook::MitmHookActionsConfig; diff --git a/codex-rs/network-proxy/src/policy.rs b/codex-rs/network-proxy/src/policy.rs index 4a6f6054d0..332a26d606 100644 --- a/codex-rs/network-proxy/src/policy.rs +++ b/codex-rs/network-proxy/src/policy.rs @@ -61,6 +61,12 @@ pub fn is_loopback_host(host: &Host) -> bool { false } +/// The local-binding exception applies only to localhost and non-public IP literals. +pub(crate) fn is_local_literal(host: &str) -> bool { + let unscoped = unscoped_ip_literal(host).unwrap_or(host); + host == "localhost" || unscoped.parse::().is_ok_and(is_non_public_ip) +} + /// Matches the destinations in `DEFAULT_NO_PROXY_VALUE`, without resolving DNS. pub(crate) fn is_default_proxy_bypass_host(host: &str) -> bool { let host = normalize_host(host); diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index c7a0196674..4dbd2cd24a 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -14,7 +14,6 @@ use crate::mitm_hook::MitmHooksByHost; use crate::mitm_hook::evaluate_mitm_hooks; use crate::network_policy::NetworkPolicyAuditObserver; use crate::policy::Host; -use crate::policy::is_loopback_host; use crate::policy::is_non_public_ip; use crate::policy::normalize_host; use crate::policy::unscoped_ip_literal; @@ -922,18 +921,7 @@ impl NetworkProxyState { // request. Explicit local/loopback literals are allowed only when explicitly // allowlisted; hostnames that resolve to local/private IPs are blocked even if // allowlisted. - let local_literal = { - let host_no_scope = unscoped_ip_literal(host_str).unwrap_or(host_str); - if is_loopback_host(&host) { - true - } else if let Ok(ip) = host_no_scope.parse::() { - is_non_public_ip(ip) - } else { - false - } - }; - - if local_literal { + if crate::policy::is_local_literal(host_str) { if !is_explicit_local_allowlisted(&allowed_domains, &host) { return Ok(HostBlockDecision::Blocked(HostBlockReason::NotAllowedLocal)); }