From 529cd6b8602f77ce946502d2e67df6b41bc795af Mon Sep 17 00:00:00 2001 From: rafael-oai Date: Thu, 8 Oct 2026 00:56:48 +0000 Subject: [PATCH] Use a dedicated matcher for network domain policies (#51897) ## Why Network domain rules need explicit wildcard semantics independent of filesystem glob syntax, including matching Unicode hosts with `?`. ## What changed Replace `globset` in network allowlist and denylist matching with a domain matcher. Make `?` consume one Unicode scalar value, retain ASCII case-insensitive matching and existing host normalization, and preserve the distinction between `*.example.com` and `**.example.com`. Reject empty patterns, repeated wildcards outside the supported `**.` prefix, character classes, brace expansions, escapes, slashes, and embedded whitespace when loading policies. Continue allowing global wildcards only in allowlists. ## Testing Add coverage for Unicode wildcard semantics, unsupported syntax, IPv6 normalization, invalid entries in either policy list, and adversarial matching inputs. Compare the matcher against an independent dynamic-programming implementation over exhaustive short inputs. GitOrigin-RevId: 64f65fcda1a50998d70e046445d22cd2d2dcf27a --- codex-rs/network-proxy/src/domain_matcher.rs | 217 ++++++++++++++++ .../network-proxy/src/domain_matcher_tests.rs | 225 +++++++++++++++++ codex-rs/network-proxy/src/lib.rs | 1 + codex-rs/network-proxy/src/policy.rs | 234 ++---------------- codex-rs/network-proxy/src/runtime.rs | 48 ++-- codex-rs/network-proxy/src/state.rs | 8 +- 6 files changed, 485 insertions(+), 248 deletions(-) create mode 100644 codex-rs/network-proxy/src/domain_matcher.rs create mode 100644 codex-rs/network-proxy/src/domain_matcher_tests.rs diff --git a/codex-rs/network-proxy/src/domain_matcher.rs b/codex-rs/network-proxy/src/domain_matcher.rs new file mode 100644 index 0000000000..3fcd3b90d8 --- /dev/null +++ b/codex-rs/network-proxy/src/domain_matcher.rs @@ -0,0 +1,217 @@ +//! Network-domain wildcards over Unicode scalar values, independent of filesystem semantics. +//! Construction validates and normalizes patterns and expands domain prefixes. +//! Callers normalize candidate hosts before matching. + +use anyhow::Result; +use anyhow::bail; +use anyhow::ensure; +use std::collections::HashSet; +use std::net::IpAddr; +use std::net::Ipv6Addr; +use std::sync::Arc; + +#[derive(Clone, Copy, PartialEq, Eq)] +pub(crate) enum GlobalWildcard { + Allow, + Reject, +} + +#[derive(Clone)] +pub struct DomainPatternSet { + patterns: Arc<[Vec]>, +} + +impl DomainPatternSet { + pub(crate) fn new(patterns: &[String], global_wildcard: GlobalWildcard) -> Result { + let mut compiled = Vec::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); + // Supported domain patterns: + // - "example.com": match the exact host + // - "*.example.com": match any subdomain (not the apex) + // - "**.example.com": match the apex and any subdomain + // - "api?.example.com": match exactly one Unicode scalar value after "api" + // - "*": match every host when explicitly enabled for allowlist compilation + for candidate in expand_domain_pattern(&pattern) { + if !seen.insert(candidate.clone()) { + continue; + } + ensure!(!candidate.is_empty(), "domain pattern is empty"); + compiled.push(candidate.chars().collect()); + } + } + Ok(Self { + patterns: compiled.into(), + }) + } + + pub fn is_match(&self, host: impl AsRef) -> bool { + let host: Vec = host.as_ref().chars().collect(); + self.patterns.iter().any(|pattern| matches(pattern, &host)) + } +} + +/// Normalize host fragments for policy matching (trim whitespace, strip ports/brackets, lowercase). +pub fn normalize_host(host: &str) -> String { + let host = host.trim(); + if host.starts_with('[') + && let Some(end) = host.find(']') + { + return normalize_dns_host_or_ip_literal(&host[1..end]); + } + + // The proxy stack should typically hand us a host without a port, but be + // defensive and strip `:port` when there is exactly one `:`. + if host.bytes().filter(|b| *b == b':').count() == 1 { + let host = host.split(':').next().unwrap_or_default(); + return normalize_dns_host_or_ip_literal(host); + } + + // Avoid mangling unbracketed IPv6 literals, but strip trailing dots so fully qualified domain + // names are treated the same as their dotless variants. + normalize_dns_host_or_ip_literal(host) +} + +fn normalize_dns_host_or_ip_literal(host: &str) -> String { + let host = host.to_ascii_lowercase(); + let host = host.trim_end_matches('.'); + if let Some(ip) = normalize_ip_literal(host) { + return ip; + } + host.to_string() +} + +pub(crate) fn unscoped_ip_literal(host: &str) -> Option<&str> { + let (ip, _) = host.split_once('%')?; + ip.parse::().ok()?; + Some(ip) +} + +fn normalize_ip_literal(host: &str) -> Option { + if host.parse::().is_ok() { + return Some(host.to_string()); + } + for delimiter in ["%25", "%"] { + if let Some((ip, scope)) = host.split_once(delimiter) + && ip.parse::().is_ok() + { + return Some(format!("{ip}%{scope}")); + } + } + None +} + +fn normalize_pattern(pattern: &str) -> String { + let pattern = pattern.trim(); + if pattern == "*" { + return "*".to_string(); + } + + let (prefix, remainder) = if let Some(domain) = pattern.strip_prefix("**.") { + ("**.", domain) + } else if let Some(domain) = pattern.strip_prefix("*.") { + ("*.", domain) + } else { + ("", pattern) + }; + + let remainder = normalize_host(remainder); + if prefix.is_empty() { + remainder + } else { + format!("{prefix}{remainder}") + } +} + +pub(crate) fn is_global_wildcard_domain_pattern(pattern: &str) -> bool { + let normalized = normalize_pattern(pattern); + expand_domain_pattern(&normalized) + .iter() + .any(|candidate| candidate == "*") +} + +fn expand_domain_pattern(pattern: &str) -> Vec { + let pattern = pattern.trim(); + let Some(domain) = pattern + .strip_prefix("**.") + .or_else(|| pattern.strip_prefix("*.")) + else { + return vec![pattern.to_string()]; + }; + let domain = domain.trim(); + if domain.is_empty() { + return vec![String::new()]; + } + if pattern.starts_with("**.") { + vec![domain.to_string(), format!("?*.{domain}")] + } else { + vec![format!("?*.{domain}")] + } +} + +// Only the latest star needs retrying: it can absorb any extension that an earlier star could. +// Its retry position advances monotonically, bounding work by O(pattern length * host length). +fn matches(pattern: &[char], host: &[char]) -> bool { + let (mut p, mut h) = (0, 0); + let mut star = None; + while h < host.len() { + match pattern.get(p) { + Some('*') => { + star = Some((p + 1, h)); + p += 1; + } + Some(c) if *c == '?' || c.eq_ignore_ascii_case(&host[h]) => { + p += 1; + h += 1; + } + _ => match &mut star { + Some((resume, consumed)) => { + *consumed += 1; + p = *resume; + h = *consumed; + } + None => return false, + }, + } + } + pattern[p..].iter().all(|c| *c == '*') +} + +#[cfg(test)] +#[path = "domain_matcher_tests.rs"] +mod tests; diff --git a/codex-rs/network-proxy/src/domain_matcher_tests.rs b/codex-rs/network-proxy/src/domain_matcher_tests.rs new file mode 100644 index 0000000000..c38562b07d --- /dev/null +++ b/codex-rs/network-proxy/src/domain_matcher_tests.rs @@ -0,0 +1,225 @@ +use super::*; +use crate::policy::compile_allowlist; +use crate::policy::compile_denylist; +use crate::policy::normalize_host; +use pretty_assertions::assert_eq; + +#[test] +fn unicode_wildcards_count_scalars_not_bytes_or_graphemes() { + for (pattern, host, expected) in [ + ("api?.com", "apié.com", true), + ("api?.com", "api😀.com", true), + ("api??.com", "apié.com", false), + ("api?.com", "apie\u{301}.com", false), + ("api??.com", "apie\u{301}.com", true), + ("*.例?.com", "a.例子.com", true), + ("*.例?.com", "例子.com", false), + ("**.例?.com", "例子.com", true), + ("É.com", "é.com", false), + ("É.COM", "É.com", true), + ] { + for compile in [compile_allowlist, compile_denylist] { + assert_eq!( + compile(&[pattern.to_string()]) + .unwrap() + .is_match(normalize_host(host)), + expected, + "{pattern} / {host}" + ); + } + } +} + +#[test] +fn compilation_rejects_unsupported_syntax_instead_of_dropping_rules() { + for pattern in [ + "", + " ", + "*.", + "**.", + "**", + "a**b.com", + "[ab].com", + "a[bc].com", + "{a,b}.com", + r"a\*.com", + "a/b.com", + "a b.com", + "[::1]suffix", + "[fe80::1%abc[def]", + "[::1]:443[ab]", + "example.com:{a,b}", + ] { + for compile in [compile_allowlist, compile_denylist] { + assert!( + compile(&["valid.com".to_string(), pattern.to_string()]).is_err(), + "{pattern}" + ); + } + } + for pattern in [ + "[::1]", + "[::1]:443", + "[fe80::1%25en0]:443", + "region*.com", + "**.api?.com", + ] { + assert!( + compile_allowlist(&[pattern.to_string()]).is_ok(), + "{pattern}" + ); + } + for pattern in ["*", "**.*", "[*]", "**.[*]"] { + assert!( + compile_denylist(&[pattern.to_string()]).is_err(), + "{pattern}" + ); + } +} + +#[test] +fn bracketed_patterns_preserve_port_and_trailing_dot_normalization() { + for (bracketed, plain) in [ + ("[*]", "*"), + ("[*]:443", "*"), + ("[*.]", "*"), + ("[*].", "*"), + ("[::1]:443", "::1"), + ("[::1.]", "::1"), + ("[::1].", "::1"), + ("[fe80::1%25en0]:443", "fe80::1%en0"), + ] { + for prefix in ["", "*.", "**."] { + for compile in [compile_allowlist, compile_denylist] { + let compiled_bracketed = compile(&[format!(" {prefix}{bracketed} ")]); + let compiled_plain = compile(&[format!("{prefix}{plain}")]); + match (compiled_bracketed, compiled_plain) { + (Ok(bracketed), Ok(plain)) => assert_eq!(bracketed.patterns, plain.patterns), + (Err(_), Err(_)) => {} + _ => panic!("different compilation outcomes for {prefix}{bracketed:?}"), + } + } + } + } +} + +#[test] +fn matcher_agrees_with_independent_dynamic_program_for_short_inputs() { + fn words(alphabet: &[char], max_len: usize) -> Vec> { + let mut all = vec![vec![]]; + let mut level = vec![vec![]]; + for _ in 0..max_len { + level = level + .iter() + .flat_map(|word| { + alphabet.iter().map(move |c| { + let mut next = word.clone(); + next.push(*c); + next + }) + }) + .collect(); + all.extend(level.iter().cloned()); + } + all + } + fn reference(pattern: &[char], host: &[char]) -> bool { + let mut rows = vec![vec![false; host.len() + 1]; pattern.len() + 1]; + rows[0][0] = true; + for (p, c) in pattern.iter().enumerate() { + rows[p + 1][0] = *c == '*' && rows[p][0]; + for (h, candidate) in host.iter().enumerate() { + rows[p + 1][h + 1] = if *c == '*' { + rows[p][h + 1] || rows[p + 1][h] + } else { + rows[p][h] && (*c == '?' || c.eq_ignore_ascii_case(candidate)) + }; + } + } + rows[pattern.len()][host.len()] + } + for pattern in words(&['a', 'é', '?', '*'], /*max_len*/ 4) { + for host in words(&['a', 'é'], /*max_len*/ 5) { + assert_eq!( + matches(&pattern, &host), + reference(&pattern, &host), + "{pattern:?} / {host:?}" + ); + } + } +} + +#[test] +fn adversarial_retries_and_empty_boundaries() { + let pattern: Vec = format!("*{}b", "a".repeat(1024)).chars().collect(); + let host = vec!['a'; 4096]; + assert!(!matches(&pattern, &host)); + let pattern: Vec = format!("{}b", "*a".repeat(1024)).chars().collect(); + assert!(!matches(&pattern, &host)); + assert_eq!( + ( + matches(&[], &[]), + matches(&['*'], &[]), + matches(&['?'], &[]) + ), + (true, true, false) + ); + assert!(!compile_allowlist(&[]).unwrap().is_match("host")); +} + +#[test] +fn policy_loading_rejects_an_invalid_entry_in_either_list() { + for denied in [false, true] { + let mut config = crate::config::NetworkProxyConfig::default(); + config.set_allowed_domains(vec!["example.com".to_string()]); + let invalid = vec!["valid.com".to_string(), "api[12].com".to_string()]; + if denied { + config.set_denied_domains(invalid); + } else { + config.set_allowed_domains(invalid); + } + assert!( + crate::state::build_config_state(config, Default::default(), crate::Platform::native()) + .is_err() + ); + } +} + +#[test] +fn normalize_host_lowercases_and_trims() { + assert_eq!(normalize_host(" ExAmPlE.CoM "), "example.com"); +} + +#[test] +fn normalize_host_strips_port_for_host_port() { + assert_eq!(normalize_host("example.com:1234"), "example.com"); +} + +#[test] +fn normalize_host_preserves_unbracketed_ipv6() { + assert_eq!(normalize_host("2001:db8::1"), "2001:db8::1"); +} + +#[test] +fn normalize_host_strips_trailing_dot() { + assert_eq!(normalize_host("example.com."), "example.com"); + assert_eq!(normalize_host("ExAmPlE.CoM."), "example.com"); +} + +#[test] +fn normalize_host_strips_trailing_dot_with_port() { + assert_eq!(normalize_host("example.com.:443"), "example.com"); +} + +#[test] +fn normalize_host_strips_brackets_for_ipv6() { + assert_eq!(normalize_host("[::1]"), "::1"); + assert_eq!(normalize_host("[::1]:443"), "::1"); +} + +#[test] +fn normalize_host_preserves_ipv6_scope_ids() { + assert_eq!(normalize_host("fe80::1%lo0"), "fe80::1%lo0"); + assert_eq!(normalize_host("[fe80::1%lo0]"), "fe80::1%lo0"); + assert_eq!(normalize_host("[fe80::1%25lo0]"), "fe80::1%lo0"); +} diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 559ec0a9b1..451d956da3 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -8,6 +8,7 @@ mod config; mod connect_policy; mod connection_lifecycle; mod credential_broker; +mod domain_matcher; mod environment_policy; mod http_proxy; mod mitm; diff --git a/codex-rs/network-proxy/src/policy.rs b/codex-rs/network-proxy/src/policy.rs index 3b2486be1f..d4f2142295 100644 --- a/codex-rs/network-proxy/src/policy.rs +++ b/codex-rs/network-proxy/src/policy.rs @@ -1,13 +1,12 @@ #[cfg(test)] use crate::config::NetworkMode; -use anyhow::Context; +use crate::domain_matcher::DomainPatternSet; +use crate::domain_matcher::GlobalWildcard; +pub(crate) use crate::domain_matcher::is_global_wildcard_domain_pattern; +pub use crate::domain_matcher::normalize_host; +pub(crate) use crate::domain_matcher::unscoped_ip_literal; use anyhow::Result; -use anyhow::bail; use anyhow::ensure; -use globset::GlobBuilder; -use globset::GlobSet; -use globset::GlobSetBuilder; -use std::collections::HashSet; use std::net::IpAddr; use std::net::Ipv4Addr; use std::net::Ipv6Addr; @@ -120,133 +119,12 @@ fn is_non_public_ipv6(ip: Ipv6Addr) -> bool { || ip.is_unicast_link_local() } -/// Normalize host fragments for policy matching (trim whitespace, strip ports/brackets, lowercase). -pub fn normalize_host(host: &str) -> String { - let host = host.trim(); - if host.starts_with('[') - && let Some(end) = host.find(']') - { - return normalize_dns_host_or_ip_literal(&host[1..end]); - } - - // The proxy stack should typically hand us a host without a port, but be - // defensive and strip `:port` when there is exactly one `:`. - if host.bytes().filter(|b| *b == b':').count() == 1 { - let host = host.split(':').next().unwrap_or_default(); - return normalize_dns_host_or_ip_literal(host); - } - - // Avoid mangling unbracketed IPv6 literals, but strip trailing dots so fully qualified domain - // names are treated the same as their dotless variants. - normalize_dns_host_or_ip_literal(host) +pub(crate) fn compile_allowlist(patterns: &[String]) -> Result { + DomainPatternSet::new(patterns, GlobalWildcard::Allow) } -fn normalize_dns_host_or_ip_literal(host: &str) -> String { - let host = host.to_ascii_lowercase(); - let host = host.trim_end_matches('.'); - if let Some(ip) = normalize_ip_literal(host) { - return ip; - } - host.to_string() -} - -pub(crate) fn unscoped_ip_literal(host: &str) -> Option<&str> { - let (ip, _) = host.split_once('%')?; - ip.parse::().ok()?; - Some(ip) -} - -fn normalize_ip_literal(host: &str) -> Option { - if host.parse::().is_ok() { - return Some(host.to_string()); - } - for delimiter in ["%25", "%"] { - if let Some((ip, scope)) = host.split_once(delimiter) - && ip.parse::().is_ok() - { - return Some(format!("{ip}%{scope}")); - } - } - None -} - -fn normalize_pattern(pattern: &str) -> String { - let pattern = pattern.trim(); - if pattern == "*" { - return "*".to_string(); - } - - let (prefix, remainder) = if let Some(domain) = pattern.strip_prefix("**.") { - ("**.", domain) - } else if let Some(domain) = pattern.strip_prefix("*.") { - ("*.", domain) - } else { - ("", pattern) - }; - - let remainder = normalize_host(remainder); - if prefix.is_empty() { - remainder - } else { - format!("{prefix}{remainder}") - } -} - -pub(crate) fn is_global_wildcard_domain_pattern(pattern: &str) -> bool { - let normalized = normalize_pattern(pattern); - expand_domain_pattern(&normalized) - .iter() - .any(|candidate| candidate == "*") -} - -#[derive(Clone, Copy, PartialEq, Eq)] -enum GlobalWildcard { - Allow, - Reject, -} - -pub(crate) fn compile_allowlist_globset(patterns: &[String]) -> Result { - compile_globset_with_policy(patterns, GlobalWildcard::Allow) -} - -pub(crate) fn compile_denylist_globset(patterns: &[String]) -> Result { - compile_globset_with_policy(patterns, GlobalWildcard::Reject) -} - -// Browser network-policy matchers implement a subset of this hostname grammar. -// Keep changes to shared grammar and normalization in sync with their contract -// cases and the Rust tests below, including compile_globset_supports_question_mark_wildcards. -fn compile_globset_with_policy( - patterns: &[String], - global_wildcard: GlobalWildcard, -) -> Result { - let mut builder = GlobSetBuilder::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 pattern = normalize_pattern(pattern); - // Supported domain patterns: - // - "example.com": match the exact host - // - "*.example.com": match any subdomain (not the apex) - // - "**.example.com": match the apex and any subdomain - // - "api?.example.com": match exactly one character after "api" - // - "*": match every host when explicitly enabled for allowlist compilation - for candidate in expand_domain_pattern(&pattern) { - if !seen.insert(candidate.clone()) { - continue; - } - let glob = GlobBuilder::new(&candidate) - .case_insensitive(true) - .build() - .with_context(|| format!("invalid glob pattern: {candidate}"))?; - builder.add(glob); - } - } - Ok(builder.build()?) +pub(crate) fn compile_denylist(patterns: &[String]) -> Result { + DomainPatternSet::new(patterns, GlobalWildcard::Reject) } #[derive(Debug, Clone)] @@ -257,24 +135,6 @@ pub(crate) enum DomainPattern { } impl DomainPattern { - /// Parse a policy pattern for constraint comparisons. - /// - /// Validation of glob syntax happens when building the globset; here we only - /// decode the wildcard prefixes to keep constraint checks lightweight. - pub(crate) fn parse(input: &str) -> Self { - let input = input.trim(); - if input.is_empty() { - return Self::Exact(String::new()); - } - if let Some(domain) = input.strip_prefix("**.") { - Self::parse_domain(domain, Self::ApexAndSubdomains) - } else if let Some(domain) = input.strip_prefix("*.") { - Self::parse_domain(domain, Self::SubdomainsOnly) - } else { - Self::Exact(input.to_string()) - } - } - /// Parse a policy pattern for constraint comparisons, validating domain parts with `url`. pub(crate) fn parse_for_constraints(input: &str) -> Self { let input = input.trim(); @@ -290,14 +150,6 @@ impl DomainPattern { Self::Exact(parse_domain_for_constraints(input)) } - fn parse_domain(domain: &str, build: impl FnOnce(String) -> Self) -> Self { - let domain = domain.trim(); - if domain.is_empty() { - return Self::Exact(String::new()); - } - build(domain.to_string()) - } - pub(crate) fn allows(&self, candidate: &DomainPattern) -> bool { match self { DomainPattern::Exact(domain) => match candidate { @@ -345,18 +197,6 @@ fn parse_domain_for_constraints(domain: &str) -> String { } } -fn expand_domain_pattern(pattern: &str) -> Vec { - match DomainPattern::parse(pattern) { - DomainPattern::Exact(domain) => vec![domain], - DomainPattern::SubdomainsOnly(domain) => { - vec![format!("?*.{domain}")] - } - DomainPattern::ApexAndSubdomains(domain) => { - vec![domain.clone(), format!("?*.{domain}")] - } - } -} - fn normalize_domain(domain: &str) -> String { domain.trim_end_matches('.').to_ascii_lowercase() } @@ -404,7 +244,7 @@ mod tests { #[test] fn compile_globset_normalizes_trailing_dots() { - let set = compile_denylist_globset(&["Example.COM.".to_string()]).unwrap(); + let set = compile_denylist(&["Example.COM.".to_string()]).unwrap(); assert_eq!(true, set.is_match("example.com")); assert_eq!(false, set.is_match("api.example.com")); @@ -412,7 +252,7 @@ mod tests { #[test] fn compile_globset_normalizes_wildcards() { - let set = compile_denylist_globset(&["*.Example.COM.".to_string()]).unwrap(); + let set = compile_denylist(&["*.Example.COM.".to_string()]).unwrap(); assert_eq!(true, set.is_match("api.example.com")); assert_eq!(false, set.is_match("example.com")); @@ -420,7 +260,7 @@ mod tests { #[test] fn compile_globset_supports_mid_label_wildcards() { - let set = compile_denylist_globset(&["region*.v2.argotunnel.com".to_string()]).unwrap(); + let set = compile_denylist(&["region*.v2.argotunnel.com".to_string()]).unwrap(); assert_eq!(true, set.is_match("region1.v2.argotunnel.com")); assert_eq!(true, set.is_match("region.v2.argotunnel.com")); @@ -454,10 +294,7 @@ mod tests { (" API?.EXAMPLE.COM. ", "API1.EXAMPLE.COM.", true), ] { let patterns = [pattern.to_string()]; - for set in [ - compile_allowlist_globset(&patterns)?, - compile_denylist_globset(&patterns)?, - ] { + for set in [compile_allowlist(&patterns)?, compile_denylist(&patterns)?] { assert_eq!( set.is_match(normalize_host(host)), expected, @@ -470,7 +307,7 @@ mod tests { #[test] fn compile_globset_normalizes_apex_and_subdomains() { - let set = compile_denylist_globset(&["**.Example.COM.".to_string()]).unwrap(); + let set = compile_denylist(&["**.Example.COM.".to_string()]).unwrap(); assert_eq!(true, set.is_match("example.com")); assert_eq!(true, set.is_match("api.example.com")); @@ -478,14 +315,14 @@ mod tests { #[test] fn compile_globset_normalizes_bracketed_ipv6_literals() { - let set = compile_denylist_globset(&["[::1]".to_string()]).unwrap(); + let set = compile_denylist(&["[::1]".to_string()]).unwrap(); assert_eq!(true, set.is_match("::1")); } #[test] fn compile_globset_preserves_scoped_ipv6_literals() { - let set = compile_denylist_globset(&["[fe80::1%25lo0]".to_string()]).unwrap(); + let set = compile_denylist(&["[fe80::1%25lo0]".to_string()]).unwrap(); assert_eq!(true, set.is_match("fe80::1%lo0")); assert_eq!(false, set.is_match("fe80::1%lo1")); @@ -530,43 +367,4 @@ mod tests { assert!(is_non_public_ip("fe80::1".parse().unwrap())); assert!(is_non_public_ip("fc00::1".parse().unwrap())); } - - #[test] - fn normalize_host_lowercases_and_trims() { - assert_eq!(normalize_host(" ExAmPlE.CoM "), "example.com"); - } - - #[test] - fn normalize_host_strips_port_for_host_port() { - assert_eq!(normalize_host("example.com:1234"), "example.com"); - } - - #[test] - fn normalize_host_preserves_unbracketed_ipv6() { - assert_eq!(normalize_host("2001:db8::1"), "2001:db8::1"); - } - - #[test] - fn normalize_host_strips_trailing_dot() { - assert_eq!(normalize_host("example.com."), "example.com"); - assert_eq!(normalize_host("ExAmPlE.CoM."), "example.com"); - } - - #[test] - fn normalize_host_strips_trailing_dot_with_port() { - assert_eq!(normalize_host("example.com.:443"), "example.com"); - } - - #[test] - fn normalize_host_strips_brackets_for_ipv6() { - assert_eq!(normalize_host("[::1]"), "::1"); - assert_eq!(normalize_host("[::1]:443"), "::1"); - } - - #[test] - fn normalize_host_preserves_ipv6_scope_ids() { - assert_eq!(normalize_host("fe80::1%lo0"), "fe80::1%lo0"); - assert_eq!(normalize_host("[fe80::1%lo0]"), "fe80::1%lo0"); - assert_eq!(normalize_host("[fe80::1%25lo0]"), "fe80::1%lo0"); - } } diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index c8d2f6c7a4..87457c7464 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -3,6 +3,7 @@ use crate::config::NetworkMode; use crate::config::NetworkProxyConfig; use crate::config::ValidatedUnixSocketPath; use crate::credential_broker::CredentialBroker; +use crate::domain_matcher::DomainPatternSet; use crate::mitm::MitmState; use crate::mitm_hook::HookEvaluation; use crate::mitm_hook::MitmHooksByHost; @@ -23,7 +24,6 @@ use crate::state::validate_policy_against_constraints; use anyhow::Context; use anyhow::Result; use codex_utils_absolute_path::AbsolutePathBuf; -use globset::GlobSet; use opentelemetry::trace::SpanContext; use serde::Deserialize; use serde::Serialize; @@ -178,8 +178,8 @@ pub struct ConfigState { pub(crate) executor_os: crate::Platform, pub config: NetworkProxyConfig, pub(crate) brokerage_created_default_allowlist: bool, - pub allow_set: GlobSet, - pub deny_set: GlobSet, + pub allow_set: DomainPatternSet, + pub deny_set: DomainPatternSet, pub mitm: Option>, pub mitm_hooks: MitmHooksByHost, pub constraints: NetworkProxyConstraints, @@ -296,7 +296,7 @@ struct ExecutionAttribution { impl std::fmt::Debug for NetworkProxyState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // Avoid logging internal state (config contents, derived globsets, etc.) which can be noisy + // Avoid logging internal state (config contents, compiled domain patterns, etc.) which can be noisy // and may contain sensitive paths. f.debug_struct("NetworkProxyState").finish_non_exhaustive() } @@ -768,12 +768,12 @@ impl NetworkProxyState { // 1) explicit deny always wins // 2) local/private networking is opt-in (defense-in-depth) // 3) DNS requires an allowlist match or an approval - if globset_matches_host_or_unscoped(&deny_set, host_str) { + if domains_match_host_or_unscoped(&deny_set, host_str) { return Ok(HostBlockDecision::Blocked(HostBlockReason::Denied)); } let is_authorized = authorization == HostAuthorization::Approved - || globset_matches_host_or_unscoped(&allow_set, host_str); + || domains_match_host_or_unscoped(&allow_set, host_str); if !allow_local_binding { // If the intent is "prevent access to local/internal networks", we must not rely solely // on string checks like `localhost` / `127.0.0.1`. Attackers can use DNS rebinding or @@ -1244,7 +1244,7 @@ fn log_domain_list_changes(list_name: &str, previous: &[String], next: &[String] } } -fn globset_matches_host_or_unscoped(set: &GlobSet, host: &str) -> bool { +fn domains_match_host_or_unscoped(set: &DomainPatternSet, host: &str) -> bool { set.is_match(host) || unscoped_ip_literal(host).is_some_and(|ip| set.is_match(ip)) } @@ -1281,18 +1281,14 @@ pub(crate) fn network_proxy_state_for_policy( } let state = ConfigState { executor_os: crate::Platform::native(), - allow_set: crate::policy::compile_allowlist_globset( - &config.allowed_domains().unwrap_or_default(), - ) - .unwrap(), + allow_set: crate::policy::compile_allowlist(&config.allowed_domains().unwrap_or_default()) + .unwrap(), blocked: VecDeque::new(), blocked_total: 0, brokerage_created_default_allowlist, constraints: NetworkProxyConstraints::default(), - deny_set: crate::policy::compile_denylist_globset( - &config.denied_domains().unwrap_or_default(), - ) - .unwrap(), + deny_set: crate::policy::compile_denylist(&config.denied_domains().unwrap_or_default()) + .unwrap(), mitm: None, mitm_hooks: crate::mitm_hook::compile_mitm_hooks(&config).unwrap(), config, @@ -1324,8 +1320,8 @@ mod tests { use super::*; use crate::config::NetworkProxyConfig; - use crate::policy::compile_allowlist_globset; - use crate::policy::compile_denylist_globset; + use crate::policy::compile_allowlist; + use crate::policy::compile_denylist; use crate::state::NetworkProxyConstraints; use crate::state::build_config_state; use crate::state::validate_policy_against_constraints; @@ -2214,7 +2210,7 @@ mod tests { #[test] fn compile_globset_is_case_insensitive() { let patterns = vec!["ExAmPle.CoM".to_string()]; - let set = compile_denylist_globset(&patterns).unwrap(); + let set = compile_denylist(&patterns).unwrap(); assert!(set.is_match("example.com")); assert!(set.is_match("EXAMPLE.COM")); } @@ -2222,7 +2218,7 @@ mod tests { #[test] fn compile_globset_excludes_apex_for_subdomain_patterns() { let patterns = vec!["*.openai.com".to_string()]; - let set = compile_denylist_globset(&patterns).unwrap(); + let set = compile_denylist(&patterns).unwrap(); assert!(set.is_match("api.openai.com")); assert!(!set.is_match("openai.com")); assert!(!set.is_match("evilopenai.com")); @@ -2231,7 +2227,7 @@ mod tests { #[test] fn compile_globset_includes_apex_for_double_wildcard_patterns() { let patterns = vec!["**.openai.com".to_string()]; - let set = compile_denylist_globset(&patterns).unwrap(); + let set = compile_denylist(&patterns).unwrap(); assert!(set.is_match("openai.com")); assert!(set.is_match("api.openai.com")); assert!(!set.is_match("evilopenai.com")); @@ -2240,13 +2236,13 @@ mod tests { #[test] fn compile_globset_rejects_global_wildcard() { let patterns = vec!["*".to_string()]; - assert!(compile_denylist_globset(&patterns).is_err()); + assert!(compile_denylist(&patterns).is_err()); } #[test] fn compile_globset_allows_global_wildcard_when_enabled() { let patterns = vec!["*".to_string()]; - let set = compile_allowlist_globset(&patterns).unwrap(); + let set = compile_allowlist(&patterns).unwrap(); assert!(set.is_match("example.com")); assert!(set.is_match("api.openai.com")); assert!(set.is_match("localhost")); @@ -2255,19 +2251,19 @@ mod tests { #[test] fn compile_globset_rejects_bracketed_global_wildcard() { let patterns = vec!["[*]".to_string()]; - assert!(compile_denylist_globset(&patterns).is_err()); + assert!(compile_denylist(&patterns).is_err()); } #[test] fn compile_globset_rejects_double_wildcard_bracketed_global_wildcard() { let patterns = vec!["**.[*]".to_string()]; - assert!(compile_denylist_globset(&patterns).is_err()); + assert!(compile_denylist(&patterns).is_err()); } #[test] fn compile_globset_dedupes_patterns_without_changing_behavior() { let patterns = vec!["example.com".to_string(), "example.com".to_string()]; - let set = compile_denylist_globset(&patterns).unwrap(); + let set = compile_denylist(&patterns).unwrap(); assert!(set.is_match("example.com")); assert!(set.is_match("EXAMPLE.COM")); assert!(!set.is_match("not-example.com")); @@ -2276,7 +2272,7 @@ mod tests { #[test] fn compile_globset_rejects_invalid_patterns() { let patterns = vec!["[".to_string()]; - assert!(compile_denylist_globset(&patterns).is_err()); + assert!(compile_denylist(&patterns).is_err()); } #[test] diff --git a/codex-rs/network-proxy/src/state.rs b/codex-rs/network-proxy/src/state.rs index c5e4e06708..b31367170e 100644 --- a/codex-rs/network-proxy/src/state.rs +++ b/codex-rs/network-proxy/src/state.rs @@ -8,8 +8,8 @@ use crate::mitm_hook::MitmHookConfig; use crate::mitm_hook::compile_mitm_hooks; use crate::mitm_hook::validate_mitm_hook_config; use crate::policy::DomainPattern; -use crate::policy::compile_allowlist_globset; -use crate::policy::compile_denylist_globset; +use crate::policy::compile_allowlist; +use crate::policy::compile_denylist; use crate::policy::is_global_wildcard_domain_pattern; use crate::runtime::ConfigState; use serde::Deserialize; @@ -90,8 +90,8 @@ pub fn build_config_state( let denied_domains = config.denied_domains().unwrap_or_default(); validate_non_global_wildcard_domain_patterns("network.denied_domains", &denied_domains) .map_err(NetworkProxyConstraintError::into_anyhow)?; - let deny_set = compile_denylist_globset(&denied_domains)?; - let allow_set = compile_allowlist_globset(&allowed_domains)?; + let deny_set = compile_denylist(&denied_domains)?; + let allow_set = compile_allowlist(&allowed_domains)?; let mitm_hooks = compile_mitm_hooks(&config)?; let mitm = if config.mitm { Some(Arc::new(MitmState::new(MitmUpstreamConfig {