From 6b69ac60bd666b10d9dcfaaff762c68e9d16b26a Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 9 Jul 2026 12:38:11 -0700 Subject: [PATCH] fix(network-proxy): update dns tests for flattened config Co-authored-by: Codex noreply@openai.com --- codex-rs/network-proxy/src/dns_tests.rs | 107 +++++++++++++----------- codex-rs/network-proxy/src/proxy.rs | 10 +-- codex-rs/network-proxy/src/runtime.rs | 4 +- 3 files changed, 62 insertions(+), 59 deletions(-) diff --git a/codex-rs/network-proxy/src/dns_tests.rs b/codex-rs/network-proxy/src/dns_tests.rs index 7013fe15be..831a4bb552 100644 --- a/codex-rs/network-proxy/src/dns_tests.rs +++ b/codex-rs/network-proxy/src/dns_tests.rs @@ -1,7 +1,6 @@ use super::*; use crate::attribution::write_attribution_frame; use crate::config::NetworkProxyConfig; -use crate::config::NetworkProxySettings; use crate::network_policy::test_support::POLICY_DECISION_EVENT_NAME; use crate::network_policy::test_support::capture_events; use crate::network_policy::test_support::find_event_by_name; @@ -19,11 +18,11 @@ use tokio::io::AsyncReadExt; use tokio::io::AsyncWriteExt; use tokio::net::TcpStream as TokioTcpStream; -fn settings(allowed_domains: &[&str], denied_domains: &[&str]) -> NetworkProxySettings { - let mut settings = NetworkProxySettings { +fn settings(allowed_domains: &[&str], denied_domains: &[&str]) -> NetworkProxyConfig { + let mut settings = NetworkProxyConfig { enabled: true, allow_local_binding: false, - ..NetworkProxySettings::default() + ..NetworkProxyConfig::default() }; settings.set_allowed_domains( allowed_domains @@ -75,7 +74,7 @@ async fn query_listener( )); } - let query = query(name, RecordType::A, 16); + let query = query(name, RecordType::A, /*id*/ 16); let query_len = u16::try_from(query.len()).expect("bounded DNS query length"); stream.write_all(&query_len.to_be_bytes()).await?; stream.write_all(&query).await?; @@ -105,7 +104,7 @@ async fn resolves_allowed_a_and_aaaa_queries_with_bounded_answers() { ] { let wire = resolve_query_with_lookup( &state, - &query("example.test", record_type, 7), + &query("example.test", record_type, /*id*/ 7), Some("127.0.0.1:1234"), |_| { let addresses = addresses.clone(); @@ -136,8 +135,8 @@ async fn synthesized_response_never_exceeds_classic_udp_size() { .collect::>(); let wire = resolve_query_with_lookup( &state, - &query("many.example.test", RecordType::AAAA, 7), - None, + &query("many.example.test", RecordType::AAAA, /*id*/ 7), + /*client_addr*/ None, |_| async move { Ok(addresses) }, ) .await; @@ -154,13 +153,18 @@ async fn denied_name_is_not_sent_to_the_host_resolver() { let state = network_proxy_state_for_policy(settings(&["allowed.test"], &[])); let lookups = Arc::new(AtomicUsize::new(0)); let response = response( - resolve_query_with_lookup(&state, &query("denied.test", RecordType::A, 8), None, { - let lookups = Arc::clone(&lookups); - move |_| async move { - lookups.fetch_add(1, Ordering::SeqCst); - Ok(Vec::new()) - } - }) + resolve_query_with_lookup( + &state, + &query("denied.test", RecordType::A, /*id*/ 8), + /*client_addr*/ None, + { + let lookups = Arc::clone(&lookups); + move |_| async move { + lookups.fetch_add(1, Ordering::SeqCst); + Ok(Vec::new()) + } + }, + ) .await, ); @@ -174,8 +178,8 @@ async fn refuses_entire_answer_set_when_resolution_contains_a_private_address() let response = response( resolve_query_with_lookup( &state, - &query("example.test", RecordType::A, 9), - None, + &query("example.test", RecordType::A, /*id*/ 9), + /*client_addr*/ None, |_| async { Ok(vec![ "93.184.216.34".parse().expect("public address"), @@ -196,8 +200,8 @@ async fn explicitly_allowlisted_local_literal_preserves_local_policy_semantics() let response = response( resolve_query_with_lookup( &state, - &query("127.0.0.1", RecordType::A, 9), - None, + &query("127.0.0.1", RecordType::A, /*id*/ 9), + /*client_addr*/ None, |_| async { Ok(vec!["127.0.0.1".parse().expect("loopback address")]) }, ) .await, @@ -213,8 +217,8 @@ async fn policy_changes_apply_without_restarting_the_dns_service() { let allowed = response( resolve_query_with_lookup( &state, - &query("example.test", RecordType::A, 10), - None, + &query("example.test", RecordType::A, /*id*/ 10), + /*client_addr*/ None, |_| async { Ok(vec!["93.184.216.34".parse().expect("public address")]) }, ) .await, @@ -223,13 +227,8 @@ async fn policy_changes_apply_without_restarting_the_dns_service() { let mut replacement_network = settings(&["other.test"], &[]); replacement_network.enabled = true; - let replacement = build_config_state( - NetworkProxyConfig { - network: replacement_network, - }, - NetworkProxyConstraints::default(), - ) - .expect("replacement config state"); + let replacement = build_config_state(replacement_network, NetworkProxyConstraints::default()) + .expect("replacement config state"); state .replace_config_state(replacement) .await @@ -237,8 +236,8 @@ async fn policy_changes_apply_without_restarting_the_dns_service() { let denied = response( resolve_query_with_lookup( &state, - &query("example.test", RecordType::A, 11), - None, + &query("example.test", RecordType::A, /*id*/ 11), + /*client_addr*/ None, |_| async { Ok(vec!["93.184.216.34".parse().expect("public address")]) }, ) .await, @@ -251,13 +250,8 @@ async fn live_disable_is_a_proxy_state_denial_without_host_lookup() { let state = network_proxy_state_for_policy(settings(&["example.test"], &[])); let mut disabled_network = settings(&["example.test"], &[]); disabled_network.enabled = false; - let disabled = build_config_state( - NetworkProxyConfig { - network: disabled_network, - }, - NetworkProxyConstraints::default(), - ) - .expect("disabled config state"); + let disabled = build_config_state(disabled_network, NetworkProxyConstraints::default()) + .expect("disabled config state"); state .replace_config_state(disabled) .await @@ -266,13 +260,18 @@ async fn live_disable_is_a_proxy_state_denial_without_host_lookup() { let lookups = Arc::new(AtomicUsize::new(0)); let (response, events) = capture_events(|| async { response( - resolve_query_with_lookup(&state, &query("example.test", RecordType::A, 12), None, { - let lookups = Arc::clone(&lookups); - move |_| async move { - lookups.fetch_add(1, Ordering::SeqCst); - Ok(Vec::new()) - } - }) + resolve_query_with_lookup( + &state, + &query("example.test", RecordType::A, /*id*/ 12), + /*client_addr*/ None, + { + let lookups = Arc::clone(&lookups); + move |_| async move { + lookups.fetch_add(1, Ordering::SeqCst); + Ok(Vec::new()) + } + }, + ) .await, ) }) @@ -295,8 +294,8 @@ async fn resolver_errors_are_not_reported_as_policy_denials() { let response = response( resolve_query_with_lookup( &state, - &query("example.test", RecordType::A, 12), - None, + &query("example.test", RecordType::A, /*id*/ 12), + /*client_addr*/ None, |_| async { Err(io::Error::other("resolver unavailable")) }, ) .await, @@ -329,15 +328,21 @@ async fn refuses_unsupported_and_untrusted_query_shapes() { RecordType::AAAA, )); let queries = [ - query("example.test", RecordType::CNAME, 12), - query("example.test", RecordType::TXT, 13), + query("example.test", RecordType::CNAME, /*id*/ 12), + query("example.test", RecordType::TXT, /*id*/ 13), multi_question.to_vec().expect("serialize query"), vec![0, 14, 0xff], ]; for query in queries { let response = response( - resolve_query_with_lookup(&state, &query, None, |_| async { Ok(Vec::new()) }).await, + resolve_query_with_lookup( + &state, + &query, + /*client_addr*/ None, + |_| async { Ok(Vec::new()) }, + ) + .await, ); assert_eq!(response.response_code(), ResponseCode::Refused); assert_eq!(response.answers(), &[]); @@ -373,7 +378,7 @@ async fn denied_query_is_audited_and_reported_with_execution_attribution() { let (_, events) = capture_events(|| async { resolve_query_with_lookup( &attributed, - &query("denied.test", RecordType::A, 15), + &query("denied.test", RecordType::A, /*id*/ 15), Some("127.0.0.1:1234"), |_| async { Ok(Vec::new()) }, ) @@ -407,7 +412,7 @@ async fn listener_accepts_unscoped_policy_and_binds_attributed_execution() { Some("local".to_string()), )); - let unscoped = query_listener(addr, None, "127.0.0.1") + let unscoped = query_listener(addr, /*attribution_token*/ None, "127.0.0.1") .await .expect("unscoped DNS query"); assert_eq!(unscoped.response_code(), ResponseCode::NoError); diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 9aaa7de2c1..5cb38fd861 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -218,11 +218,8 @@ impl NetworkProxyBuilder { if self.managed_by_codex { let runtime = config::resolve_runtime(¤t_cfg)?; #[cfg(target_os = "windows")] - let (managed_http_addr, managed_socks_addr) = config::clamp_bind_addrs( - runtime.http_addr, - runtime.socks_addr, - ¤t_cfg, - ); + let (managed_http_addr, managed_socks_addr) = + config::clamp_bind_addrs(runtime.http_addr, runtime.socks_addr, ¤t_cfg); #[cfg(target_os = "windows")] let reserved = reserve_windows_managed_listeners( managed_http_addr, @@ -1300,7 +1297,8 @@ mod tests { "stale-token".to_string(), ), ]); - let global = proxy.prepare_for_optional_environment(HashMap::new(), None)?; + let global = + proxy.prepare_for_optional_environment(HashMap::new(), /*environment_id*/ None)?; let local = proxy.prepare_for_optional_environment(base_env.clone(), Some("local"))?; let remote = proxy.prepare_for_optional_environment(HashMap::new(), Some("remote"))?; let local_addrs = proxy.environment_proxy_addrs("local")?; diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index e6012e69f3..1b377456bb 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -546,11 +546,11 @@ impl NetworkProxyState { }; let (deny_set, allow_set, allow_local_binding, allowed_domains) = { let guard = self.state.read().await; - let allowed_domains = guard.config.network.allowed_domains(); + let allowed_domains = guard.config.allowed_domains(); ( guard.deny_set.clone(), guard.allow_set.clone(), - guard.config.network.allow_local_binding, + guard.config.allow_local_binding, allowed_domains, ) };