From a60197d6a06b026f7afb72e7f7469ea8ec95683e Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Fri, 13 Nov 2026 16:44:51 +0000 Subject: [PATCH] Expose canonical network proxy policy patterns (#61351) ## What changed Add `canonical_allow_pattern`, `canonical_deny_pattern`, and `canonical_review_pattern` to validate and normalize selectors for policy comparisons using the domain matcher's syntax. Treat `?*.example.com` and `*.example.com` as equivalent comparison keys. Canonicalize unscoped IPv6 addresses for deny and review rules, and preserve wildcard ports on local allow rules so they remain distinguishable from explicit local grants. Deny rules continue to reject global wildcards. Share pattern validation with the matcher and local-literal classification with runtime policy checks. ## Testing Add a regression test covering equivalent subdomain selectors, local wildcard ports, and differing IPv6 comparison keys for grants and restrictions. GitOrigin-RevId: c72262ae9f625765d5351e9da403be1f87042a47 --- codex-rs/network-proxy/src/domain_matcher.rs | 123 +++++++++++++----- .../network-proxy/src/domain_matcher_tests.rs | 18 +++ codex-rs/network-proxy/src/lib.rs | 3 + codex-rs/network-proxy/src/policy.rs | 6 + codex-rs/network-proxy/src/runtime.rs | 14 +- 5 files changed, 115 insertions(+), 49 deletions(-) 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)); }