feat(sandbox): Landlock PTY + direct TCP + DNS (+ #708 basic auth credential resolve)

crates/openshell-sandbox/src/child_env.rs

@@ -5,15 +5,51 @@ 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` / `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(',') {
+            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
+}
+
 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.
@@ -35,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::*;
@@ -43,6 +91,11 @@ 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; 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");
         cmd.stdin(Stdio::null())
             .stdout(Stdio::piped())
@@ -61,6 +114,49 @@ mod tests {
         assert!(stdout.contains("no_proxy=127.0.0.1,localhost,::1"));
     }
 
+    #[test]
+    fn no_proxy_includes_direct_tcp_hosts() {
+        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; 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 no_proxy = build_no_proxy();
+        assert_eq!(
+            no_proxy,
+            "127.0.0.1,localhost,::1,oauth2.googleapis.com,gmail.googleapis.com"
+        );
+
+        // Clean up
+        // 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() {
+        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; 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,",
+        ) };
+
+        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"
+        );
+
+        // 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 apply_tls_env_sets_node_and_bundle_paths() {
         let mut cmd = Command::new("/usr/bin/env");

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(())

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> {
+        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: