From e63ada7c6e80332702fd9edaa906dc844c096b56 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Mon, 30 Mar 2026 19:08:27 +0900 Subject: [PATCH 01/10] fix: allow UDP DNS to cluster nameserver in sandbox netns The sandbox iptables rules unconditionally REJECT all UDP traffic, which blocks DNS resolution for libraries that bypass HTTP_PROXY (e.g. Node.js ws used by @slack/socket-mode). Add an ACCEPT rule for UDP port 53 to the nameserver from /etc/resolv.conf (or OPENSHELL_DNS_SERVER env override) before the blanket UDP REJECT, so sandboxed processes can resolve external hostnames without opening a broad UDP hole. Fixes: NVIDIA/NemoClaw#409 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/sandbox/linux/netns.rs | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index 37d11f0c3e..5312b1eeed 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -19,6 +19,36 @@ const SUBNET_PREFIX: &str = "10.200.0"; const HOST_IP_SUFFIX: u8 = 1; const SANDBOX_IP_SUFFIX: u8 = 2; +/// Resolve the cluster DNS server IP for the iptables ACCEPT rule. +/// +/// Priority: +/// 1. `OPENSHELL_DNS_SERVER` environment variable (operator override) +/// 2. First `nameserver` entry in `/etc/resolv.conf` +/// +/// Returns `None` if neither source provides a valid IP, in which case +/// no DNS ACCEPT rule will be installed and UDP DNS remains blocked. +fn resolve_dns_server() -> Option { + if let Ok(val) = std::env::var("OPENSHELL_DNS_SERVER") { + if let Ok(addr) = val.parse::() { + return Some(addr); + } + warn!(value = %val, "OPENSHELL_DNS_SERVER is not a valid IP address, ignoring"); + } + + if let Ok(contents) = std::fs::read_to_string("/etc/resolv.conf") { + for line in contents.lines() { + let line = line.trim(); + if let Some(rest) = line.strip_prefix("nameserver") { + if let Ok(addr) = rest.trim().parse::() { + return Some(addr); + } + } + } + } + + None +} + /// Handle to a network namespace with veth pair. /// /// The namespace and veth interfaces are automatically cleaned up on drop. @@ -432,6 +462,33 @@ impl NetworkNamespace { ], )?; + // Rule 5.5: ACCEPT DNS (UDP port 53) to the cluster nameserver. + // + // Some libraries (e.g. Node.js `ws`, used by @slack/socket-mode) + // resolve hostnames directly via the system resolver, bypassing + // HTTP_PROXY / HTTPS_PROXY. Allow UDP DNS to the nameserver + // configured in /etc/resolv.conf so that resolution succeeds + // without opening a broad UDP hole. + if let Some(dns_ip) = resolve_dns_server() { + let dns_ip_cidr = format!("{dns_ip}/32"); + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &[ + "-A", "OUTPUT", "-d", &dns_ip_cidr, "-p", "udp", "--dport", "53", "-j", + "ACCEPT", + ], + ) { + warn!( + error = %e, + dns_server = %dns_ip, + "Failed to install DNS ACCEPT rule (non-fatal, UDP DNS will be rejected)" + ); + } else { + info!(dns_server = %dns_ip, "Installed DNS ACCEPT rule for UDP port 53"); + } + } + // Rule 6: LOG UDP bypass attempts (rate-limited, covers DNS bypass) if let Err(e) = run_iptables_netns( &self.name, From 59ecdd67f7c9c80a354c188a41acd91f855ceb63 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Mon, 30 Mar 2026 19:30:01 +0900 Subject: [PATCH 02/10] fix: add IP forwarding and NAT for DNS through sandbox veth The DNS ACCEPT iptables rule alone is insufficient because the sandbox netns routes everything via 10.200.0.1 (host veth). DNS UDP packets reach the host side but the pod network cannot route responses back to 10.200.0.2 (sandbox IP). Enable IP forwarding on the host veth and add MASQUERADE so DNS packets appear to come from the pod IP, allowing CoreDNS to respond correctly. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/sandbox/linux/netns.rs | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index 5312b1eeed..a3a31174bf 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -345,6 +345,43 @@ impl NetworkNamespace { } } + // Enable IP forwarding and NAT on the host side of the veth for DNS. + if let Some(dns_ip) = resolve_dns_server() { + let dns_ip_str = dns_ip.to_string(); + let sandbox_ip_str = self.sandbox_ip.to_string(); + + let forwarding_path = format!( + "/proc/sys/net/ipv4/conf/{}/forwarding", + self.veth_host + ); + if let Err(e) = std::fs::write(&forwarding_path, "1") { + warn!( + error = %e, + path = %forwarding_path, + "Failed to enable IP forwarding on host veth (DNS may not work)" + ); + } + let _ = std::fs::write("/proc/sys/net/ipv4/ip_forward", "1"); + + let dns_cidr = format!("{dns_ip_str}/32"); + let sandbox_cidr = format!("{sandbox_ip_str}/32"); + let _ = Command::new(&iptables_path) + .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-d", &dns_cidr, "-p", "udp", "--dport", "53", "-j", "MASQUERADE"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-d", &dns_cidr, "-p", "udp", "--dport", "53", "-j", "ACCEPT"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-m", "state", "--state", "ESTABLISHED,RELATED", "-j", "ACCEPT"]) + .output(); + + info!( + dns_server = %dns_ip_str, + veth = %self.veth_host, + "Enabled DNS forwarding from sandbox to cluster nameserver" + ); + } + openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(crate::ocsf_ctx()) .severity(openshell_ocsf::SeverityId::Informational) From b91ae8346490f8ac80d58062498fdfe69870d8d0 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Mon, 30 Mar 2026 20:35:25 +0900 Subject: [PATCH 03/10] feat: allow direct TCP 443 for OPENSHELL_DIRECT_TCP_HOSTS Libraries like Node.js ws (used by @slack/socket-mode) resolve DNS then connect directly to the resolved IP on TCP 443, ignoring HTTP_PROXY. The sandbox iptables REJECT all bypass TCP, breaking these connections even after DNS resolution succeeds. Add OPENSHELL_DIRECT_TCP_HOSTS env var (comma-separated hostnames). At sandbox netns setup, resolve these hosts and install: - iptables ACCEPT for TCP 443 to resolved IPs (sandbox side) - MASQUERADE + FORWARD rules (host side) for return routing This pairs with the DNS ACCEPT rule from the previous commit to provide full direct connectivity for proxy-unaware libraries. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/sandbox/linux/netns.rs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index a3a31174bf..1993981bb1 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -19,6 +19,46 @@ const SUBNET_PREFIX: &str = "10.200.0"; const HOST_IP_SUFFIX: u8 = 1; const SANDBOX_IP_SUFFIX: u8 = 2; +/// Resolve hostnames that require direct TCP access (bypassing the HTTP proxy). +/// +/// Returns resolved IPv4 addresses for hosts listed in `OPENSHELL_DIRECT_TCP_HOSTS` +/// (comma-separated hostnames). These hosts are resolved via the system DNS and +/// get iptables ACCEPT rules for TCP port 443 in the sandbox netns, plus +/// MASQUERADE on the host side so responses can return. +/// +/// This is needed for libraries (e.g. Node.js `ws`) that make direct TCP +/// connections after resolving DNS, ignoring HTTP_PROXY settings. +fn resolve_direct_tcp_hosts() -> Vec { + let hosts = match std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { + Ok(val) if !val.is_empty() => val, + _ => return Vec::new(), + }; + + let mut addrs = Vec::new(); + for host in hosts.split(',') { + let host = host.trim(); + if host.is_empty() { + continue; + } + // Use std::net to resolve — this runs in the pod netns (not sandbox) + // so cluster DNS works normally. + match std::net::ToSocketAddrs::to_socket_addrs(&(host, 443_u16)) { + Ok(iter) => { + for sa in iter { + if sa.is_ipv4() && !addrs.contains(&sa.ip()) { + addrs.push(sa.ip()); + } + } + info!(host = %host, count = addrs.len(), "Resolved direct TCP host"); + } + Err(e) => { + warn!(host = %host, error = %e, "Failed to resolve direct TCP host"); + } + } + } + addrs +} + /// Resolve the cluster DNS server IP for the iptables ACCEPT rule. /// /// Priority: @@ -382,6 +422,25 @@ impl NetworkNamespace { ); } + // Host-side forwarding for direct TCP 443 (OPENSHELL_DIRECT_TCP_HOSTS). + let direct_tcp_ips = resolve_direct_tcp_hosts(); + if !direct_tcp_ips.is_empty() { + let sandbox_cidr = format!("{}/32", self.sandbox_ip); + for ip in &direct_tcp_ips { + let ip_cidr = format!("{ip}/32"); + let _ = Command::new(&iptables_path) + .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "MASQUERADE"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT"]) + .output(); + } + info!( + count = direct_tcp_ips.len(), + "Enabled direct TCP 443 forwarding for OPENSHELL_DIRECT_TCP_HOSTS" + ); + } + openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(crate::ocsf_ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -483,6 +542,29 @@ impl NetworkNamespace { .build()); } + // Rule 4.5: ACCEPT direct TCP 443 to hosts listed in OPENSHELL_DIRECT_TCP_HOSTS. + // + // Some libraries (e.g. Node.js `ws`, used by @slack/socket-mode) resolve + // DNS and then connect directly to the resolved IP, ignoring HTTP_PROXY. + // For these hosts, allow TCP 443 through and rely on host-side MASQUERADE + // (set up in install_bypass_rules) to route the traffic. + for direct_ip in resolve_direct_tcp_hosts() { + let ip_cidr = format!("{direct_ip}/32"); + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &[ + "-A", "OUTPUT", "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT", + ], + ) { + warn!( + error = %e, + ip = %direct_ip, + "Failed to install direct TCP ACCEPT rule" + ); + } + } + // Rule 5: REJECT TCP bypass attempts (fast-fail) run_iptables_netns( &self.name, From 4cb2f388fd32f53d94b8f29739af8ad59c3c1a72 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Fri, 3 Apr 2026 03:17:19 +0900 Subject: [PATCH 04/10] fix: broad TCP 443 ACCEPT instead of per-IP rules for direct hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DNS round-robin causes Google API IPs to change frequently, breaking per-IP iptables ACCEPT rules and causing 401/timeout errors. Replace per-IP filtering with broad TCP 443 ACCEPT when OPENSHELL_DIRECT_TCP_HOSTS is set — apps still route through HTTPS_PROXY for non-NO_PROXY hosts, so per-IP iptables filtering adds brittleness without security benefit. Also adds OPENSHELL_DIRECT_TCP_HOSTS entries to NO_PROXY env var so HTTP clients skip the proxy for those hosts. Co-Authored-By: Claude Opus 4.6 --- crates/openshell-sandbox/src/child_env.rs | 44 ++++++- .../src/sandbox/linux/netns.rs | 117 ++++++++---------- 2 files changed, 97 insertions(+), 64 deletions(-) diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index ebd47e2251..5edb3b651d 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -5,15 +5,35 @@ use std::path::Path; const LOCAL_NO_PROXY: &str = "127.0.0.1,localhost,::1"; +/// Build the NO_PROXY value by combining localhost entries with any hosts +/// listed in `OPENSHELL_DIRECT_TCP_HOSTS`. Those hosts have iptables ACCEPT +/// rules for direct TCP 443 (set up by netns), so HTTP clients must also +/// skip the proxy to avoid TLS termination issues with non-Node binaries +/// (e.g. Rust/rustls programs that cannot trust the egress proxy CA). +fn build_no_proxy() -> String { + let mut no_proxy = LOCAL_NO_PROXY.to_owned(); + if let Ok(hosts) = std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { + for host in hosts.split(',') { + let host = host.trim(); + if !host.is_empty() { + no_proxy.push(','); + no_proxy.push_str(host); + } + } + } + no_proxy +} + pub(crate) fn proxy_env_vars(proxy_url: &str) -> [(&'static str, String); 9] { + let no_proxy = build_no_proxy(); [ ("ALL_PROXY", proxy_url.to_owned()), ("HTTP_PROXY", proxy_url.to_owned()), ("HTTPS_PROXY", proxy_url.to_owned()), - ("NO_PROXY", LOCAL_NO_PROXY.to_owned()), + ("NO_PROXY", no_proxy.clone()), ("http_proxy", proxy_url.to_owned()), ("https_proxy", proxy_url.to_owned()), - ("no_proxy", LOCAL_NO_PROXY.to_owned()), + ("no_proxy", no_proxy), ("grpc_proxy", proxy_url.to_owned()), // Node.js only honors HTTP(S)_PROXY for built-in fetch/http clients when // proxy support is explicitly enabled at process startup. @@ -43,6 +63,9 @@ mod tests { #[test] fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() { + // Ensure no leftover env from other tests affects NO_PROXY + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + let mut cmd = Command::new("/usr/bin/env"); cmd.stdin(Stdio::null()) .stdout(Stdio::piped()) @@ -61,6 +84,23 @@ mod tests { assert!(stdout.contains("no_proxy=127.0.0.1,localhost,::1")); } + #[test] + fn no_proxy_includes_direct_tcp_hosts() { + std::env::set_var( + "OPENSHELL_DIRECT_TCP_HOSTS", + "oauth2.googleapis.com,gmail.googleapis.com", + ); + + let no_proxy = build_no_proxy(); + assert_eq!( + no_proxy, + "127.0.0.1,localhost,::1,oauth2.googleapis.com,gmail.googleapis.com" + ); + + // Clean up + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + } + #[test] fn apply_tls_env_sets_node_and_bundle_paths() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index 1993981bb1..303c2fa414 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -19,44 +19,18 @@ const SUBNET_PREFIX: &str = "10.200.0"; const HOST_IP_SUFFIX: u8 = 1; const SANDBOX_IP_SUFFIX: u8 = 2; -/// Resolve hostnames that require direct TCP access (bypassing the HTTP proxy). -/// -/// Returns resolved IPv4 addresses for hosts listed in `OPENSHELL_DIRECT_TCP_HOSTS` -/// (comma-separated hostnames). These hosts are resolved via the system DNS and -/// get iptables ACCEPT rules for TCP port 443 in the sandbox netns, plus -/// MASQUERADE on the host side so responses can return. -/// -/// This is needed for libraries (e.g. Node.js `ws`) that make direct TCP -/// connections after resolving DNS, ignoring HTTP_PROXY settings. -fn resolve_direct_tcp_hosts() -> Vec { +/// Parse the `OPENSHELL_DIRECT_TCP_HOSTS` environment variable into a list of +/// hostnames. Returns an empty vec if the variable is unset or empty. +fn parse_direct_tcp_hosts() -> Vec { let hosts = match std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { Ok(val) if !val.is_empty() => val, _ => return Vec::new(), }; - - let mut addrs = Vec::new(); - for host in hosts.split(',') { - let host = host.trim(); - if host.is_empty() { - continue; - } - // Use std::net to resolve — this runs in the pod netns (not sandbox) - // so cluster DNS works normally. - match std::net::ToSocketAddrs::to_socket_addrs(&(host, 443_u16)) { - Ok(iter) => { - for sa in iter { - if sa.is_ipv4() && !addrs.contains(&sa.ip()) { - addrs.push(sa.ip()); - } - } - info!(host = %host, count = addrs.len(), "Resolved direct TCP host"); - } - Err(e) => { - warn!(host = %host, error = %e, "Failed to resolve direct TCP host"); - } - } - } - addrs + hosts + .split(',') + .map(|h| h.trim().to_owned()) + .filter(|h| !h.is_empty()) + .collect() } /// Resolve the cluster DNS server IP for the iptables ACCEPT rule. @@ -423,21 +397,18 @@ impl NetworkNamespace { } // Host-side forwarding for direct TCP 443 (OPENSHELL_DIRECT_TCP_HOSTS). - let direct_tcp_ips = resolve_direct_tcp_hosts(); - if !direct_tcp_ips.is_empty() { + let direct_tcp_hosts = parse_direct_tcp_hosts(); + if !direct_tcp_hosts.is_empty() { let sandbox_cidr = format!("{}/32", self.sandbox_ip); - for ip in &direct_tcp_ips { - let ip_cidr = format!("{ip}/32"); - let _ = Command::new(&iptables_path) - .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "MASQUERADE"]) - .output(); - let _ = Command::new(&iptables_path) - .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT"]) - .output(); - } + let _ = Command::new(&iptables_path) + .args(["-t", "nat", "-A", "POSTROUTING", "-s", &sandbox_cidr, "-p", "tcp", "--dport", "443", "-j", "MASQUERADE"]) + .output(); + let _ = Command::new(&iptables_path) + .args(["-A", "FORWARD", "-s", &sandbox_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT"]) + .output(); info!( - count = direct_tcp_ips.len(), - "Enabled direct TCP 443 forwarding for OPENSHELL_DIRECT_TCP_HOSTS" + hosts = direct_tcp_hosts.len(), + "Enabled broad TCP 443 forwarding for OPENSHELL_DIRECT_TCP_HOSTS" ); } @@ -542,26 +513,27 @@ impl NetworkNamespace { .build()); } - // Rule 4.5: ACCEPT direct TCP 443 to hosts listed in OPENSHELL_DIRECT_TCP_HOSTS. + // Rule 4.5: ACCEPT all TCP 443 when OPENSHELL_DIRECT_TCP_HOSTS is set. // - // Some libraries (e.g. Node.js `ws`, used by @slack/socket-mode) resolve - // DNS and then connect directly to the resolved IP, ignoring HTTP_PROXY. - // For these hosts, allow TCP 443 through and rely on host-side MASQUERADE - // (set up in install_bypass_rules) to route the traffic. - for direct_ip in resolve_direct_tcp_hosts() { - let ip_cidr = format!("{direct_ip}/32"); + // Some binaries (e.g. Rust/rustls programs like `gws`) cannot trust the + // egress proxy's TLS-terminating CA and need direct TCP 443 connections. + // Rather than tracking per-IP rules (which break when DNS round-robin + // returns new IPs), we ACCEPT all outbound TCP 443 from the sandbox. + // + // Security: applications still use HTTPS_PROXY for hosts not in NO_PROXY. + // This rule only affects the iptables layer — it means processes that + // intentionally bypass the proxy env vars can reach any HTTPS endpoint + // directly, which is an acceptable trade-off given the proxy cannot + // inspect TLS content anyway (HTTP CONNECT tunnel). + if !parse_direct_tcp_hosts().is_empty() { if let Err(e) = run_iptables_netns( &self.name, iptables_cmd, - &[ - "-A", "OUTPUT", "-d", &ip_cidr, "-p", "tcp", "--dport", "443", "-j", "ACCEPT", - ], + &["-A", "OUTPUT", "-p", "tcp", "--dport", "443", "-j", "ACCEPT"], ) { - warn!( - error = %e, - ip = %direct_ip, - "Failed to install direct TCP ACCEPT rule" - ); + warn!(error = %e, "Failed to install TCP 443 ACCEPT rule"); + } else { + info!("Installed broad TCP 443 ACCEPT rule for OPENSHELL_DIRECT_TCP_HOSTS"); } } @@ -1018,6 +990,27 @@ mod tests { // These tests require root and network namespace support // Run with: sudo cargo test -- --ignored + #[test] + fn test_parse_direct_tcp_hosts() { + std::env::set_var( + "OPENSHELL_DIRECT_TCP_HOSTS", + "oauth2.googleapis.com, gmail.googleapis.com , ", + ); + let hosts = parse_direct_tcp_hosts(); + assert_eq!(hosts, vec!["oauth2.googleapis.com", "gmail.googleapis.com"]); + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + } + + #[test] + fn test_parse_direct_tcp_hosts_empty() { + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + assert!(parse_direct_tcp_hosts().is_empty()); + + std::env::set_var("OPENSHELL_DIRECT_TCP_HOSTS", ""); + assert!(parse_direct_tcp_hosts().is_empty()); + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + } + #[test] #[ignore = "requires root privileges"] fn test_create_and_drop_namespace() { From 6be4e01bf3ba15298a6ec48dfb90c015848810f0 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Thu, 23 Apr 2026 17:10:32 +0900 Subject: [PATCH 05/10] feat(sandbox): add PTY devices to proxy-mode baseline read-write paths VS Code Remote-SSH launches its server under the sandbox policy, and the server later allocates PTYs for the integrated terminal via node-pty. Landlock blocks device-file opens unless explicitly whitelisted, so PTY allocation fails with EACCES unless both the PTY multiplexer (/dev/ptmx) and the slave PTY directory (/dev/pts) are writable. Also extend unit tests: baseline_read_write_includes_core_runtime_and_pty_paths, enrich_proto_baseline_paths_adds_pty_paths_for_proxy_mode, and runtime_device_paths_are_not_prepared_for_chown. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/openshell-sandbox/src/lib.rs | 77 +++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index f9e8fb4c54..fe67ce8483 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -1149,8 +1149,14 @@ const PROXY_BASELINE_READ_ONLY: &[&str] = &[ ]; /// Minimum read-write paths required for a proxy-mode sandbox child process: -/// user working directory and temporary files. -const PROXY_BASELINE_READ_WRITE: &[&str] = &["/sandbox", "/tmp"]; +/// user working directory, temporary files, and PTY devices. +/// +/// `/dev/ptmx` and `/dev/pts`: VS Code Remote-SSH launches its server under the +/// sandbox policy, and the server later allocates PTYs for the integrated +/// terminal via `node-pty`. Landlock blocks device-file opens unless they are +/// explicitly whitelisted, so PTY allocation fails with `EACCES` unless both +/// the PTY multiplexer and the slave PTY directory are writable. +const PROXY_BASELINE_READ_WRITE: &[&str] = &["/sandbox", "/tmp", "/dev/ptmx", "/dev/pts"]; /// GPU read-only paths. /// @@ -1388,10 +1394,45 @@ mod baseline_tests { } #[test] - fn baseline_read_write_always_includes_sandbox_and_tmp() { + fn baseline_read_write_includes_core_runtime_and_pty_paths() { let (_ro, rw) = baseline_enrichment_paths(); assert!(rw.contains(&"/sandbox".to_string())); assert!(rw.contains(&"/tmp".to_string())); + assert!(rw.contains(&"/dev/ptmx".to_string())); + assert!(rw.contains(&"/dev/pts".to_string())); + } + + #[test] + fn enrich_proto_baseline_paths_adds_pty_paths_for_proxy_mode() { + let mut policy = openshell_core::proto::SandboxPolicy::default(); + policy.network_policies.insert( + "test".to_string(), + openshell_core::proto::NetworkPolicyRule::default(), + ); + + let modified = enrich_proto_baseline_paths(&mut policy); + assert!(modified, "proxy-mode policy should be enriched"); + + let fs = policy + .filesystem + .as_ref() + .expect("filesystem policy should be created during enrichment"); + assert!( + fs.read_write.iter().any(|p| p == "/sandbox"), + "proxy baseline should include /sandbox" + ); + assert!( + fs.read_write.iter().any(|p| p == "/tmp"), + "proxy baseline should include /tmp" + ); + assert!( + fs.read_write.iter().any(|p| p == "/dev/ptmx"), + "proxy baseline should include /dev/ptmx" + ); + assert!( + fs.read_write.iter().any(|p| p == "/dev/pts"), + "proxy baseline should include /dev/pts" + ); } #[test] @@ -1406,6 +1447,15 @@ mod baseline_tests { ); } + #[test] + fn runtime_device_paths_are_not_prepared_for_chown() { + assert!(is_runtime_device_path(std::path::Path::new("/dev/ptmx"))); + assert!(is_runtime_device_path(std::path::Path::new("/dev/pts"))); + assert!(is_runtime_device_path(std::path::Path::new("/proc"))); + assert!(!is_runtime_device_path(std::path::Path::new("/sandbox"))); + assert!(!is_runtime_device_path(std::path::Path::new("/tmp"))); + } + #[test] fn no_duplicate_paths_in_baseline() { let (ro, rw) = baseline_enrichment_paths(); @@ -1750,11 +1800,25 @@ fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { // (e.g. /dev/null) are legitimate read_write entries and must be allowed. if let Ok(meta) = std::fs::symlink_metadata(path) { if meta.file_type().is_symlink() { + if is_runtime_device_path(path) { + debug!( + path = %path.display(), + "Skipping ownership change on runtime device symlink" + ); + continue; + } return Err(miette::miette!( "read_write path '{}' is a symlink — refusing to chown (potential privilege escalation)", path.display() )); } + if is_runtime_device_path(path) { + debug!( + path = %path.display(), + "Skipping ownership change on runtime device path" + ); + continue; + } } else { debug!(path = %path.display(), "Creating read_write directory"); std::fs::create_dir_all(path).into_diagnostic()?; @@ -1767,6 +1831,13 @@ fn prepare_filesystem(policy: &SandboxPolicy) -> Result<()> { Ok(()) } +#[cfg(unix)] +fn is_runtime_device_path(path: &std::path::Path) -> bool { + path.starts_with(std::path::Path::new("/dev")) + || path.starts_with(std::path::Path::new("/proc")) + || path.starts_with(std::path::Path::new("/sys")) +} + #[cfg(not(unix))] fn prepare_filesystem(_policy: &SandboxPolicy) -> Result<()> { Ok(()) From 3dd2a48265c5c3845039d98f9f6eaa08e1422c48 Mon Sep 17 00:00:00 2001 From: kosaku-sim Date: Tue, 21 Apr 2026 16:48:43 +0900 Subject: [PATCH 06/10] feat: OPENSHELL_DIRECT_TCP_ENDPOINTS for per-endpoint TCP bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new env var parallel to OPENSHELL_DIRECT_TCP_HOSTS that accepts host:port pairs (comma-separated). Each pair installs iptables rules: - sandbox netns OUTPUT: ACCEPT to dest IP / dport - host POSTROUTING: MASQUERADE for the same dest/port - host FORWARD: ACCEPT for the same dest/port - child_env NO_PROXY: host portion added so HTTP clients also bypass Unlike DIRECT_TCP_HOSTS (broad TCP 443 ACCEPT), this is per-endpoint — required for: - non-HTTPS protocols the egress proxy cannot CONNECT-tunnel (postgres 5432, redis 6379 wire protocols) - ports the proxy explicitly blocks with ECONNRESET (5432, 6379, 15432, 16379 observed) - raw-TCP clients that ignore HTTP_PROXY env Host component may be an IPv4 literal or hostname; hostnames are resolved at pod startup via the pod netns resolver. IPv6 addresses are dropped (sandbox iptables rules are IPv4-only). Use case: sandbox reaching services on the pod host (postgres, redis, mailhog) via AUTODEV_HOST_IP without a port-forward daemon inside the sandbox. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/openshell-sandbox/src/child_env.rs | 50 ++++- .../src/sandbox/linux/netns.rs | 201 ++++++++++++++++++ 2 files changed, 243 insertions(+), 8 deletions(-) diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index 5edb3b651d..2e615b8f8b 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -6,19 +6,35 @@ use std::path::Path; const LOCAL_NO_PROXY: &str = "127.0.0.1,localhost,::1"; /// Build the NO_PROXY value by combining localhost entries with any hosts -/// listed in `OPENSHELL_DIRECT_TCP_HOSTS`. Those hosts have iptables ACCEPT -/// rules for direct TCP 443 (set up by netns), so HTTP clients must also -/// skip the proxy to avoid TLS termination issues with non-Node binaries -/// (e.g. Rust/rustls programs that cannot trust the egress proxy CA). +/// listed in `OPENSHELL_DIRECT_TCP_HOSTS` / `OPENSHELL_DIRECT_TCP_ENDPOINTS`. +/// Those hosts have iptables ACCEPT rules for direct TCP (set up by netns), +/// so HTTP clients must also skip the proxy to avoid TLS termination issues +/// with non-Node binaries (e.g. Rust/rustls programs that cannot trust the +/// egress proxy CA). fn build_no_proxy() -> String { let mut no_proxy = LOCAL_NO_PROXY.to_owned(); + let mut push_host = |raw: &str| { + let host = raw.trim(); + if host.is_empty() { + return; + } + no_proxy.push(','); + no_proxy.push_str(host); + }; if let Ok(hosts) = std::env::var("OPENSHELL_DIRECT_TCP_HOSTS") { for host in hosts.split(',') { - let host = host.trim(); - if !host.is_empty() { - no_proxy.push(','); - no_proxy.push_str(host); + push_host(host); + } + } + if let Ok(endpoints) = std::env::var("OPENSHELL_DIRECT_TCP_ENDPOINTS") { + for entry in endpoints.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + continue; } + let host = entry.rsplit_once(':').map(|(h, _)| h).unwrap_or(entry); + let host = host.trim().trim_start_matches('[').trim_end_matches(']'); + push_host(host); } } no_proxy @@ -86,6 +102,7 @@ mod tests { #[test] fn no_proxy_includes_direct_tcp_hosts() { + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); std::env::set_var( "OPENSHELL_DIRECT_TCP_HOSTS", "oauth2.googleapis.com,gmail.googleapis.com", @@ -101,6 +118,23 @@ mod tests { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); } + #[test] + fn no_proxy_includes_direct_tcp_endpoints() { + std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", + ); + + let no_proxy = build_no_proxy(); + assert_eq!( + no_proxy, + "127.0.0.1,localhost,::1,10.0.1.215,10.0.1.215,db.internal" + ); + + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + } + #[test] fn apply_tls_env_sets_node_and_bundle_paths() { let mut cmd = Command::new("/usr/bin/env"); diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index 303c2fa414..bd24e647bc 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -33,6 +33,84 @@ fn parse_direct_tcp_hosts() -> Vec { .collect() } +/// A single `host:port` endpoint for direct TCP bypass. +#[derive(Debug, Clone, PartialEq, Eq)] +struct DirectTcpEndpoint { + host: String, + port: u16, +} + +/// Parse `OPENSHELL_DIRECT_TCP_ENDPOINTS` (comma-separated `host:port` pairs). +/// +/// Each entry bypasses the egress proxy with per-endpoint iptables ACCEPT on +/// the sandbox side and MASQUERADE + FORWARD on the host side. Use for arbitrary +/// TCP ports beyond 443 (postgres 5432, redis 6379, smtp 1025, etc.) that the +/// egress proxy rejects or that raw-TCP clients need. +/// +/// `host` may be an IPv4 literal or a hostname. Hostnames are resolved at pod +/// startup via the cluster resolver. +fn parse_direct_tcp_endpoints() -> Vec { + let raw = match std::env::var("OPENSHELL_DIRECT_TCP_ENDPOINTS") { + Ok(val) if !val.is_empty() => val, + _ => return Vec::new(), + }; + let mut out = Vec::new(); + for entry in raw.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + continue; + } + let Some((host, port)) = entry.rsplit_once(':') else { + warn!(entry = %entry, "OPENSHELL_DIRECT_TCP_ENDPOINTS entry missing ':port'"); + continue; + }; + let host = host.trim().trim_start_matches('[').trim_end_matches(']'); + let port: u16 = match port.trim().parse() { + Ok(p) => p, + Err(e) => { + warn!(entry = %entry, error = %e, "Invalid port in OPENSHELL_DIRECT_TCP_ENDPOINTS"); + continue; + } + }; + if host.is_empty() { + continue; + } + out.push(DirectTcpEndpoint { host: host.to_owned(), port }); + } + out +} + +/// Resolve a direct-TCP endpoint to one or more IPv4 addresses. +/// +/// IPv4 literals pass through unchanged. Hostnames are resolved via the system +/// resolver (running in the pod netns where cluster DNS works). IPv6 addresses +/// are dropped — sandbox netns rules are IPv4-only. +fn resolve_endpoint_ipv4s(ep: &DirectTcpEndpoint) -> Vec { + if let Ok(addr) = ep.host.parse::() { + return vec![addr]; + } + match std::net::ToSocketAddrs::to_socket_addrs(&(ep.host.as_str(), ep.port)) { + Ok(iter) => { + let mut seen = Vec::new(); + for sa in iter { + if let std::net::IpAddr::V4(ip) = sa.ip() { + if !seen.contains(&ip) { + seen.push(ip); + } + } + } + if seen.is_empty() { + warn!(host = %ep.host, "No IPv4 address resolved for direct-TCP endpoint"); + } + seen + } + Err(e) => { + warn!(host = %ep.host, error = %e, "Failed to resolve direct-TCP endpoint"); + Vec::new() + } + } +} + /// Resolve the cluster DNS server IP for the iptables ACCEPT rule. /// /// Priority: @@ -412,6 +490,45 @@ impl NetworkNamespace { ); } + // Host-side forwarding for OPENSHELL_DIRECT_TCP_ENDPOINTS (host:port + // pairs). Per-endpoint MASQUERADE + FORWARD on the specific dest IP and + // port so the sandbox can reach services on the pod host (e.g. postgres + // 5432, redis 6379) without going through the egress proxy — which + // blocks well-known DB ports and does not handle raw TCP protocols. + let direct_tcp_endpoints = parse_direct_tcp_endpoints(); + if !direct_tcp_endpoints.is_empty() { + let sandbox_cidr = format!("{}/32", self.sandbox_ip); + let mut installed = 0usize; + for ep in &direct_tcp_endpoints { + let port_str = ep.port.to_string(); + for ip in resolve_endpoint_ipv4s(ep) { + let ip_cidr = format!("{ip}/32"); + let _ = Command::new(&iptables_path) + .args([ + "-t", "nat", "-A", "POSTROUTING", + "-s", &sandbox_cidr, "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "MASQUERADE", + ]) + .output(); + let _ = Command::new(&iptables_path) + .args([ + "-A", "FORWARD", + "-s", &sandbox_cidr, "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "ACCEPT", + ]) + .output(); + installed += 1; + } + } + info!( + endpoints = direct_tcp_endpoints.len(), + rules = installed, + "Enabled direct TCP forwarding for OPENSHELL_DIRECT_TCP_ENDPOINTS" + ); + } + openshell_ocsf::ocsf_emit!( openshell_ocsf::ConfigStateChangeBuilder::new(crate::ocsf_ctx()) .severity(openshell_ocsf::SeverityId::Informational) @@ -537,6 +654,49 @@ impl NetworkNamespace { } } + // Rule 4.6: ACCEPT per-endpoint direct TCP for OPENSHELL_DIRECT_TCP_ENDPOINTS. + // + // Unlike DIRECT_TCP_HOSTS (broad TCP 443), endpoints are dest-IP + dport + // pairs — required for non-HTTPS services the proxy can't/won't handle: + // postgres/redis wire protocols, and ports the proxy explicitly blocks + // (e.g. 5432/6379 hardcoded). + let endpoints = parse_direct_tcp_endpoints(); + if !endpoints.is_empty() { + let mut accepted = 0usize; + for ep in &endpoints { + let port_str = ep.port.to_string(); + for ip in resolve_endpoint_ipv4s(ep) { + let ip_cidr = format!("{ip}/32"); + if let Err(e) = run_iptables_netns( + &self.name, + iptables_cmd, + &[ + "-A", "OUTPUT", + "-d", &ip_cidr, + "-p", "tcp", "--dport", &port_str, + "-j", "ACCEPT", + ], + ) { + warn!( + error = %e, + ip = %ip, + port = ep.port, + "Failed to install direct TCP endpoint ACCEPT rule" + ); + } else { + accepted += 1; + } + } + } + if accepted > 0 { + info!( + rules = accepted, + endpoints = endpoints.len(), + "Installed direct TCP endpoint ACCEPT rules for OPENSHELL_DIRECT_TCP_ENDPOINTS" + ); + } + } + // Rule 5: REJECT TCP bypass attempts (fast-fail) run_iptables_netns( &self.name, @@ -1011,6 +1171,47 @@ mod tests { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); } + #[test] + fn test_parse_direct_tcp_endpoints_basic() { + std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", + ); + let eps = parse_direct_tcp_endpoints(); + assert_eq!( + eps, + vec![ + DirectTcpEndpoint { host: "10.0.1.215".into(), port: 5432 }, + DirectTcpEndpoint { host: "10.0.1.215".into(), port: 6379 }, + DirectTcpEndpoint { host: "db.internal".into(), port: 1025 }, + ] + ); + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + } + + #[test] + fn test_parse_direct_tcp_endpoints_invalid_entries_skipped() { + std::env::set_var( + "OPENSHELL_DIRECT_TCP_ENDPOINTS", + "host-no-port, :5432, host:abc, good.internal:8025", + ); + let eps = parse_direct_tcp_endpoints(); + assert_eq!( + eps, + vec![DirectTcpEndpoint { host: "good.internal".into(), port: 8025 }] + ); + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + } + + #[test] + fn test_parse_direct_tcp_endpoints_empty() { + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + assert!(parse_direct_tcp_endpoints().is_empty()); + std::env::set_var("OPENSHELL_DIRECT_TCP_ENDPOINTS", ""); + assert!(parse_direct_tcp_endpoints().is_empty()); + std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + } + #[test] #[ignore = "requires root privileges"] fn test_create_and_drop_namespace() { From 49ce5baaa0756ed667f5927c8a0c6dea977b2b02 Mon Sep 17 00:00:00 2001 From: shirow ozawa Date: Thu, 8 Oct 2026 13:49:04 +0900 Subject: [PATCH 07/10] test(sandbox): wrap env mutations in unsafe blocks for edition 2024 The direct-TCP tests call std::env::set_var / remove_var directly. These are unsafe in edition 2024, so `cargo test -p openshell-sandbox --lib` failed to compile (19 errors in netns.rs and child_env.rs) and no sandbox test could run. Wrap each call in an unsafe block; behavior is unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/openshell-sandbox/src/child_env.rs | 25 +++++++---- .../src/sandbox/linux/netns.rs | 42 ++++++++++++------- 2 files changed, 43 insertions(+), 24 deletions(-) diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index 2e615b8f8b..27571f1d56 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -80,7 +80,8 @@ mod tests { #[test] fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() { // Ensure no leftover env from other tests affects NO_PROXY - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; let mut cmd = Command::new("/usr/bin/env"); cmd.stdin(Stdio::null()) @@ -102,11 +103,13 @@ mod tests { #[test] fn no_proxy_includes_direct_tcp_hosts() { - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); - std::env::set_var( + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_HOSTS", "oauth2.googleapis.com,gmail.googleapis.com", - ); + ) }; let no_proxy = build_no_proxy(); assert_eq!( @@ -115,16 +118,19 @@ mod tests { ); // Clean up - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn no_proxy_includes_direct_tcp_endpoints() { - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); - std::env::set_var( + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", - ); + ) }; let no_proxy = build_no_proxy(); assert_eq!( @@ -132,7 +138,8 @@ mod tests { "127.0.0.1,localhost,::1,10.0.1.215,10.0.1.215,db.internal" ); - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index bd24e647bc..b495b5f9e3 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -1152,31 +1152,37 @@ mod tests { #[test] fn test_parse_direct_tcp_hosts() { - std::env::set_var( + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_HOSTS", "oauth2.googleapis.com, gmail.googleapis.com , ", - ); + ) }; let hosts = parse_direct_tcp_hosts(); assert_eq!(hosts, vec!["oauth2.googleapis.com", "gmail.googleapis.com"]); - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn test_parse_direct_tcp_hosts_empty() { - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; assert!(parse_direct_tcp_hosts().is_empty()); - std::env::set_var("OPENSHELL_DIRECT_TCP_HOSTS", ""); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_HOSTS", "") }; assert!(parse_direct_tcp_hosts().is_empty()); - std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn test_parse_direct_tcp_endpoints_basic() { - std::env::set_var( + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", - ); + ) }; let eps = parse_direct_tcp_endpoints(); assert_eq!( eps, @@ -1186,30 +1192,36 @@ mod tests { DirectTcpEndpoint { host: "db.internal".into(), port: 1025 }, ] ); - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] fn test_parse_direct_tcp_endpoints_invalid_entries_skipped() { - std::env::set_var( + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "host-no-port, :5432, host:abc, good.internal:8025", - ); + ) }; let eps = parse_direct_tcp_endpoints(); assert_eq!( eps, vec![DirectTcpEndpoint { host: "good.internal".into(), port: 8025 }] ); - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] fn test_parse_direct_tcp_endpoints_empty() { - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; assert!(parse_direct_tcp_endpoints().is_empty()); - std::env::set_var("OPENSHELL_DIRECT_TCP_ENDPOINTS", ""); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_ENDPOINTS", "") }; assert!(parse_direct_tcp_endpoints().is_empty()); - std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS"); + // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] From 78010598f40799e25435ac8321be807f2ef47de0 Mon Sep 17 00:00:00 2001 From: shirow ozawa Date: Thu, 8 Oct 2026 13:49:04 +0900 Subject: [PATCH 08/10] feat(sandbox): resolve provider-shaped credential aliases Port the alias resolution from NVIDIA/OpenShell#1286 (header and request-line scope only). A client that validates the credential's shape before sending cannot carry the canonical `openshell:resolve:env:KEY` placeholder; for example gh's `--attach` only accepts github_pat_ / ghp_ / gho_ / ghu_ tokens. Such a client can send `OPENSHELL-RESOLVE-ENV-KEY` (e.g. `github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN`), and the proxy now resolves it to the same secret as the canonical placeholder. - resolve_placeholder falls back to the alias when there is no exact match. The alias must be the whole token: a non-empty prefix of RFC 3986 unreserved characters, the marker, and an env key running to the end. - The fail-closed scans after rewriting also reject a remaining alias marker in the header block and in the percent-decoded request line, so an unresolved alias is never forwarded upstream. The child env keeps the canonical placeholder; callers opt in to an alias per invocation (GH_TOKEN=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN gh ...). Aliases inside Basic credentials are not resolved; git keeps using the canonical placeholder. Refs simount/NemoClaw-on-AWS#254 Co-Authored-By: Claude Opus 5.5 --- crates/openshell-sandbox/src/secrets.rs | 189 +++++++++++++++++++++++- 1 file changed, 186 insertions(+), 3 deletions(-) diff --git a/crates/openshell-sandbox/src/secrets.rs b/crates/openshell-sandbox/src/secrets.rs index a27537c910..085e46139a 100644 --- a/crates/openshell-sandbox/src/secrets.rs +++ b/crates/openshell-sandbox/src/secrets.rs @@ -10,12 +10,55 @@ const PLACEHOLDER_PREFIX: &str = "openshell:resolve:env:"; /// Public access to the placeholder prefix for fail-closed scanning in other modules. pub(crate) const PLACEHOLDER_PREFIX_PUBLIC: &str = PLACEHOLDER_PREFIX; +/// Marker of a provider-shaped alias: `OPENSHELL-RESOLVE-ENV-`. +/// +/// Some clients validate the credential's shape before sending it (e.g. gh's +/// `--attach` only accepts `github_pat_` / `ghp_` / ... tokens) and cannot carry +/// the canonical `openshell:resolve:env:KEY` placeholder. They send an alias +/// such as `github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN` instead, which the +/// proxy resolves to the same secret as the canonical placeholder. +/// Ported from NVIDIA/OpenShell#1286 (header and request-line scope only). +const PROVIDER_ALIAS_MARKER: &str = "OPENSHELL-RESOLVE-ENV-"; + /// Characters that are valid in an env var key name (used to extract /// placeholder boundaries within concatenated strings like path segments). fn is_env_key_char(b: u8) -> bool { b.is_ascii_alphanumeric() || b == b'_' } +/// Characters allowed in the shape prefix of a provider-shaped alias +/// (RFC 3986 unreserved characters). +fn is_alias_token_char(b: u8) -> bool { + b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-' | b'.' | b'~') +} + +/// Extract the env key from a whole-token provider-shaped alias. +/// +/// The token must be `OPENSHELL-RESOLVE-ENV-` where the prefix is +/// non-empty and made of unreserved characters, and the key runs to the end of +/// the token. Anything else is not an alias. +fn alias_env_key(token: &str) -> Option<&str> { + let marker_start = token.find(PROVIDER_ALIAS_MARKER)?; + if marker_start == 0 { + return None; + } + if !token[..marker_start].bytes().all(is_alias_token_char) { + return None; + } + let key_start = marker_start + PROVIDER_ALIAS_MARKER.len(); + let key_end = token[key_start..] + .bytes() + .position(|b| !is_env_key_char(b)) + .map_or(token.len(), |p| key_start + p); + (key_end == token.len() && key_end > key_start).then_some(&token[key_start..key_end]) +} + +/// True if the text still carries a credential placeholder or alias marker. +/// Used by the fail-closed scans after rewriting. +fn contains_credential_marker(text: &str) -> bool { + text.contains(PLACEHOLDER_PREFIX) || text.contains(PROVIDER_ALIAS_MARKER) +} + // --------------------------------------------------------------------------- // Error and result types // --------------------------------------------------------------------------- @@ -91,7 +134,15 @@ impl SecretResolver { /// Returns `None` if the placeholder is unknown or the resolved value /// contains prohibited control characters (CRLF, null byte). pub(crate) fn resolve_placeholder(&self, value: &str) -> Option<&str> { - let secret = self.by_placeholder.get(value).map(String::as_str)?; + let secret = match self.by_placeholder.get(value) { + Some(secret) => secret.as_str(), + None => { + let key = alias_env_key(value)?; + self.by_placeholder + .get(&placeholder_for_env_key(key)) + .map(String::as_str)? + } + }; match validate_resolved_secret(secret) { Ok(s) => Some(s), Err(reason) => { @@ -610,14 +661,14 @@ pub(crate) fn rewrite_http_header_block( // Fail-closed scan: check for any remaining unresolved placeholders // in both raw form and percent-decoded form of the output header block. let output_header = String::from_utf8_lossy(&output[..output.len().min(header_end + 256)]); - if output_header.contains(PLACEHOLDER_PREFIX) { + if contains_credential_marker(&output_header) { return Err(UnresolvedPlaceholderError { location: "header" }); } // Also check percent-decoded form of the request line (F5 — encoded placeholder bypass) let rewritten_rl = output_header.split("\r\n").next().unwrap_or(""); let decoded_rl = percent_decode(rewritten_rl); - if decoded_rl.contains(PLACEHOLDER_PREFIX) { + if contains_credential_marker(&decoded_rl) { return Err(UnresolvedPlaceholderError { location: "path" }); } @@ -1473,4 +1524,136 @@ mod tests { assert_eq!(result.resolved, "/bottok123/method?key=key456"); assert_eq!(result.redacted, "/bot[CREDENTIAL]/method?key=[CREDENTIAL]"); } + + // === Provider-shaped aliases (ported from NVIDIA/OpenShell#1286) === + // + // A client that validates the credential's shape before sending (e.g. gh's + // `--attach`, which only accepts `github_pat_` / `ghp_` / ... tokens) cannot + // carry the canonical `openshell:resolve:env:KEY` placeholder. Such a client + // sends `OPENSHELL-RESOLVE-ENV-KEY` instead, and the proxy must + // resolve it to the same secret as the canonical placeholder. + + fn github_resolver() -> SecretResolver { + let (_, resolver) = SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "github_pat_REAL".to_string())] + .into_iter() + .collect(), + ); + resolver.expect("resolver") + } + + #[test] + fn provider_alias_resolves_to_the_canonical_secret() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + Some("github_pat_REAL") + ); + } + + #[test] + fn provider_alias_in_token_scheme_header_is_rewritten() { + let resolver = github_resolver(); + assert_eq!( + rewrite_header_line( + "Authorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", + &resolver, + ), + "Authorization: token github_pat_REAL" + ); + } + + #[test] + fn provider_alias_accepts_unreserved_prefix_characters() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("a.b~c-d_e9OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + Some("github_pat_REAL") + ); + } + + #[test] + fn provider_alias_with_unknown_key_is_not_resolved() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_requires_a_non_empty_prefix() { + let resolver = github_resolver(); + assert_eq!(resolver.resolve_placeholder("OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), None); + } + + #[test] + fn provider_alias_rejects_prefix_with_reserved_characters() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("a/b:OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_must_end_at_the_key() { + let resolver = github_resolver(); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN!x"), + None + ); + assert_eq!(resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-"), None); + } + + #[test] + fn provider_alias_resolution_rejects_crlf_secret() { + let (_, resolver) = SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "bad\r\nX-Injected: 1".to_string())] + .into_iter() + .collect(), + ); + let resolver = resolver.expect("resolver"); + assert_eq!( + resolver.resolve_placeholder("github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN"), + None + ); + } + + #[test] + fn provider_alias_header_block_round_trip() { + let resolver = github_resolver(); + let raw = b"POST /user-attachments/assets?name=a.webm HTTP/1.1\r\nHost: uploads.github.com\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN\r\nContent-Length: 4\r\n\r\nbody"; + let result = rewrite_http_header_block(raw, Some(&resolver)).expect("should resolve"); + let rewritten = String::from_utf8(result.rewritten).expect("utf8"); + assert!(rewritten.contains("Authorization: token github_pat_REAL\r\n")); + assert!(!rewritten.contains("OPENSHELL-RESOLVE-ENV-")); + assert!(rewritten.ends_with("\r\n\r\nbody")); + } + + #[test] + fn unresolved_provider_alias_in_header_fails_closed() { + let resolver = github_resolver(); + let raw = b"GET /user HTTP/1.1\r\nHost: api.github.com\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN\r\n\r\n"; + let err = rewrite_http_header_block(raw, Some(&resolver)).expect_err("must fail closed"); + assert_eq!(err.location, "header"); + } + + #[test] + fn provider_alias_in_request_line_fails_closed() { + let resolver = github_resolver(); + let raw = b"GET /x?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN HTTP/1.1\r\nHost: api.github.com\r\n\r\n"; + assert!(rewrite_http_header_block(raw, Some(&resolver)).is_err()); + let encoded = b"GET /x%3Ft%3Dgithub_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN HTTP/1.1\r\nHost: api.github.com\r\n\r\n"; + assert!(rewrite_http_header_block(encoded, Some(&resolver)).is_err()); + } + + #[test] + fn canonical_placeholder_still_resolves_after_alias_support() { + let resolver = github_resolver(); + assert_eq!( + rewrite_header_line("Authorization: token openshell:resolve:env:GITHUB_TOKEN", &resolver), + "Authorization: token github_pat_REAL" + ); + } } From 16152ef946098956529efe41c4b03147084eba25 Mon Sep 17 00:00:00 2001 From: shirow ozawa Date: Fri, 9 Oct 2026 13:50:31 +0900 Subject: [PATCH 09/10] fix(sandbox): fail closed on provider aliases in the forward proxy and request target The forward-proxy path (rewrite_forward_request) still scanned only for the canonical placeholder after rewriting, so an unknown alias in a header, or any alias in the request line, was forwarded upstream. rewrite_target_for_eval also let an alias in the target through to OPA. - rewrite_forward_request: reject a remaining placeholder or alias marker in the output, and in the raw and percent-decoded request line - rewrite_target_for_eval: reject an alias marker in the target (raw and percent-decoded). Aliases are resolved in headers only, so one in the target would never be resolved - share the marker checks (contains_credential_marker, request_line_has_credential_marker) and drop the now-unused PLACEHOLDER_PREFIX_PUBLIC Addresses review: https://github.com/simount/OpenShell/pull/4#discussion_r4226572075 Co-Authored-By: Claude Opus 5.5 --- crates/openshell-sandbox/src/proxy.rs | 62 ++++++++++++++++++++++++- crates/openshell-sandbox/src/secrets.rs | 31 +++++++++++-- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index b52cc60b90..8cd561a524 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -1916,10 +1916,15 @@ fn rewrite_forward_request( output.extend_from_slice(&raw[header_end..used]); } - // Fail-closed: scan for any remaining unresolved placeholders + // Fail-closed: scan for any remaining unresolved placeholders or + // provider-shaped aliases, including the percent-decoded request line. if secret_resolver.is_some() { let output_str = String::from_utf8_lossy(&output); - if output_str.contains(crate::secrets::PLACEHOLDER_PREFIX_PUBLIC) { + let request_line = output_str.split("\r\n").next().unwrap_or(""); + if crate::secrets::request_line_has_credential_marker(request_line) { + return Err(crate::secrets::UnresolvedPlaceholderError { location: "path" }); + } + if crate::secrets::contains_credential_marker(&output_str) { return Err(crate::secrets::UnresolvedPlaceholderError { location: "header" }); } } @@ -3170,6 +3175,59 @@ mod tests { assert!(!result_str.contains("openshell:resolve:env:ANTHROPIC_API_KEY")); } + fn github_alias_resolver() -> Option { + SecretResolver::from_provider_env( + [("GITHUB_TOKEN".to_string(), "github_pat_REAL".to_string())] + .into_iter() + .collect(), + ) + .1 + } + + #[test] + fn test_rewrite_resolves_provider_alias_auth_header() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN\r\n\r\n"; + let result = rewrite_forward_request(raw, raw.len(), "/p", resolver.as_ref()) + .expect("should succeed"); + let result_str = String::from_utf8_lossy(&result); + assert!(result_str.contains("Authorization: token github_pat_REAL")); + assert!(!result_str.contains("OPENSHELL-RESOLVE-ENV-")); + } + + #[test] + fn test_rewrite_rejects_unknown_provider_alias() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\nAuthorization: token github_pat_OPENSHELL-RESOLVE-ENV-OTHER_TOKEN\r\n\r\n"; + let err = rewrite_forward_request(raw, raw.len(), "/p", resolver.as_ref()) + .expect_err("unresolved alias must fail closed"); + assert_eq!(err.location, "header"); + } + + #[test] + fn test_rewrite_rejects_provider_alias_in_path() { + let resolver = github_alias_resolver(); + let raw = b"GET http://host/p HTTP/1.1\r\nHost: host\r\n\r\n"; + assert!( + rewrite_forward_request( + raw, + raw.len(), + "/p?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", + resolver.as_ref(), + ) + .is_err() + ); + assert!( + rewrite_forward_request( + raw, + raw.len(), + "/p?t=github_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN", + resolver.as_ref(), + ) + .is_err() + ); + } + // --- Forward proxy SSRF defence tests --- // // The forward proxy handler uses the same SSRF logic as the CONNECT path: diff --git a/crates/openshell-sandbox/src/secrets.rs b/crates/openshell-sandbox/src/secrets.rs index 085e46139a..dfb9c5c756 100644 --- a/crates/openshell-sandbox/src/secrets.rs +++ b/crates/openshell-sandbox/src/secrets.rs @@ -7,8 +7,6 @@ use std::fmt; const PLACEHOLDER_PREFIX: &str = "openshell:resolve:env:"; -/// Public access to the placeholder prefix for fail-closed scanning in other modules. -pub(crate) const PLACEHOLDER_PREFIX_PUBLIC: &str = PLACEHOLDER_PREFIX; /// Marker of a provider-shaped alias: `OPENSHELL-RESOLVE-ENV-`. /// @@ -55,10 +53,16 @@ fn alias_env_key(token: &str) -> Option<&str> { /// True if the text still carries a credential placeholder or alias marker. /// Used by the fail-closed scans after rewriting. -fn contains_credential_marker(text: &str) -> bool { +pub(crate) fn contains_credential_marker(text: &str) -> bool { text.contains(PLACEHOLDER_PREFIX) || text.contains(PROVIDER_ALIAS_MARKER) } +/// True if a request line carries a placeholder or alias marker in its raw or +/// percent-decoded form (F5 — encoded placeholder bypass). +pub(crate) fn request_line_has_credential_marker(line: &str) -> bool { + contains_credential_marker(line) || contains_credential_marker(&percent_decode(line)) +} + // --------------------------------------------------------------------------- // Error and result types // --------------------------------------------------------------------------- @@ -697,6 +701,13 @@ pub(crate) fn rewrite_target_for_eval( target: &str, resolver: &SecretResolver, ) -> Result { + // Provider-shaped aliases are resolved in headers only. An alias in the + // target would never be resolved, so fail closed before it reaches OPA or + // the upstream (raw and percent-decoded forms). + if target.contains(PROVIDER_ALIAS_MARKER) || percent_decode(target).contains(PROVIDER_ALIAS_MARKER) { + return Err(UnresolvedPlaceholderError { location: "path" }); + } + if !target.contains(PLACEHOLDER_PREFIX) { // Also check percent-decoded form let decoded = percent_decode(target); @@ -1648,6 +1659,20 @@ mod tests { assert!(rewrite_http_header_block(encoded, Some(&resolver)).is_err()); } + #[test] + fn rewrite_target_for_eval_rejects_provider_alias() { + // Aliases are resolved in headers only. One in the request target is + // never resolved, so it must fail closed instead of reaching OPA and upstream. + let resolver = github_resolver(); + let err = rewrite_target_for_eval("/x?t=github_pat_OPENSHELL-RESOLVE-ENV-GITHUB_TOKEN", &resolver) + .expect_err("alias in target must fail closed"); + assert_eq!(err.location, "path"); + assert!( + rewrite_target_for_eval("/x?t=github_pat_OPENSHELL%2DRESOLVE%2DENV%2DGITHUB_TOKEN", &resolver) + .is_err() + ); + } + #[test] fn canonical_placeholder_still_resolves_after_alias_support() { let resolver = github_resolver(); From 39f7b66966e1d3e046c3268610a87839b54b8611 Mon Sep 17 00:00:00 2001 From: shirow ozawa Date: Fri, 9 Oct 2026 13:50:31 +0900 Subject: [PATCH 10/10] test(sandbox): serialize tests that touch OPENSHELL_DIRECT_TCP_* env vars Wrapping set_var / remove_var in unsafe blocks did not meet their safety contract: tests run in parallel, and the tests in child_env.rs and sandbox/linux/netns.rs read and write the same variables. Every such test now holds a shared lock (child_env::lock_direct_tcp_env) for its whole body, including the ones that only read through build_no_proxy / proxy_env_vars. Addresses review: https://github.com/simount/OpenShell/pull/4#discussion_r4226572115 https://github.com/simount/OpenShell/pull/4#discussion_r4226572149 Co-Authored-By: Claude Opus 5.5 --- crates/openshell-sandbox/src/child_env.rs | 29 ++++++++++++++----- .../src/sandbox/linux/netns.rs | 29 +++++++++++-------- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/crates/openshell-sandbox/src/child_env.rs b/crates/openshell-sandbox/src/child_env.rs index 27571f1d56..014ded65a4 100644 --- a/crates/openshell-sandbox/src/child_env.rs +++ b/crates/openshell-sandbox/src/child_env.rs @@ -71,6 +71,18 @@ pub(crate) fn tls_env_vars( ] } +/// Serializes tests that read or write `OPENSHELL_DIRECT_TCP_*`. +/// +/// `std::env::set_var` / `remove_var` are unsafe because a concurrent read or +/// write of the environment is undefined behavior. Tests run in parallel by +/// default, and the tests in this module and in `sandbox::linux::netns` touch +/// the same variables, so every such test holds this lock for its whole body. +#[cfg(test)] +pub(crate) fn lock_direct_tcp_env() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + #[cfg(test)] mod tests { use super::*; @@ -79,8 +91,9 @@ mod tests { #[test] fn apply_proxy_env_includes_node_proxy_opt_in_and_local_bypass() { + let _env = lock_direct_tcp_env(); // Ensure no leftover env from other tests affects NO_PROXY - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; let mut cmd = Command::new("/usr/bin/env"); @@ -103,9 +116,10 @@ mod tests { #[test] fn no_proxy_includes_direct_tcp_hosts() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_HOSTS", "oauth2.googleapis.com,gmail.googleapis.com", @@ -118,15 +132,16 @@ mod tests { ); // Clean up - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn no_proxy_includes_direct_tcp_endpoints() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", @@ -138,7 +153,7 @@ mod tests { "127.0.0.1,localhost,::1,10.0.1.215,10.0.1.215,db.internal" ); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } diff --git a/crates/openshell-sandbox/src/sandbox/linux/netns.rs b/crates/openshell-sandbox/src/sandbox/linux/netns.rs index b495b5f9e3..3d315cf299 100644 --- a/crates/openshell-sandbox/src/sandbox/linux/netns.rs +++ b/crates/openshell-sandbox/src/sandbox/linux/netns.rs @@ -1152,33 +1152,36 @@ mod tests { #[test] fn test_parse_direct_tcp_hosts() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_HOSTS", "oauth2.googleapis.com, gmail.googleapis.com , ", ) }; let hosts = parse_direct_tcp_hosts(); assert_eq!(hosts, vec!["oauth2.googleapis.com", "gmail.googleapis.com"]); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn test_parse_direct_tcp_hosts_empty() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; assert!(parse_direct_tcp_hosts().is_empty()); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_HOSTS", "") }; assert!(parse_direct_tcp_hosts().is_empty()); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_HOSTS") }; } #[test] fn test_parse_direct_tcp_endpoints_basic() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "10.0.1.215:5432, 10.0.1.215:6379 , db.internal:1025,", @@ -1192,13 +1195,14 @@ mod tests { DirectTcpEndpoint { host: "db.internal".into(), port: 1025 }, ] ); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] fn test_parse_direct_tcp_endpoints_invalid_entries_skipped() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var( "OPENSHELL_DIRECT_TCP_ENDPOINTS", "host-no-port, :5432, host:abc, good.internal:8025", @@ -1208,19 +1212,20 @@ mod tests { eps, vec![DirectTcpEndpoint { host: "good.internal".into(), port: 8025 }] ); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; } #[test] fn test_parse_direct_tcp_endpoints_empty() { - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + let _env = crate::child_env::lock_direct_tcp_env(); + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; assert!(parse_direct_tcp_endpoints().is_empty()); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::set_var("OPENSHELL_DIRECT_TCP_ENDPOINTS", "") }; assert!(parse_direct_tcp_endpoints().is_empty()); - // SAFETY: test-only env mutation, wrapped for edition 2024 (behavior unchanged). + // SAFETY: test-only; all tests touching these env vars hold lock_direct_tcp_env(). unsafe { std::env::remove_var("OPENSHELL_DIRECT_TCP_ENDPOINTS") }; }