From aea82c63eac85f5d5fd85de5ac79a0b1ff8369d4 Mon Sep 17 00:00:00 2001
From: viyatb-oai <viyatb@openai.com>
Date: Thu, 26 Mar 2026 16:18:04 -0700
Subject: [PATCH] fix(network-proxy): fail closed on network-proxy DNS lookup
 errors (#15909)

## Summary

Fail closed when the network proxy's local/private IP pre-check hits a
DNS lookup error or timeout, instead of treating the hostname as public
and allowing the request.

## Root cause

`host_resolves_to_non_public_ip()` returned `false` on resolver failure,
which created a fail-open path in the `allow_local_binding = false`
boundary. The eventual connect path performs its own DNS resolution
later, so a transient pre-check failure is not evidence that the
destination is public.

## Changes

- Treat DNS lookup errors/timeouts as local/private for blocking
purposes
- Add a regression test for an allowlisted hostname that fails DNS
resolution

## Validation

- `cargo test -p codex-network-proxy`
- `cargo clippy -p codex-network-proxy --all-targets -- -D warnings`
- `just fmt`
- `just argument-comment-lint`
---
 codex-rs/network-proxy/src/runtime.rs | 44 ++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 8 deletions(-)

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 {