Merge branch 'main' into dev/mcgrew/network-otel-logs

codex-rs/network-proxy/README.md

@@ -3,7 +3,7 @@
 `codex-network-proxy` is Codex's local network policy enforcement proxy. It runs:
 
 - an HTTP proxy (default `127.0.0.1:3128`)
-- an optional SOCKS5 proxy (default `127.0.0.1:8081`, disabled by default)
+- a SOCKS5 proxy (default `127.0.0.1:8081`, enabled by default)
 - an admin HTTP API (default `127.0.0.1:8080`)
 
 It enforces an allow/deny policy and a "limited" mode intended for read-only network access.
@@ -26,14 +26,14 @@ Example config:
 enabled = true
 proxy_url = "http://127.0.0.1:3128"
 admin_url = "http://127.0.0.1:8080"
-# Optional SOCKS5 listener (disabled by default).
-enable_socks5 = false
+# SOCKS5 listener (enabled by default).
+enable_socks5 = true
 socks_url = "http://127.0.0.1:8081"
-enable_socks5_udp = false
+enable_socks5_udp = true
 # When `enabled` is false, the proxy no-ops and does not bind listeners.
 # When true, respect HTTP(S)_PROXY/ALL_PROXY for upstream requests (HTTP(S) proxies only),
 # including CONNECT tunnels in full mode.
-allow_upstream_proxy = false
+allow_upstream_proxy = true
 # By default, non-loopback binds are clamped to loopback for safety.
 # If you want to expose these listeners beyond localhost, you must opt in explicitly.
 dangerously_allow_non_loopback_proxy = false
@@ -42,13 +42,13 @@ mode = "full" # default when unset; use "limited" for read-only mode
 
 # Hosts must match the allowlist (unless denied).
 # If `allowed_domains` is empty, the proxy blocks requests until an allowlist is configured.
-allowed_domains = ["*.openai.com"]
+allowed_domains = ["*.openai.com", "localhost", "127.0.0.1", "::1"]
 denied_domains = ["evil.example"]
 
 # If false, local/private networking is rejected. Explicit allowlisting of local IP literals
 # (or `localhost`) is required to permit them.
 # Hostnames that resolve to local/private IPs are still blocked even if allowlisted.
-allow_local_binding = false
+allow_local_binding = true
 
 # macOS-only: allows proxying to a unix socket when request includes `x-unix-socket: /path`.
 allow_unix_sockets = ["/tmp/example.sock"]

codex-rs/network-proxy/src/config.rs

