diff --git a/codex-rs/network-proxy/src/runtime.rs b/codex-rs/network-proxy/src/runtime.rs index 9381fab7f..ec43a5479 100644 --- a/codex-rs/network-proxy/src/runtime.rs +++ b/codex-rs/network-proxy/src/runtime.rs @@ -698,12 +698,22 @@ async fn host_resolves_to_non_public_ip(host: &str, port: u16) -> bool { return is_non_public_ip(ip); } - // If DNS lookup fails, default to "not local/private" rather than blocking. In practice, the - // subsequent connect attempt will fail anyway, and blocking on transient resolver issues would - // make the proxy fragile. The allowlist/denylist remains the primary control plane. + // Block the request if this DNS lookup fails. We resolve the hostname again when we connect, + // so a failed check here does not prove the destination is public. let addrs = match timeout(DNS_LOOKUP_TIMEOUT, lookup_host((host, port))).await { Ok(Ok(addrs)) => addrs, - Ok(Err(_)) | Err(_) => return false, + Ok(Err(err)) => { + debug!( + "blocking host because DNS lookup failed during local/private IP check (host={host}, port={port}): {err}" + ); + return true; + } + Err(_) => { + debug!( + "blocking host because DNS lookup timed out during local/private IP check (host={host}, port={port})" + ); + return true; + } }; for addr in addrs { @@ -903,17 +913,18 @@ mod tests { }); assert_eq!( - state.host_blocked("evil.example", 80).await.unwrap(), + // Use a public IP literal to avoid relying on ambient DNS behavior. + state.host_blocked("8.8.8.8", 80).await.unwrap(), HostBlockDecision::Allowed ); - state.add_denied_domain("evil.example").await.unwrap(); + state.add_denied_domain("8.8.8.8").await.unwrap(); let (allowed, denied) = state.current_patterns().await.unwrap(); assert_eq!(allowed, vec!["*".to_string()]); - assert_eq!(denied, vec!["evil.example".to_string()]); + assert_eq!(denied, vec!["8.8.8.8".to_string()]); assert_eq!( - state.host_blocked("evil.example", 80).await.unwrap(), + state.host_blocked("8.8.8.8", 80).await.unwrap(), HostBlockDecision::Blocked(HostBlockReason::Denied) ); } @@ -1237,6 +1248,23 @@ mod tests { ); } + #[tokio::test] + async fn host_blocked_rejects_allowlisted_hostname_when_dns_lookup_fails() { + let state = network_proxy_state_for_policy(NetworkProxySettings { + allowed_domains: vec!["does-not-resolve.invalid".to_string()], + allow_local_binding: false, + ..NetworkProxySettings::default() + }); + + assert_eq!( + state + .host_blocked("does-not-resolve.invalid", 80) + .await + .unwrap(), + HostBlockDecision::Blocked(HostBlockReason::NotAllowedLocal) + ); + } + #[test] fn validate_policy_against_constraints_disallows_widening_allowed_domains() { let constraints = NetworkProxyConstraints {