Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions codex-rs/network-proxy/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,10 @@ pub const NO_PROXY_ENV_KEYS: &[&str] = &[

pub const DEFAULT_NO_PROXY_VALUE: &str = concat!(
"localhost,127.0.0.1,::1,",
"*.local,.local,",
"169.254.0.0/16,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
"169.254.0.0/16,",
"10.0.0.0/8,",
"172.16.0.0/12,",
"192.168.0.0/16"
);

pub fn proxy_url_env_value<'a>(
Expand Down Expand Up @@ -452,7 +454,9 @@ fn apply_proxy_env_overrides(
// HTTP(S)_PROXY. Keep them aligned with the managed HTTP proxy endpoint.
set_env_keys(env, WEBSOCKET_PROXY_ENV_KEYS, &http_proxy_url);

// Keep local/private targets direct so local IPC and metadata endpoints avoid the proxy.
// Keep loopback and IP-literal private targets direct so local IPC/LAN access avoids the proxy.
// Do not include hostname suffixes here: those can force clients to resolve internal names
// locally instead of letting the proxy resolve them.
set_env_keys(env, NO_PROXY_ENV_KEYS, DEFAULT_NO_PROXY_VALUE);

env.insert("ELECTRON_GET_USE_PROXY".to_string(), "true".to_string());
Expand Down Expand Up @@ -927,6 +931,11 @@ mod tests {
env.get("NO_PROXY"),
Some(&DEFAULT_NO_PROXY_VALUE.to_string())
);
let no_proxy = env.get("NO_PROXY").expect("NO_PROXY should be set");
assert!(no_proxy.contains("10.0.0.0/8"));
assert!(no_proxy.contains("172.16.0.0/12"));
assert!(no_proxy.contains("192.168.0.0/16"));
assert!(no_proxy.contains("169.254.0.0/16"));
assert_eq!(env.get(ALLOW_LOCAL_BINDING_ENV_KEY), Some(&"0".to_string()));
assert_eq!(env.get("ELECTRON_GET_USE_PROXY"), Some(&"true".to_string()));
#[cfg(target_os = "macos")]
Expand Down
8 changes: 6 additions & 2 deletions codex-rs/sandboxing/src/seatbelt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,15 @@ fn dynamic_network_policy_for_network(
if should_use_restricted_network_policy {
let mut policy = String::new();
if proxy.allow_local_binding {
policy.push_str("; allow loopback local binding and loopback traffic\n");
policy.push_str("(allow network-bind (local ip \"localhost:*\"))\n");
policy.push_str("; allow local binding and loopback traffic\n");
policy.push_str("(allow network-bind (local ip \"*:*\"))\n");
policy.push_str("(allow network-inbound (local ip \"localhost:*\"))\n");
policy.push_str("(allow network-outbound (remote ip \"localhost:*\"))\n");
}
if proxy.allow_local_binding && !proxy.ports.is_empty() {
policy.push_str("; allow DNS lookups while application traffic remains proxy-routed\n");
policy.push_str("(allow network-outbound (remote ip \"*:53\"))\n");
Comment thread
viyatb-oai marked this conversation as resolved.
}
for port in &proxy.ports {
policy.push_str(&format!(
"(allow network-outbound (remote ip \"localhost:{port}\"))\n"
Expand Down
51 changes: 48 additions & 3 deletions codex-rs/sandboxing/src/seatbelt_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,17 @@ fn create_seatbelt_args_routes_network_through_proxy_ports() {
"policy should not include blanket outbound allowance when proxy ports are present:\n{policy}"
);
assert!(
!policy.contains("(allow network-bind (local ip \"localhost:*\"))"),
"policy should not allow loopback binding unless explicitly enabled:\n{policy}"
!policy.contains("(allow network-bind (local ip \"*:*\"))"),
"policy should not allow local binding unless explicitly enabled:\n{policy}"
);
assert!(
!policy.contains("(allow network-inbound (local ip \"localhost:*\"))"),
"policy should not allow loopback inbound unless explicitly enabled:\n{policy}"
);
assert!(
!policy.contains("(allow network-outbound (remote ip \"*:53\"))"),
"policy should not allow raw DNS unless local binding is explicitly enabled:\n{policy}"
);
}

#[test]
Expand Down Expand Up @@ -290,7 +294,7 @@ fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() {
);

assert!(
policy.contains("(allow network-bind (local ip \"localhost:*\"))"),
policy.contains("(allow network-bind (local ip \"*:*\"))"),
"policy should allow loopback local binding when explicitly enabled:\n{policy}"
);
assert!(
Expand All @@ -301,6 +305,10 @@ fn create_seatbelt_args_allows_local_binding_when_explicitly_enabled() {
policy.contains("(allow network-outbound (remote ip \"localhost:*\"))"),
"policy should allow loopback outbound when explicitly enabled:\n{policy}"
);
assert!(
policy.contains("(allow network-outbound (remote ip \"*:53\"))"),
"policy should allow DNS egress when local binding is explicitly enabled:\n{policy}"
);
assert!(
!policy.contains("\n(allow network-outbound)\n"),
"policy should keep proxy-routed behavior without blanket outbound allowance:\n{policy}"
Expand Down Expand Up @@ -338,6 +346,39 @@ fn dynamic_network_policy_preserves_restricted_policy_when_proxy_config_without_
!policy.contains("(allow network-outbound (remote ip \"localhost:"),
"policy should not include proxy port allowance when proxy config is present without ports:\n{policy}"
);
assert!(
!policy.contains("(allow network-outbound (remote ip \"*:53\"))"),
"policy should stay fail-closed for DNS when no proxy ports are available:\n{policy}"
);
}

#[test]
fn dynamic_network_policy_blocks_dns_when_local_binding_has_no_proxy_ports() {
let policy = dynamic_network_policy(
&SandboxPolicy::WorkspaceWrite {
writable_roots: vec![],
read_only_access: Default::default(),
network_access: true,
exclude_tmpdir_env_var: false,
exclude_slash_tmp: false,
},
/*enforce_managed_network*/ false,
&ProxyPolicyInputs {
ports: vec![],
has_proxy_config: true,
allow_local_binding: true,
..ProxyPolicyInputs::default()
},
);

assert!(
policy.contains("(allow network-bind (local ip \"*:*\"))"),
"policy should still allow explicitly configured local binding:\n{policy}"
);
assert!(
!policy.contains("(allow network-outbound (remote ip \"*:53\"))"),
"policy should not allow DNS egress when no proxy ports are available:\n{policy}"
);
}

#[test]
Expand Down Expand Up @@ -367,6 +408,10 @@ fn dynamic_network_policy_preserves_restricted_policy_for_managed_network_withou
!policy.contains("\n(allow network-outbound)\n"),
"policy should not include blanket outbound allowance when managed network is active without proxy endpoints:\n{policy}"
);
assert!(
!policy.contains("(allow network-outbound (remote ip \"*:53\"))"),
"policy should stay fail-closed for DNS when no proxy endpoints are available:\n{policy}"
);
}

#[test]
Expand Down
Loading