@@ -15,6 +15,7 @@ pub struct NetworkProxyConfig {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
+#[serde(default)]
 pub struct NetworkProxySettings {
     #[serde(default)]
     pub enabled: bool,
@@ -22,13 +23,10 @@ pub struct NetworkProxySettings {
     pub proxy_url: String,
     #[serde(default = "default_admin_url")]
     pub admin_url: String,
-    #[serde(default)]
     pub enable_socks5: bool,
     #[serde(default = "default_socks_url")]
     pub socks_url: String,
-    #[serde(default)]
     pub enable_socks5_udp: bool,
-    #[serde(default)]
     pub allow_upstream_proxy: bool,
     #[serde(default)]
     pub dangerously_allow_non_loopback_proxy: bool,
@@ -42,7 +40,6 @@ pub struct NetworkProxySettings {
     pub denied_domains: Vec<String>,
     #[serde(default)]
     pub allow_unix_sockets: Vec<String>,
-    #[serde(default)]
     pub allow_local_binding: bool,
 }
 
@@ -52,17 +49,17 @@ impl Default for NetworkProxySettings {
             enabled: false,
             proxy_url: default_proxy_url(),
             admin_url: default_admin_url(),
-            enable_socks5: false,
+            enable_socks5: true,
             socks_url: default_socks_url(),
-            enable_socks5_udp: false,
-            allow_upstream_proxy: false,
+            enable_socks5_udp: true,
+            allow_upstream_proxy: true,
             dangerously_allow_non_loopback_proxy: false,
             dangerously_allow_non_loopback_admin: false,
             mode: NetworkMode::default(),
             allowed_domains: Vec::new(),
             denied_domains: Vec::new(),
             allow_unix_sockets: Vec::new(),
-            allow_local_binding: false,
+            allow_local_binding: true,
         }
     }
 }
@@ -329,6 +326,47 @@ mod tests {
 
     use pretty_assertions::assert_eq;
 
+    #[test]
+    fn network_proxy_settings_default_matches_local_use_baseline() {
+        assert_eq!(
+            NetworkProxySettings::default(),
+            NetworkProxySettings {
+                enabled: false,
+                proxy_url: "http://127.0.0.1:3128".to_string(),
+                admin_url: "http://127.0.0.1:8080".to_string(),
+                enable_socks5: true,
+                socks_url: "http://127.0.0.1:8081".to_string(),
+                enable_socks5_udp: true,
+                allow_upstream_proxy: true,
+                dangerously_allow_non_loopback_proxy: false,
+                dangerously_allow_non_loopback_admin: false,
+                mode: NetworkMode::Full,
+                allowed_domains: Vec::new(),
+                denied_domains: Vec::new(),
+                allow_unix_sockets: Vec::new(),
+                allow_local_binding: true,
+            }
+        );
+    }
+
+    #[test]
+    fn partial_network_config_uses_struct_defaults_for_missing_fields() {
+        let config: NetworkProxyConfig = serde_json::from_str(
+            r#"{
+                "network": {
+                    "enabled": true
+                }
+            }"#,
+        )
+        .unwrap();
+        let expected = NetworkProxySettings {
+            enabled: true,
+            ..NetworkProxySettings::default()
+        };
+
+        assert_eq!(config.network, expected);
+    }
+
     #[test]
     fn parse_host_port_defaults_for_empty_string() {
         assert!(parse_host_port("", 1234).is_err());

codex-rs/network-proxy/src/proxy.rs

@@ -29,10 +29,10 @@ struct ReservedListeners {
 }
 
 impl ReservedListeners {
-    fn new(http: StdTcpListener, socks: StdTcpListener, admin: StdTcpListener) -> Self {
+    fn new(http: StdTcpListener, socks: Option<StdTcpListener>, admin: StdTcpListener) -> Self {
         Self {
             http: Mutex::new(Some(http)),
-            socks: Mutex::new(Some(socks)),
+            socks: Mutex::new(socks),
             admin: Mutex::new(Some(admin)),
         }
     }
@@ -133,15 +133,20 @@ impl NetworkProxyBuilder {
         let current_cfg = state.current_cfg().await?;
         let (requested_http_addr, requested_socks_addr, requested_admin_addr, reserved_listeners) =
             if self.managed_by_codex {
+                let runtime = config::resolve_runtime(&current_cfg)?;
                 let (http_listener, socks_listener, admin_listener) =
-                    reserve_loopback_ephemeral_listeners()
+                    reserve_loopback_ephemeral_listeners(current_cfg.network.enable_socks5)
                         .context("reserve managed loopback proxy listeners")?;
                 let http_addr = http_listener
                     .local_addr()
                     .context("failed to read reserved HTTP proxy address")?;
-                let socks_addr = socks_listener
-                    .local_addr()
-                    .context("failed to read reserved SOCKS5 proxy address")?;
+                let socks_addr = if let Some(socks_listener) = socks_listener.as_ref() {
+                    socks_listener
+                        .local_addr()
+                        .context("failed to read reserved SOCKS5 proxy address")?
+                } else {
+                    runtime.socks_addr
+                };
                 let admin_addr = admin_listener
                     .local_addr()
                     .context("failed to read reserved admin API address")?;
@@ -186,13 +191,19 @@ impl NetworkProxyBuilder {
     }
 }
 
-fn reserve_loopback_ephemeral_listeners() -> Result<(StdTcpListener, StdTcpListener, StdTcpListener)>
-{
-    Ok((
-        reserve_loopback_ephemeral_listener().context("reserve HTTP proxy listener")?,
-        reserve_loopback_ephemeral_listener().context("reserve SOCKS5 proxy listener")?,
-        reserve_loopback_ephemeral_listener().context("reserve admin API listener")?,
-    ))
+fn reserve_loopback_ephemeral_listeners(
+    reserve_socks_listener: bool,
+) -> Result<(StdTcpListener, Option<StdTcpListener>, StdTcpListener)> {
+    let http_listener =
+        reserve_loopback_ephemeral_listener().context("reserve HTTP proxy listener")?;
+    let socks_listener = if reserve_socks_listener {
+        Some(reserve_loopback_ephemeral_listener().context("reserve SOCKS5 proxy listener")?)
+    } else {
+        None
+    };
+    let admin_listener =
+        reserve_loopback_ephemeral_listener().context("reserve admin API listener")?;
+    Ok((http_listener, socks_listener, admin_listener))
 }
 
 fn reserve_loopback_ephemeral_listener() -> Result<StdTcpListener> {
@@ -612,6 +623,43 @@ mod tests {
         );
     }
 
+    #[tokio::test]
+    async fn managed_proxy_builder_does_not_reserve_socks_listener_when_disabled() {
+        let settings = NetworkProxySettings {
+            enable_socks5: false,
+            socks_url: "http://127.0.0.1:43129".to_string(),
+            ..NetworkProxySettings::default()
+        };
+        let state = Arc::new(network_proxy_state_for_policy(settings));
+        let proxy = match NetworkProxy::builder().state(state).build().await {
+            Ok(proxy) => proxy,
+            Err(err) => {
+                if err
+                    .chain()
+                    .any(|cause| cause.to_string().contains("Operation not permitted"))
+                {
+                    return;
+                }
+                panic!("failed to build managed proxy: {err:#}");
+            }
+        };
+
+        assert!(proxy.http_addr.ip().is_loopback());
+        assert!(proxy.admin_addr.ip().is_loopback());
+        assert_eq!(
+            proxy.socks_addr,
+            "127.0.0.1:43129".parse::<SocketAddr>().unwrap()
+        );
+        assert!(
+            proxy
+                .reserved_listeners
+                .as_ref()
+                .expect("managed builder should reserve listeners")
+                .take_socks()
+                .is_none()
+        );
+    }
+
     #[test]
     fn proxy_url_env_value_resolves_lowercase_aliases() {
         let mut env = HashMap::new();