diff --git a/codex-rs/cli/src/debug_sandbox.rs b/codex-rs/cli/src/debug_sandbox.rs index e5cccfc53..f64c45c73 100644 --- a/codex-rs/cli/src/debug_sandbox.rs +++ b/codex-rs/cli/src/debug_sandbox.rs @@ -164,14 +164,17 @@ async fn run_command_under_sandbox( let res = tokio::task::spawn_blocking(move || { if use_elevated { run_windows_sandbox_capture_elevated( - policy_str.as_str(), - &sandbox_cwd, - base_dir.as_path(), - command_vec, - &cwd_clone, - env_map, - /*timeout_ms*/ None, - config.permissions.windows_sandbox_private_desktop, + codex_windows_sandbox::ElevatedSandboxCaptureRequest { + policy_json_or_preset: policy_str.as_str(), + sandbox_policy_cwd: &sandbox_cwd, + codex_home: base_dir.as_path(), + command: command_vec, + cwd: &cwd_clone, + env_map, + timeout_ms: None, + use_private_desktop: config.permissions.windows_sandbox_private_desktop, + proxy_enforced: false, + }, ) } else { run_windows_sandbox_capture( diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 11d8600ba..fde8782f0 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -478,7 +478,11 @@ async fn exec_windows_sandbox( })?; let command_path = command.first().cloned(); let sandbox_level = windows_sandbox_level; - let use_elevated = matches!(sandbox_level, WindowsSandboxLevel::Elevated); + let proxy_enforced = network.is_some(); + // Windows firewall enforcement is tied to the logon-user sandbox identities, so + // proxy-enforced sessions must use that backend even when the configured mode is + // the default restricted-token sandbox. + let use_elevated = proxy_enforced || matches!(sandbox_level, WindowsSandboxLevel::Elevated); let additional_deny_write_paths = windows_restricted_token_filesystem_overlay .map(|overlay| { overlay @@ -491,14 +495,17 @@ async fn exec_windows_sandbox( let spawn_res = tokio::task::spawn_blocking(move || { if use_elevated { run_windows_sandbox_capture_elevated( - policy_str.as_str(), - &sandbox_cwd, - codex_home.as_ref(), - command, - &cwd, - env, - timeout_ms, - windows_sandbox_private_desktop, + codex_windows_sandbox::ElevatedSandboxCaptureRequest { + policy_json_or_preset: policy_str.as_str(), + sandbox_policy_cwd: &sandbox_cwd, + codex_home: codex_home.as_ref(), + command, + cwd: &cwd, + env_map: env, + timeout_ms, + use_private_desktop: windows_sandbox_private_desktop, + proxy_enforced, + }, ) } else { run_windows_sandbox_capture_with_extra_deny_write_paths( diff --git a/codex-rs/core/src/windows_sandbox.rs b/codex-rs/core/src/windows_sandbox.rs index 1fdf3ee33..ee3a040a6 100644 --- a/codex-rs/core/src/windows_sandbox.rs +++ b/codex-rs/core/src/windows_sandbox.rs @@ -180,13 +180,15 @@ pub fn run_elevated_setup( codex_home: &Path, ) -> anyhow::Result<()> { codex_windows_sandbox::run_elevated_setup( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - /*read_roots_override*/ None, - /*write_roots_override*/ None, + codex_windows_sandbox::SandboxSetupRequest { + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced: false, + }, + codex_windows_sandbox::SetupRootOverrides::default(), ) } @@ -234,6 +236,7 @@ pub fn run_setup_refresh_with_extra_read_roots( env_map, codex_home, extra_read_roots, + /*proxy_enforced*/ false, ) } diff --git a/codex-rs/network-proxy/src/proxy.rs b/codex-rs/network-proxy/src/proxy.rs index 8f596f684..3eecef3ee 100644 --- a/codex-rs/network-proxy/src/proxy.rs +++ b/codex-rs/network-proxy/src/proxy.rs @@ -51,6 +51,43 @@ impl ReservedListeners { } } +struct ReservedListenerSet { + http_listener: StdTcpListener, + socks_listener: Option, +} + +impl ReservedListenerSet { + fn new(http_listener: StdTcpListener, socks_listener: Option) -> Self { + Self { + http_listener, + socks_listener, + } + } + + fn http_addr(&self) -> Result { + self.http_listener + .local_addr() + .context("failed to read reserved HTTP proxy address") + } + + fn socks_addr(&self, default_addr: SocketAddr) -> Result { + self.socks_listener + .as_ref() + .map_or(Ok(default_addr), |listener| { + listener + .local_addr() + .context("failed to read reserved SOCKS5 proxy address") + }) + } + + fn into_reserved_listeners(self) -> Arc { + Arc::new(ReservedListeners::new( + self.http_listener, + self.socks_listener, + )) + } +} + #[derive(Clone)] pub struct NetworkProxyBuilder { state: Option>, @@ -134,38 +171,41 @@ impl NetworkProxyBuilder { .set_blocked_request_observer(self.blocked_request_observer.clone()) .await; let current_cfg = state.current_cfg().await?; - let (requested_http_addr, requested_socks_addr, reserved_listeners) = - if self.managed_by_codex { - let runtime = config::resolve_runtime(¤t_cfg)?; - let (http_listener, socks_listener) = - 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 = 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 - }; - ( - http_addr, - socks_addr, - Some(Arc::new(ReservedListeners::new( - http_listener, - socks_listener, - ))), - ) - } else { - let runtime = config::resolve_runtime(¤t_cfg)?; - ( - self.http_addr.unwrap_or(runtime.http_addr), - self.socks_addr.unwrap_or(runtime.socks_addr), - None, - ) - }; + let (requested_http_addr, requested_socks_addr, reserved_listeners) = 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.network, + ); + #[cfg(target_os = "windows")] + let reserved = reserve_windows_managed_listeners( + managed_http_addr, + managed_socks_addr, + current_cfg.network.enable_socks5, + ) + .context("reserve managed loopback proxy listeners")?; + #[cfg(not(target_os = "windows"))] + let reserved = reserve_loopback_ephemeral_listeners(current_cfg.network.enable_socks5) + .context("reserve managed loopback proxy listeners")?; + let http_addr = reserved.http_addr()?; + let socks_addr = reserved.socks_addr(runtime.socks_addr)?; + ( + http_addr, + socks_addr, + Some(reserved.into_reserved_listeners()), + ) + } else { + let runtime = config::resolve_runtime(¤t_cfg)?; + ( + self.http_addr.unwrap_or(runtime.http_addr), + self.socks_addr.unwrap_or(runtime.socks_addr), + None, + ) + }; // Reapply bind clamping for caller overrides so unix-socket proxying stays loopback-only. let (http_addr, socks_addr) = config::clamp_bind_addrs( @@ -192,7 +232,7 @@ impl NetworkProxyBuilder { fn reserve_loopback_ephemeral_listeners( reserve_socks_listener: bool, -) -> Result<(StdTcpListener, Option)> { +) -> Result { let http_listener = reserve_loopback_ephemeral_listener().context("reserve HTTP proxy listener")?; let socks_listener = if reserve_socks_listener { @@ -200,7 +240,53 @@ fn reserve_loopback_ephemeral_listeners( } else { None }; - Ok((http_listener, socks_listener)) + Ok(ReservedListenerSet::new(http_listener, socks_listener)) +} + +#[cfg(target_os = "windows")] +fn reserve_windows_managed_listeners( + http_addr: SocketAddr, + socks_addr: SocketAddr, + reserve_socks_listener: bool, +) -> Result { + let http_addr = windows_managed_loopback_addr(http_addr); + let socks_addr = windows_managed_loopback_addr(socks_addr); + + match try_reserve_windows_managed_listeners(http_addr, socks_addr, reserve_socks_listener) { + Ok(listeners) => Ok(listeners), + Err(err) if err.kind() == std::io::ErrorKind::AddrInUse => { + warn!("managed Windows proxy ports are busy; falling back to ephemeral loopback ports"); + reserve_loopback_ephemeral_listeners(reserve_socks_listener) + .context("reserve fallback loopback proxy listeners") + } + Err(err) => Err(err).context("reserve Windows managed proxy listeners"), + } +} + +#[cfg(target_os = "windows")] +fn try_reserve_windows_managed_listeners( + http_addr: SocketAddr, + socks_addr: SocketAddr, + reserve_socks_listener: bool, +) -> std::io::Result { + let http_listener = StdTcpListener::bind(http_addr)?; + let socks_listener = if reserve_socks_listener { + Some(StdTcpListener::bind(socks_addr)?) + } else { + None + }; + Ok(ReservedListenerSet::new(http_listener, socks_listener)) +} + +#[cfg(target_os = "windows")] +fn windows_managed_loopback_addr(addr: SocketAddr) -> SocketAddr { + if !addr.ip().is_loopback() { + warn!( + "managed Windows proxies must bind to loopback; clamping {addr} to 127.0.0.1:{}", + addr.port() + ); + } + SocketAddr::from(([127, 0, 0, 1], addr.port())) } fn reserve_loopback_ephemeral_listener() -> Result { @@ -570,10 +656,12 @@ mod tests { use std::net::Ipv4Addr; #[tokio::test] - async fn managed_proxy_builder_uses_loopback_ephemeral_ports() { - let state = Arc::new(network_proxy_state_for_policy( - NetworkProxySettings::default(), - )); + async fn managed_proxy_builder_uses_loopback_ports() { + let state = Arc::new(network_proxy_state_for_policy(NetworkProxySettings { + proxy_url: "http://127.0.0.1:43128".to_string(), + socks_url: "http://127.0.0.1:48081".to_string(), + ..NetworkProxySettings::default() + })); let proxy = match NetworkProxy::builder().state(state).build().await { Ok(proxy) => proxy, Err(err) => { @@ -589,8 +677,22 @@ mod tests { assert!(proxy.http_addr.ip().is_loopback()); assert!(proxy.socks_addr.ip().is_loopback()); - assert_ne!(proxy.http_addr.port(), 0); - assert_ne!(proxy.socks_addr.port(), 0); + #[cfg(target_os = "windows")] + { + assert_eq!( + proxy.http_addr, + "127.0.0.1:43128".parse::().unwrap() + ); + assert_eq!( + proxy.socks_addr, + "127.0.0.1:48081".parse::().unwrap() + ); + } + #[cfg(not(target_os = "windows"))] + { + assert_ne!(proxy.http_addr.port(), 0); + assert_ne!(proxy.socks_addr.port(), 0); + } } #[tokio::test] @@ -622,6 +724,7 @@ mod tests { async fn managed_proxy_builder_does_not_reserve_socks_listener_when_disabled() { let settings = NetworkProxySettings { enable_socks5: false, + proxy_url: "http://127.0.0.1:43128".to_string(), socks_url: "http://127.0.0.1:43129".to_string(), ..NetworkProxySettings::default() }; @@ -640,6 +743,7 @@ mod tests { }; assert!(proxy.http_addr.ip().is_loopback()); + assert_ne!(proxy.http_addr.port(), 0); assert_eq!( proxy.socks_addr, "127.0.0.1:43129".parse::().unwrap() @@ -654,6 +758,47 @@ mod tests { ); } + #[cfg(target_os = "windows")] + #[test] + fn windows_managed_loopback_addr_clamps_non_loopback_inputs() { + assert_eq!( + windows_managed_loopback_addr("0.0.0.0:3128".parse::().unwrap()), + "127.0.0.1:3128".parse::().unwrap() + ); + assert_eq!( + windows_managed_loopback_addr("[::]:8081".parse::().unwrap()), + "127.0.0.1:8081".parse::().unwrap() + ); + } + + #[cfg(target_os = "windows")] + #[test] + fn reserve_windows_managed_listeners_falls_back_when_http_port_is_busy() { + let occupied = StdTcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))).unwrap(); + let busy_port = occupied.local_addr().unwrap().port(); + + let reserved = reserve_windows_managed_listeners( + SocketAddr::from(([127, 0, 0, 1], busy_port)), + SocketAddr::from(([127, 0, 0, 1], 48081)), + false, + ) + .unwrap(); + + assert!(reserved.socks_listener.is_none()); + assert!( + reserved + .http_listener + .local_addr() + .unwrap() + .ip() + .is_loopback() + ); + assert_ne!( + reserved.http_listener.local_addr().unwrap().port(), + busy_port + ); + } + #[test] fn proxy_url_env_value_resolves_lowercase_aliases() { let mut env = HashMap::new(); diff --git a/codex-rs/windows-sandbox-rs/sandbox_smoketests.py b/codex-rs/windows-sandbox-rs/sandbox_smoketests.py index 0fc049d4e..f629a4c6d 100644 --- a/codex-rs/windows-sandbox-rs/sandbox_smoketests.py +++ b/codex-rs/windows-sandbox-rs/sandbox_smoketests.py @@ -6,8 +6,13 @@ import os import sys import shutil import subprocess +import contextlib +import http.client +import http.server +import threading from pathlib import Path from typing import List, Optional, Tuple +from urllib.parse import urlsplit def _resolve_codex_cmd() -> List[str]: """Resolve the Codex CLI to invoke `codex sandbox windows`. @@ -135,6 +140,68 @@ def make_symlink(link: Path, target: Path) -> bool: cp = subprocess.run(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) return cp.returncode == 0 and link.exists() +class _QuietHandler(http.server.BaseHTTPRequestHandler): + def log_message(self, format, *args): + pass + +class _TargetHandler(_QuietHandler): + def do_GET(self): + body = b"proxy-ok" + self.send_response(200) + self.send_header("Content-Type", "text/plain") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + +class _ProxyHandler(_QuietHandler): + def do_GET(self): + parsed = urlsplit(self.path) + if not parsed.scheme or not parsed.hostname: + self.send_error(400, "absolute URL required") + return + if parsed.hostname not in ("127.0.0.1", "localhost"): + self.send_error(403, "only loopback hosts are allowed in smoke proxy") + return + path = parsed.path or "/" + if parsed.query: + path = f"{path}?{parsed.query}" + conn = None + try: + conn = http.client.HTTPConnection(parsed.hostname, parsed.port or 80, timeout=2) + conn.request("GET", path) + upstream = conn.getresponse() + body = upstream.read() + except Exception as err: + self.send_error(502, f"proxy upstream error: {err}") + return + finally: + if conn is not None: + with contextlib.suppress(Exception): + conn.close() + self.send_response(upstream.status, upstream.reason) + self.send_header("Content-Type", "text/plain") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + +@contextlib.contextmanager +def start_loopback_proxy_fixture(): + target = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _TargetHandler) + proxy = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _ProxyHandler) + target_port = target.server_address[1] + proxy_port = proxy.server_address[1] + target_thread = threading.Thread(target=target.serve_forever, daemon=True) + proxy_thread = threading.Thread(target=proxy.serve_forever, daemon=True) + target_thread.start() + proxy_thread.start() + try: + yield target_port, proxy_port + finally: + proxy.shutdown() + target.shutdown() + proxy.server_close() + target.server_close() + def summarize(results: List[CaseResult]) -> int: ok = sum(1 for r in results if r.ok) total = len(results) @@ -278,7 +345,65 @@ def main() -> int: "try { iwr http://neverssl.com -TimeoutSec 2 } catch { exit 1 }"], WS_ROOT) add("WS: iwr network blocked", rc != 0, f"rc={rc}") - # 17. RO: deny TEMP writes via PowerShell + # 17. WS: direct loopback blocked, proxy loopback allowed via env proxy + if have("curl"): + with start_loopback_proxy_fixture() as (target_port, proxy_port): + proxy_home = WS_ROOT / ".codex_proxy_smoke" + remove_if_exists(proxy_home) + proxy_home.mkdir(parents=True, exist_ok=True) + proxy_url = f"http://127.0.0.1:{proxy_port}" + proxy_env = { + "CODEX_HOME": str(proxy_home), + "HTTP_PROXY": proxy_url, + "http_proxy": proxy_url, + "ALL_PROXY": proxy_url, + "all_proxy": proxy_url, + "NO_PROXY": "", + "no_proxy": "", + } + proxied_cmd = [ + "curl", + "--noproxy", + "", + "--connect-timeout", + "2", + "--max-time", + "4", + f"http://127.0.0.1:{target_port}/proxied", + ] + rc_proxy, out_proxy, err_proxy = run_sbx( + "workspace-write", + proxied_cmd, + WS_ROOT, + env_extra=proxy_env, + ) + add( + "WS: loopback proxy allowed", + rc_proxy == 0 and "proxy-ok" in out_proxy, + f"rc={rc_proxy}, out={out_proxy}, err={err_proxy}", + ) + + direct_cmd = [ + "curl", + "--noproxy", + "*", + "--connect-timeout", + "1", + "--max-time", + "2", + f"http://127.0.0.1:{target_port}/direct", + ] + rc_direct, _out_direct, err_direct = run_sbx( + "workspace-write", + direct_cmd, + WS_ROOT, + env_extra={"CODEX_HOME": str(proxy_home)}, + ) + add("WS: direct loopback blocked", rc_direct != 0, f"rc={rc_direct}, err={err_direct}") + else: + add("WS: direct/proxy loopback tests (curl missing)", True, "curl not installed") + + # 18. RO: deny TEMP writes via PowerShell rc, out, err = run_sbx("read-only", ["powershell", "-NoLogo", "-NoProfile", "-Command", "Set-Content -LiteralPath $env:TEMP\\ro_tmpfail.txt -Value 'x'"], WS_ROOT) @@ -287,21 +412,21 @@ def main() -> int: else: add("RO: TEMP write denied (PS, skipped)", True) - # 18. WS: curl version check — don't rely on stub, just succeed + # 19. WS: curl version check — don't rely on stub, just succeed if have("curl"): rc, out, err = run_sbx("workspace-write", ["cmd", "/c", "curl --version"], WS_ROOT) add("WS: curl present (version prints)", rc == 0, f"rc={rc}, err={err}") else: add("WS: curl present (optional, skipped)", True) - # 19. Optional: ripgrep version + # 20. Optional: ripgrep version if have("rg"): rc, out, err = run_sbx("workspace-write", ["cmd", "/c", "rg --version"], WS_ROOT) add("WS: rg --version (optional)", rc == 0, f"rc={rc}, err={err}") else: add("WS: rg --version (optional, skipped)", True) - # 20. Optional: git --version + # 21. Optional: git --version if have("git"): rc, out, err = run_sbx("workspace-write", ["git", "--version"], WS_ROOT) add("WS: git --version (optional)", rc == 0, f"rc={rc}, err={err}") diff --git a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs index f6c4563e1..365ac37d6 100644 --- a/codex-rs/windows-sandbox-rs/src/elevated_impl.rs +++ b/codex-rs/windows-sandbox-rs/src/elevated_impl.rs @@ -1,4 +1,20 @@ +use std::collections::HashMap; +use std::path::Path; + +pub struct ElevatedSandboxCaptureRequest<'a> { + pub policy_json_or_preset: &'a str, + pub sandbox_policy_cwd: &'a Path, + pub codex_home: &'a Path, + pub command: Vec, + pub cwd: &'a Path, + pub env_map: HashMap, + pub timeout_ms: Option, + pub use_private_desktop: bool, + pub proxy_enforced: bool, +} + mod windows_impl { + use super::ElevatedSandboxCaptureRequest; use crate::acl::allow_null_device; use crate::allow::compute_allow_paths; use crate::allow::AllowDenyPaths; @@ -203,15 +219,19 @@ mod windows_impl { /// Launches the command runner under the sandbox user and captures its output. #[allow(clippy::too_many_arguments)] pub fn run_windows_sandbox_capture( - policy_json_or_preset: &str, - sandbox_policy_cwd: &Path, - codex_home: &Path, - command: Vec, - cwd: &Path, - mut env_map: HashMap, - timeout_ms: Option, - use_private_desktop: bool, + request: ElevatedSandboxCaptureRequest<'_>, ) -> Result { + let ElevatedSandboxCaptureRequest { + policy_json_or_preset, + sandbox_policy_cwd, + codex_home, + command, + cwd, + mut env_map, + timeout_ms, + use_private_desktop, + proxy_enforced, + } = request; let policy = parse_policy(policy_json_or_preset)?; normalize_null_device_env(&mut env_map); ensure_non_interactive_pager(&mut env_map); @@ -224,8 +244,14 @@ mod windows_impl { let logs_base_dir: Option<&Path> = Some(sandbox_base.as_path()); log_start(&command, logs_base_dir); - let sandbox_creds = - require_logon_sandbox_creds(&policy, sandbox_policy_cwd, cwd, &env_map, codex_home)?; + let sandbox_creds = require_logon_sandbox_creds( + &policy, + sandbox_policy_cwd, + cwd, + &env_map, + codex_home, + proxy_enforced, + )?; let sandbox_sid = resolve_sid(&sandbox_creds.username).map_err(|err: anyhow::Error| { io::Error::new(io::ErrorKind::PermissionDenied, err.to_string()) })?; @@ -480,11 +506,9 @@ pub use windows_impl::run_windows_sandbox_capture; #[cfg(not(target_os = "windows"))] mod stub { + use super::ElevatedSandboxCaptureRequest; use anyhow::bail; use anyhow::Result; - use codex_protocol::protocol::SandboxPolicy; - use std::collections::HashMap; - use std::path::Path; #[derive(Debug, Default)] pub struct CaptureResult { @@ -497,14 +521,7 @@ mod stub { /// Stub implementation for non-Windows targets; sandboxing only works on Windows. #[allow(clippy::too_many_arguments)] pub fn run_windows_sandbox_capture( - _policy_json_or_preset: &str, - _sandbox_policy_cwd: &Path, - _codex_home: &Path, - _command: Vec, - _cwd: &Path, - _env_map: HashMap, - _timeout_ms: Option, - _use_private_desktop: bool, + _request: ElevatedSandboxCaptureRequest<'_>, ) -> Result { bail!("Windows sandbox is only available on Windows") } diff --git a/codex-rs/windows-sandbox-rs/src/firewall.rs b/codex-rs/windows-sandbox-rs/src/firewall.rs index b4939fd42..a95d3b57e 100644 --- a/codex-rs/windows-sandbox-rs/src/firewall.rs +++ b/codex-rs/windows-sandbox-rs/src/firewall.rs @@ -9,10 +9,13 @@ use windows::core::BSTR; use windows::Win32::Foundation::VARIANT_TRUE; use windows::Win32::NetworkManagement::WindowsFirewall::INetFwPolicy2; use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRule3; +use windows::Win32::NetworkManagement::WindowsFirewall::INetFwRules; use windows::Win32::NetworkManagement::WindowsFirewall::NetFwPolicy2; use windows::Win32::NetworkManagement::WindowsFirewall::NetFwRule; use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_ACTION_BLOCK; use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_ANY; +use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_TCP; +use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_IP_PROTOCOL_UDP; use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_PROFILE2_ALL; use windows::Win32::NetworkManagement::WindowsFirewall::NET_FW_RULE_DIR_OUT; use windows::Win32::System::Com::CoCreateInstance; @@ -27,9 +30,128 @@ use codex_windows_sandbox::SetupFailure; // This is the stable identifier we use to find/update the rule idempotently. // It intentionally does not change between installs. const OFFLINE_BLOCK_RULE_NAME: &str = "codex_sandbox_offline_block_outbound"; +const OFFLINE_BLOCK_LOOPBACK_TCP_RULE_NAME: &str = "codex_sandbox_offline_block_loopback_tcp"; +const OFFLINE_BLOCK_LOOPBACK_UDP_RULE_NAME: &str = "codex_sandbox_offline_block_loopback_udp"; // Friendly text shown in the firewall UI. -const OFFLINE_BLOCK_RULE_FRIENDLY: &str = "Codex Sandbox Offline - Block Outbound"; +const OFFLINE_BLOCK_RULE_FRIENDLY: &str = "Codex Sandbox Offline - Block Non-Loopback Outbound"; +const OFFLINE_BLOCK_LOOPBACK_TCP_RULE_FRIENDLY: &str = + "Codex Sandbox Offline - Block Loopback TCP (Except Proxy)"; +const OFFLINE_BLOCK_LOOPBACK_UDP_RULE_FRIENDLY: &str = "Codex Sandbox Offline - Block Loopback UDP"; +const OFFLINE_PROXY_ALLOW_RULE_NAME: &str = "codex_sandbox_offline_allow_loopback_proxy"; +const LOOPBACK_REMOTE_ADDRESSES: &str = "127.0.0.0/8,::1"; +const NON_LOOPBACK_REMOTE_ADDRESSES: &str = + "0.0.0.0-126.255.255.255,128.0.0.0-255.255.255.255,::,::2-ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff"; + +struct BlockRuleSpec<'a> { + internal_name: &'a str, + friendly_desc: &'a str, + protocol: i32, + local_user_spec: &'a str, + offline_sid: &'a str, + remote_addresses: Option<&'a str>, + remote_ports: Option<&'a str>, +} + +pub fn ensure_offline_proxy_allowlist( + offline_sid: &str, + proxy_ports: &[u16], + allow_local_binding: bool, + log: &mut File, +) -> Result<()> { + let local_user_spec = format!("O:LSD:(A;;CC;;;{offline_sid})"); + + let hr = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) }; + if hr.is_err() { + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallComInitFailed, + format!("CoInitializeEx failed: {hr:?}"), + ))); + } + + let result = unsafe { + (|| -> Result<()> { + let policy: INetFwPolicy2 = CoCreateInstance(&NetFwPolicy2, None, CLSCTX_INPROC_SERVER) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallPolicyAccessFailed, + format!("CoCreateInstance NetFwPolicy2 failed: {err:?}"), + )) + })?; + let rules = policy.Rules().map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallPolicyAccessFailed, + format!("INetFwPolicy2::Rules failed: {err:?}"), + )) + })?; + + if allow_local_binding { + // Remove the legacy overlapping allow rule before returning to the local-binding + // mode so stale proxy exceptions do not linger. + remove_rule_if_present(&rules, OFFLINE_PROXY_ALLOW_RULE_NAME, log)?; + remove_rule_if_present(&rules, OFFLINE_BLOCK_LOOPBACK_UDP_RULE_NAME, log)?; + remove_rule_if_present(&rules, OFFLINE_BLOCK_LOOPBACK_TCP_RULE_NAME, log)?; + return Ok(()); + } + + ensure_block_rule( + &rules, + &BlockRuleSpec { + internal_name: OFFLINE_BLOCK_LOOPBACK_UDP_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_LOOPBACK_UDP_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_UDP.0, + local_user_spec: &local_user_spec, + offline_sid, + remote_addresses: Some(LOOPBACK_REMOTE_ADDRESSES), + remote_ports: None, + }, + log, + )?; + + // Install a broad TCP loopback block before narrowing it to the allowed proxy port + // complement. If the narrowing update fails, the sandbox remains fail-closed. + ensure_block_rule( + &rules, + &BlockRuleSpec { + internal_name: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_TCP.0, + local_user_spec: &local_user_spec, + offline_sid, + remote_addresses: Some(LOOPBACK_REMOTE_ADDRESSES), + remote_ports: None, + }, + log, + )?; + + // Remove the legacy overlapping allow rule only after the explicit block rules are in + // place so transitions back to proxy-only mode do not fail open. + remove_rule_if_present(&rules, OFFLINE_PROXY_ALLOW_RULE_NAME, log)?; + + if let Some(blocked_remote_ports) = blocked_loopback_tcp_remote_ports(proxy_ports) { + ensure_block_rule( + &rules, + &BlockRuleSpec { + internal_name: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_LOOPBACK_TCP_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_TCP.0, + local_user_spec: &local_user_spec, + offline_sid, + remote_addresses: Some(LOOPBACK_REMOTE_ADDRESSES), + remote_ports: Some(&blocked_remote_ports), + }, + log, + )?; + } + Ok(()) + })() + }; + + unsafe { + CoUninitialize(); + } + result +} pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Result<()> { let local_user_spec = format!("O:LSD:(A;;CC;;;{offline_sid})"); @@ -61,11 +183,15 @@ pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Resul // Block all outbound IP protocols for this user. ensure_block_rule( &rules, - OFFLINE_BLOCK_RULE_NAME, - OFFLINE_BLOCK_RULE_FRIENDLY, - NET_FW_IP_PROTOCOL_ANY.0, - &local_user_spec, - offline_sid, + &BlockRuleSpec { + internal_name: OFFLINE_BLOCK_RULE_NAME, + friendly_desc: OFFLINE_BLOCK_RULE_FRIENDLY, + protocol: NET_FW_IP_PROTOCOL_ANY.0, + local_user_spec: &local_user_spec, + offline_sid, + remote_addresses: Some(NON_LOOPBACK_REMOTE_ADDRESSES), + remote_ports: None, + }, log, )?; Ok(()) @@ -78,16 +204,22 @@ pub fn ensure_offline_outbound_block(offline_sid: &str, log: &mut File) -> Resul result } -fn ensure_block_rule( - rules: &windows::Win32::NetworkManagement::WindowsFirewall::INetFwRules, - internal_name: &str, - friendly_desc: &str, - protocol: i32, - local_user_spec: &str, - offline_sid: &str, - log: &mut File, -) -> Result<()> { +fn remove_rule_if_present(rules: &INetFwRules, internal_name: &str, log: &mut File) -> Result<()> { let name = BSTR::from(internal_name); + if unsafe { rules.Item(&name) }.is_ok() { + unsafe { rules.Remove(&name) }.map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("Rules::Remove failed for {internal_name}: {err:?}"), + )) + })?; + log_line(log, &format!("firewall rule removed name={internal_name}"))?; + } + Ok(()) +} + +fn ensure_block_rule(rules: &INetFwRules, spec: &BlockRuleSpec<'_>, log: &mut File) -> Result<()> { + let name = BSTR::from(spec.internal_name); let rule: INetFwRule3 = match unsafe { rules.Item(&name) } { Ok(existing) => existing.cast().map_err(|err| { anyhow::Error::new(SetupFailure::new( @@ -112,13 +244,7 @@ fn ensure_block_rule( )) })?; // Set all properties before adding the rule so we don't leave half-configured rules. - configure_rule( - &new_rule, - friendly_desc, - protocol, - local_user_spec, - offline_sid, - )?; + configure_rule(&new_rule, spec)?; unsafe { rules.Add(&new_rule) }.map_err(|err| { anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, @@ -130,26 +256,24 @@ fn ensure_block_rule( }; // Always re-apply fields to keep the setup idempotent. - configure_rule(&rule, friendly_desc, protocol, local_user_spec, offline_sid)?; + configure_rule(&rule, spec)?; + + let remote_addresses_log = spec.remote_addresses.unwrap_or("*"); + let remote_ports_log = spec.remote_ports.unwrap_or("*"); log_line( log, &format!( - "firewall rule configured name={internal_name} protocol={protocol} LocalUserAuthorizedList={local_user_spec}" + "firewall rule configured name={} protocol={} RemoteAddresses={remote_addresses_log} RemotePorts={remote_ports_log} LocalUserAuthorizedList={}", + spec.internal_name, spec.protocol, spec.local_user_spec ), )?; Ok(()) } -fn configure_rule( - rule: &INetFwRule3, - friendly_desc: &str, - protocol: i32, - local_user_spec: &str, - offline_sid: &str, -) -> Result<()> { +fn configure_rule(rule: &INetFwRule3, spec: &BlockRuleSpec<'_>) -> Result<()> { unsafe { - rule.SetDescription(&BSTR::from(friendly_desc)) + rule.SetDescription(&BSTR::from(spec.friendly_desc)) .map_err(|err| { anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, @@ -180,13 +304,29 @@ fn configure_rule( format!("SetProfiles failed: {err:?}"), )) })?; - rule.SetProtocol(protocol).map_err(|err| { + rule.SetProtocol(spec.protocol).map_err(|err| { anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, format!("SetProtocol failed: {err:?}"), )) })?; - rule.SetLocalUserAuthorizedList(&BSTR::from(local_user_spec)) + let remote_addresses = spec.remote_addresses.unwrap_or("*"); + rule.SetRemoteAddresses(&BSTR::from(remote_addresses)) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetRemoteAddresses failed: {err:?}"), + )) + })?; + let remote_ports = spec.remote_ports.unwrap_or("*"); + rule.SetRemotePorts(&BSTR::from(remote_ports)) + .map_err(|err| { + anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("SetRemotePorts failed: {err:?}"), + )) + })?; + rule.SetLocalUserAuthorizedList(&BSTR::from(spec.local_user_spec)) .map_err(|err| { anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, @@ -203,17 +343,59 @@ fn configure_rule( )) })?; let actual_str = actual.to_string(); - if !actual_str.contains(offline_sid) { + if !actual_str.contains(spec.offline_sid) { return Err(anyhow::Error::new(SetupFailure::new( SetupErrorCode::HelperFirewallRuleVerifyFailed, format!( - "offline firewall rule user scope mismatch: expected SID {offline_sid}, got {actual_str}" + "offline firewall rule user scope mismatch: expected SID {}, got {actual_str}", + spec.offline_sid ), ))); } Ok(()) } +fn blocked_loopback_tcp_remote_ports(proxy_ports: &[u16]) -> Option { + let mut allowed_ports = proxy_ports + .iter() + .copied() + .filter(|port| *port != 0) + .collect::>(); + allowed_ports.sort_unstable(); + allowed_ports.dedup(); + + let mut blocked_ranges = Vec::new(); + let mut start = 1_u32; + for port in allowed_ports { + let port = u32::from(port); + if port < start { + continue; + } + if port > start { + blocked_ranges.push(port_range_string(start, port - 1)); + } + start = port + 1; + } + + if start <= u32::from(u16::MAX) { + blocked_ranges.push(port_range_string(start, u32::from(u16::MAX))); + } + + if blocked_ranges.is_empty() { + None + } else { + Some(blocked_ranges.join(",")) + } +} + +fn port_range_string(start: u32, end: u32) -> String { + if start == end { + start.to_string() + } else { + format!("{start}-{end}") + } +} + fn log_line(log: &mut File, msg: &str) -> Result<()> { let ts = chrono::Utc::now().to_rfc3339(); writeln!(log, "[{ts}] {msg}")?; diff --git a/codex-rs/windows-sandbox-rs/src/identity.rs b/codex-rs/windows-sandbox-rs/src/identity.rs index b9d5e29ed..b7768c1a3 100644 --- a/codex-rs/windows-sandbox-rs/src/identity.rs +++ b/codex-rs/windows-sandbox-rs/src/identity.rs @@ -3,9 +3,11 @@ use crate::logging::debug_log; use crate::policy::SandboxPolicy; use crate::setup::gather_read_roots; use crate::setup::gather_write_roots; +use crate::setup::offline_proxy_settings_from_env; use crate::setup::run_elevated_setup; use crate::setup::sandbox_users_path; use crate::setup::setup_marker_path; +use crate::setup::SandboxNetworkIdentity; use crate::setup::SandboxUserRecord; use crate::setup::SandboxUsersFile; use crate::setup::SetupMarker; @@ -33,7 +35,8 @@ pub struct SandboxCreds { /// Returns true when the on-disk setup artifacts exist and match the current /// setup version. /// -/// This reuses the same marker/users validation used by `require_logon_sandbox_creds`. +/// This is a coarse readiness check; `require_logon_sandbox_creds` performs the +/// additional runtime validation for offline firewall settings. pub fn sandbox_setup_is_complete(codex_home: &Path) -> bool { let marker_ok = matches!(load_marker(codex_home), Ok(Some(marker)) if marker.version_matches()); if !marker_ok { @@ -101,7 +104,10 @@ fn decode_password(record: &SandboxUserRecord) -> Result { Ok(pwd) } -fn select_identity(policy: &SandboxPolicy, codex_home: &Path) -> Result> { +fn select_identity( + network_identity: SandboxNetworkIdentity, + codex_home: &Path, +) -> Result> { let _marker = match load_marker(codex_home)? { Some(m) if m.version_matches() => m, _ => return Ok(None), @@ -110,10 +116,9 @@ fn select_identity(policy: &SandboxPolicy, codex_home: &Path) -> Result u, _ => return Ok(None), }; - let chosen = if !policy.has_full_network_access() { - users.offline - } else { - users.online + let chosen = match network_identity { + SandboxNetworkIdentity::Offline => users.offline, + SandboxNetworkIdentity::Online => users.online, }; let password = decode_password(&chosen)?; Ok(Some(SandboxIdentity { @@ -128,25 +133,34 @@ pub fn require_logon_sandbox_creds( command_cwd: &Path, env_map: &HashMap, codex_home: &Path, + proxy_enforced: bool, ) -> Result { let sandbox_dir = crate::setup::sandbox_dir(codex_home); let needed_read = gather_read_roots(command_cwd, policy, codex_home); let needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map); + let network_identity = SandboxNetworkIdentity::from_policy(policy, proxy_enforced); + let desired_offline_proxy_settings = + offline_proxy_settings_from_env(env_map, network_identity); // NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the // restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for // granting the sandbox group access to this directory without granting the capability SID. let mut setup_reason: Option = None; - let mut _existing_marker: Option = None; let mut identity = match load_marker(codex_home)? { Some(marker) if marker.version_matches() => { - _existing_marker = Some(marker.clone()); - let selected = select_identity(policy, codex_home)?; - if selected.is_none() { - setup_reason = - Some("sandbox users missing or incompatible with marker version".to_string()); + if let Some(reason) = + marker.request_mismatch_reason(network_identity, &desired_offline_proxy_settings) + { + setup_reason = Some(reason); + None + } else { + let selected = select_identity(network_identity, codex_home)?; + if selected.is_none() { + setup_reason = + Some("sandbox users missing or incompatible with marker version".to_string()); + } + selected } - selected } _ => { setup_reason = Some("sandbox setup marker missing or incompatible".to_string()); @@ -164,18 +178,30 @@ pub fn require_logon_sandbox_creds( crate::logging::log_note("sandbox setup required", Some(&sandbox_dir)); } run_elevated_setup( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - Some(needed_read.clone()), - Some(needed_write.clone()), + crate::setup::SandboxSetupRequest { + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced, + }, + crate::setup::SetupRootOverrides { + read_roots: Some(needed_read.clone()), + write_roots: Some(needed_write.clone()), + }, )?; - identity = select_identity(policy, codex_home)?; + identity = select_identity(network_identity, codex_home)?; } // Always refresh ACLs (non-elevated) for current roots via the setup binary. - crate::setup::run_setup_refresh(policy, policy_cwd, command_cwd, env_map, codex_home)?; + crate::setup::run_setup_refresh( + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced, + )?; let identity = identity.ok_or_else(|| { anyhow!( "Windows sandbox setup is missing or out of date; rerun the sandbox setup with elevation" diff --git a/codex-rs/windows-sandbox-rs/src/lib.rs b/codex-rs/windows-sandbox-rs/src/lib.rs index 73193a207..11afda750 100644 --- a/codex-rs/windows-sandbox-rs/src/lib.rs +++ b/codex-rs/windows-sandbox-rs/src/lib.rs @@ -72,6 +72,8 @@ pub use dpapi::unprotect as dpapi_unprotect; #[cfg(target_os = "windows")] pub use elevated_impl::run_windows_sandbox_capture as run_windows_sandbox_capture_elevated; #[cfg(target_os = "windows")] +pub use elevated_impl::ElevatedSandboxCaptureRequest; +#[cfg(target_os = "windows")] pub use helper_materialization::resolve_current_exe_for_launch; #[cfg(target_os = "windows")] pub use hide_users::hide_current_user_profile_dir; @@ -116,6 +118,10 @@ pub use setup::sandbox_dir; #[cfg(target_os = "windows")] pub use setup::sandbox_secrets_dir; #[cfg(target_os = "windows")] +pub use setup::SandboxSetupRequest; +#[cfg(target_os = "windows")] +pub use setup::SetupRootOverrides; +#[cfg(target_os = "windows")] pub use setup::SETUP_VERSION; #[cfg(target_os = "windows")] pub use setup_error::extract_failure as extract_setup_failure; diff --git a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs index 5ab39c3ce..e006b696f 100644 --- a/codex-rs/windows-sandbox-rs/src/sandbox_users.rs +++ b/codex-rs/windows-sandbox-rs/src/sandbox_users.rs @@ -63,6 +63,8 @@ pub fn provision_sandbox_users( codex_home: &Path, offline_username: &str, online_username: &str, + proxy_ports: &[u16], + allow_local_binding: bool, log: &mut File, ) -> Result<()> { ensure_sandbox_users_group(log)?; @@ -80,6 +82,8 @@ pub fn provision_sandbox_users( &offline_password, online_username, &online_password, + proxy_ports, + allow_local_binding, )?; Ok(()) } @@ -388,6 +392,8 @@ struct SetupMarker { offline_username: String, online_username: String, created_at: String, + proxy_ports: Vec, + allow_local_binding: bool, read_roots: Vec, write_roots: Vec, } @@ -398,6 +404,8 @@ fn write_secrets( offline_pwd: &str, online_user: &str, online_pwd: &str, + proxy_ports: &[u16], + allow_local_binding: bool, ) -> Result<()> { let sandbox_dir = sandbox_dir(codex_home); std::fs::create_dir_all(&sandbox_dir).map_err(|err| { @@ -447,6 +455,8 @@ fn write_secrets( offline_username: offline_user.to_string(), online_username: online_user.to_string(), created_at: chrono::Utc::now().to_rfc3339(), + proxy_ports: proxy_ports.to_vec(), + allow_local_binding, read_roots: Vec::new(), write_roots: Vec::new(), }; diff --git a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs index 86c1d104e..8a72b22a9 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_main_win.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_main_win.rs @@ -84,6 +84,10 @@ struct Payload { command_cwd: PathBuf, read_roots: Vec, write_roots: Vec, + #[serde(default)] + proxy_ports: Vec, + #[serde(default)] + allow_local_binding: bool, real_user: String, #[serde(default)] mode: SetupMode, @@ -510,6 +514,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( &payload.codex_home, &payload.offline_username, &payload.online_username, + &payload.proxy_ports, + payload.allow_local_binding, log, ); if let Err(err) = provision_result { @@ -572,6 +578,21 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<( }; let mut refresh_errors: Vec = Vec::new(); if !refresh_only { + let proxy_allowlist_result = firewall::ensure_offline_proxy_allowlist( + &offline_sid_str, + &payload.proxy_ports, + payload.allow_local_binding, + log, + ); + if let Err(err) = proxy_allowlist_result { + if extract_setup_failure(&err).is_some() { + return Err(err); + } + return Err(anyhow::Error::new(SetupFailure::new( + SetupErrorCode::HelperFirewallRuleCreateOrAddFailed, + format!("ensure offline proxy allowlist failed: {err}"), + ))); + } let firewall_result = firewall::ensure_offline_outbound_block(&offline_sid_str, log); if let Err(err) = firewall_result { if extract_setup_failure(&err).is_some() { diff --git a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs index 3296d09c4..962a53d60 100644 --- a/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs +++ b/codex-rs/windows-sandbox-rs/src/setup_orchestrator.rs @@ -1,5 +1,6 @@ use serde::Deserialize; use serde::Serialize; +use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; use std::ffi::c_void; @@ -78,21 +79,39 @@ pub fn sandbox_users_path(codex_home: &Path) -> PathBuf { sandbox_secrets_dir(codex_home).join("sandbox_users.json") } +pub struct SandboxSetupRequest<'a> { + pub policy: &'a SandboxPolicy, + pub policy_cwd: &'a Path, + pub command_cwd: &'a Path, + pub env_map: &'a HashMap, + pub codex_home: &'a Path, + pub proxy_enforced: bool, +} + +#[derive(Default)] +pub struct SetupRootOverrides { + pub read_roots: Option>, + pub write_roots: Option>, +} + pub fn run_setup_refresh( policy: &SandboxPolicy, policy_cwd: &Path, command_cwd: &Path, env_map: &HashMap, codex_home: &Path, + proxy_enforced: bool, ) -> Result<()> { run_setup_refresh_inner( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - /*read_roots_override*/ None, - /*write_roots_override*/ None, + SandboxSetupRequest { + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced, + }, + SetupRootOverrides::default(), ) } @@ -103,53 +122,51 @@ pub fn run_setup_refresh_with_extra_read_roots( env_map: &HashMap, codex_home: &Path, extra_read_roots: Vec, + proxy_enforced: bool, ) -> Result<()> { let mut read_roots = gather_read_roots(command_cwd, policy, codex_home); read_roots.extend(extra_read_roots); run_setup_refresh_inner( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - Some(read_roots), - Some(Vec::new()), + SandboxSetupRequest { + policy, + policy_cwd, + command_cwd, + env_map, + codex_home, + proxy_enforced, + }, + SetupRootOverrides { + read_roots: Some(read_roots), + write_roots: Some(Vec::new()), + }, ) } fn run_setup_refresh_inner( - policy: &SandboxPolicy, - policy_cwd: &Path, - command_cwd: &Path, - env_map: &HashMap, - codex_home: &Path, - read_roots_override: Option>, - write_roots_override: Option>, + request: SandboxSetupRequest<'_>, + overrides: SetupRootOverrides, ) -> Result<()> { // Skip in danger-full-access. if matches!( - policy, + request.policy, SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } ) { return Ok(()); } - let (read_roots, write_roots) = build_payload_roots( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - read_roots_override, - write_roots_override, - ); + let (read_roots, write_roots) = build_payload_roots(&request, overrides); + let network_identity = + SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced); + let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); let payload = ElevationPayload { version: SETUP_VERSION, offline_username: OFFLINE_USERNAME.to_string(), online_username: ONLINE_USERNAME.to_string(), - codex_home: codex_home.to_path_buf(), - command_cwd: command_cwd.to_path_buf(), + codex_home: request.codex_home.to_path_buf(), + command_cwd: request.command_cwd.to_path_buf(), read_roots, write_roots, + proxy_ports: offline_proxy_settings.proxy_ports, + allow_local_binding: offline_proxy_settings.allow_local_binding, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: true, }; @@ -159,7 +176,7 @@ fn run_setup_refresh_inner( // Refresh should never request elevation; ensure verb isn't set and we don't trigger UAC. let mut cmd = Command::new(&exe); cmd.arg(&b64).stdout(Stdio::null()).stderr(Stdio::null()); - let cwd = std::env::current_dir().unwrap_or_else(|_| codex_home.to_path_buf()); + let cwd = std::env::current_dir().unwrap_or_else(|_| request.codex_home.to_path_buf()); log_note( &format!( "setup refresh: spawning {} (cwd={}, payload_len={})", @@ -167,14 +184,14 @@ fn run_setup_refresh_inner( cwd.display(), b64.len() ), - Some(&sandbox_dir(codex_home)), + Some(&sandbox_dir(request.codex_home)), ); let status = cmd .status() .map_err(|e| { log_note( &format!("setup refresh: failed to spawn {}: {e}", exe.display()), - Some(&sandbox_dir(codex_home)), + Some(&sandbox_dir(request.codex_home)), ); e }) @@ -182,7 +199,7 @@ fn run_setup_refresh_inner( if !status.success() { log_note( &format!("setup refresh: exited with status {status:?}"), - Some(&sandbox_dir(codex_home)), + Some(&sandbox_dir(request.codex_home)), ); return Err(anyhow!("setup refresh failed with status {}", status)); } @@ -196,12 +213,38 @@ pub struct SetupMarker { pub online_username: String, #[serde(default)] pub created_at: Option, + #[serde(default)] + pub proxy_ports: Vec, + #[serde(default)] + pub allow_local_binding: bool, } impl SetupMarker { pub fn version_matches(&self) -> bool { self.version == SETUP_VERSION } + + pub(crate) fn request_mismatch_reason( + &self, + network_identity: SandboxNetworkIdentity, + offline_proxy_settings: &OfflineProxySettings, + ) -> Option { + if !network_identity.uses_offline_identity() { + return None; + } + if self.proxy_ports == offline_proxy_settings.proxy_ports + && self.allow_local_binding == offline_proxy_settings.allow_local_binding + { + return None; + } + Some(format!( + "offline firewall settings changed (stored_ports={:?}, desired_ports={:?}, stored_allow_local_binding={}, desired_allow_local_binding={})", + self.proxy_ports, + offline_proxy_settings.proxy_ports, + self.allow_local_binding, + offline_proxy_settings.allow_local_binding + )) + } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -390,11 +433,106 @@ struct ElevationPayload { command_cwd: PathBuf, read_roots: Vec, write_roots: Vec, + #[serde(default)] + proxy_ports: Vec, + #[serde(default)] + allow_local_binding: bool, real_user: String, #[serde(default)] refresh_only: bool, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct OfflineProxySettings { + pub proxy_ports: Vec, + pub allow_local_binding: bool, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum SandboxNetworkIdentity { + Offline, + Online, +} + +impl SandboxNetworkIdentity { + pub(crate) fn from_policy(policy: &SandboxPolicy, proxy_enforced: bool) -> Self { + if proxy_enforced || !policy.has_full_network_access() { + Self::Offline + } else { + Self::Online + } + } + + pub(crate) fn uses_offline_identity(self) -> bool { + matches!(self, Self::Offline) + } +} + +const PROXY_ENV_KEYS: &[&str] = &[ + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "WS_PROXY", + "WSS_PROXY", + "http_proxy", + "https_proxy", + "all_proxy", + "ws_proxy", + "wss_proxy", +]; +const ALLOW_LOCAL_BINDING_ENV_KEY: &str = "CODEX_NETWORK_ALLOW_LOCAL_BINDING"; + +pub(crate) fn offline_proxy_settings_from_env( + env_map: &HashMap, + network_identity: SandboxNetworkIdentity, +) -> OfflineProxySettings { + if !network_identity.uses_offline_identity() { + return OfflineProxySettings { + proxy_ports: vec![], + allow_local_binding: false, + }; + } + OfflineProxySettings { + proxy_ports: proxy_ports_from_env(env_map), + allow_local_binding: env_map + .get(ALLOW_LOCAL_BINDING_ENV_KEY) + .is_some_and(|value| value == "1"), + } +} + +pub(crate) fn proxy_ports_from_env(env_map: &HashMap) -> Vec { + let mut ports = BTreeSet::new(); + for key in PROXY_ENV_KEYS { + if let Some(value) = env_map.get(*key) { + if let Some(port) = loopback_proxy_port_from_url(value) { + ports.insert(port); + } + } + } + ports.into_iter().collect() +} + +fn loopback_proxy_port_from_url(url: &str) -> Option { + let authority = url.trim().split_once("://")?.1.split('/').next()?; + let host_port = authority.rsplit_once('@').map_or(authority, |(_, hp)| hp); + + if let Some(host) = host_port.strip_prefix('[') { + let (host, rest) = host.split_once(']')?; + if host != "::1" { + return None; + } + let port = rest.strip_prefix(':')?.parse::().ok()?; + return (port != 0).then_some(port); + } + + let (host, port) = host_port.rsplit_once(':')?; + if !(host.eq_ignore_ascii_case("localhost") || host == "127.0.0.1") { + return None; + } + let port = port.parse::().ok()?; + (port != 0).then_some(port) +} + fn quote_arg(arg: &str) -> String { let needs = arg.is_empty() || arg @@ -574,39 +712,31 @@ fn run_setup_exe( } pub fn run_elevated_setup( - policy: &SandboxPolicy, - policy_cwd: &Path, - command_cwd: &Path, - env_map: &HashMap, - codex_home: &Path, - read_roots_override: Option>, - write_roots_override: Option>, + request: SandboxSetupRequest<'_>, + overrides: SetupRootOverrides, ) -> Result<()> { // Ensure the shared sandbox directory exists before we send it to the elevated helper. - let sbx_dir = sandbox_dir(codex_home); + let sbx_dir = sandbox_dir(request.codex_home); std::fs::create_dir_all(&sbx_dir).map_err(|err| { failure( SetupErrorCode::OrchestratorSandboxDirCreateFailed, format!("failed to create sandbox dir {}: {err}", sbx_dir.display()), ) })?; - let (read_roots, write_roots) = build_payload_roots( - policy, - policy_cwd, - command_cwd, - env_map, - codex_home, - read_roots_override, - write_roots_override, - ); + let (read_roots, write_roots) = build_payload_roots(&request, overrides); + let network_identity = + SandboxNetworkIdentity::from_policy(request.policy, request.proxy_enforced); + let offline_proxy_settings = offline_proxy_settings_from_env(request.env_map, network_identity); let payload = ElevationPayload { version: SETUP_VERSION, offline_username: OFFLINE_USERNAME.to_string(), online_username: ONLINE_USERNAME.to_string(), - codex_home: codex_home.to_path_buf(), - command_cwd: command_cwd.to_path_buf(), + codex_home: request.codex_home.to_path_buf(), + command_cwd: request.command_cwd.to_path_buf(), read_roots, write_roots, + proxy_ports: offline_proxy_settings.proxy_ports, + allow_local_binding: offline_proxy_settings.allow_local_binding, real_user: std::env::var("USERNAME").unwrap_or_else(|_| "Administrators".to_string()), refresh_only: false, }; @@ -616,28 +746,28 @@ pub fn run_elevated_setup( format!("failed to determine elevation state: {err}"), ) })?; - run_setup_exe(&payload, needs_elevation, codex_home) + run_setup_exe(&payload, needs_elevation, request.codex_home) } fn build_payload_roots( - policy: &SandboxPolicy, - policy_cwd: &Path, - command_cwd: &Path, - env_map: &HashMap, - codex_home: &Path, - read_roots_override: Option>, - write_roots_override: Option>, + request: &SandboxSetupRequest<'_>, + overrides: SetupRootOverrides, ) -> (Vec, Vec) { - let write_roots = if let Some(roots) = write_roots_override { + let write_roots = if let Some(roots) = overrides.write_roots { canonical_existing(&roots) } else { - gather_write_roots(policy, policy_cwd, command_cwd, env_map) + gather_write_roots( + request.policy, + request.policy_cwd, + request.command_cwd, + request.env_map, + ) }; - let write_roots = filter_sensitive_write_roots(write_roots, codex_home); - let mut read_roots = if let Some(roots) = read_roots_override { + let write_roots = filter_sensitive_write_roots(write_roots, request.codex_home); + let mut read_roots = if let Some(roots) = overrides.read_roots { canonical_existing(&roots) } else { - gather_read_roots(command_cwd, policy, codex_home) + gather_read_roots(request.command_cwd, request.policy, request.codex_home) }; let write_root_set: HashSet = write_roots.iter().cloned().collect(); read_roots.retain(|root| !write_root_set.contains(root)); @@ -673,13 +803,17 @@ fn filter_sensitive_write_roots(mut roots: Vec, codex_home: &Path) -> V mod tests { use super::gather_legacy_full_read_roots; use super::gather_read_roots; + use super::loopback_proxy_port_from_url; + use super::offline_proxy_settings_from_env; use super::profile_read_roots; + use super::proxy_ports_from_env; use super::WINDOWS_PLATFORM_DEFAULT_READ_ROOTS; use crate::helper_materialization::helper_bin_dir; use crate::policy::SandboxPolicy; use codex_protocol::protocol::ReadOnlyAccess; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; + use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::PathBuf; @@ -692,6 +826,143 @@ mod tests { .collect() } + #[test] + fn loopback_proxy_url_parsing_supports_common_forms() { + assert_eq!( + loopback_proxy_port_from_url("http://localhost:3128"), + Some(3128) + ); + assert_eq!( + loopback_proxy_port_from_url("https://127.0.0.1:8080"), + Some(8080) + ); + assert_eq!( + loopback_proxy_port_from_url("socks5h://user:pass@[::1]:1080"), + Some(1080) + ); + } + + #[test] + fn loopback_proxy_url_parsing_rejects_non_loopback_and_zero_port() { + assert_eq!( + loopback_proxy_port_from_url("http://example.com:3128"), + None + ); + assert_eq!(loopback_proxy_port_from_url("http://127.0.0.1:0"), None); + assert_eq!(loopback_proxy_port_from_url("localhost:8080"), None); + } + + #[test] + fn proxy_ports_from_env_dedupes_and_sorts() { + let mut env = HashMap::new(); + env.insert( + "HTTP_PROXY".to_string(), + "http://127.0.0.1:8080".to_string(), + ); + env.insert( + "http_proxy".to_string(), + "http://localhost:8080".to_string(), + ); + env.insert("ALL_PROXY".to_string(), "socks5h://[::1]:1081".to_string()); + env.insert( + "HTTPS_PROXY".to_string(), + "https://example.com:9999".to_string(), + ); + + assert_eq!(proxy_ports_from_env(&env), vec![1081, 8080]); + } + + #[test] + fn offline_proxy_settings_ignore_proxy_env_when_online_identity_selected() { + let mut env = HashMap::new(); + env.insert( + "HTTP_PROXY".to_string(), + "http://127.0.0.1:8080".to_string(), + ); + env.insert( + "CODEX_NETWORK_ALLOW_LOCAL_BINDING".to_string(), + "1".to_string(), + ); + + assert_eq!( + offline_proxy_settings_from_env(&env, super::SandboxNetworkIdentity::Online), + super::OfflineProxySettings { + proxy_ports: vec![], + allow_local_binding: false, + } + ); + } + + #[test] + fn offline_proxy_settings_capture_proxy_ports_and_local_binding_for_offline_identity() { + let mut env = HashMap::new(); + env.insert( + "HTTP_PROXY".to_string(), + "http://127.0.0.1:8080".to_string(), + ); + env.insert( + "ALL_PROXY".to_string(), + "socks5h://127.0.0.1:1081".to_string(), + ); + env.insert( + "CODEX_NETWORK_ALLOW_LOCAL_BINDING".to_string(), + "1".to_string(), + ); + + assert_eq!( + offline_proxy_settings_from_env(&env, super::SandboxNetworkIdentity::Offline), + super::OfflineProxySettings { + proxy_ports: vec![1081, 8080], + allow_local_binding: true, + } + ); + } + + #[test] + fn setup_marker_request_mismatch_reason_ignores_proxy_drift_for_online_identity() { + let marker = super::SetupMarker { + version: super::SETUP_VERSION, + offline_username: "offline".to_string(), + online_username: "online".to_string(), + created_at: None, + proxy_ports: vec![3128], + allow_local_binding: false, + }; + let desired = super::OfflineProxySettings { + proxy_ports: vec![1081, 8080], + allow_local_binding: true, + }; + + assert_eq!( + marker.request_mismatch_reason(super::SandboxNetworkIdentity::Online, &desired), + None + ); + } + + #[test] + fn setup_marker_request_mismatch_reason_reports_offline_firewall_drift() { + let marker = super::SetupMarker { + version: super::SETUP_VERSION, + offline_username: "offline".to_string(), + online_username: "online".to_string(), + created_at: None, + proxy_ports: vec![3128], + allow_local_binding: false, + }; + let desired = super::OfflineProxySettings { + proxy_ports: vec![1081, 8080], + allow_local_binding: true, + }; + + assert_eq!( + marker.request_mismatch_reason(super::SandboxNetworkIdentity::Offline, &desired), + Some( + "offline firewall settings changed (stored_ports=[3128], desired_ports=[1081, 8080], stored_allow_local_binding=false, desired_allow_local_binding=true)" + .to_string() + ) + ); + } + #[test] fn profile_read_roots_excludes_configured_top_level_entries() { let tmp = TempDir::new().expect("tempdir");