From 395aadab23f6c7bcdfebb5991234a0ff0b474903 Mon Sep 17 00:00:00 2001
From: Ammar Bandukwala <ammar@ammar.io>
Date: Thu, 11 Sep 2025 09:25:35 -0500
Subject: [PATCH] Preserve existing NO_PROXY in weak jail

---
 src/jail/weak.rs          | 13 ++++++++--
 tests/common/mod.rs       | 14 +++++++++++
 tests/weak_integration.rs | 51 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/src/jail/weak.rs b/src/jail/weak.rs
index fca2bf5d..9835cd55 100644
--- a/src/jail/weak.rs
+++ b/src/jail/weak.rs
@@ -56,8 +56,17 @@ impl Jail for WeakJail {
         cmd.env("https_proxy", &https_proxy);
 
         // Also set NO_PROXY for localhost to avoid proxying local connections
-        cmd.env("NO_PROXY", "localhost,127.0.0.1,::1");
-        cmd.env("no_proxy", "localhost,127.0.0.1,::1");
+        // Preserve any existing NO_PROXY settings by appending them
+        let mut no_proxy_hosts = "localhost,127.0.0.1,::1".to_string();
+
+        if let Ok(existing) = std::env::var("NO_PROXY").or_else(|_| std::env::var("no_proxy"))
+            && !existing.is_empty()
+        {
+            no_proxy_hosts = format!("{},{}", existing, no_proxy_hosts);
+        }
+
+        cmd.env("NO_PROXY", &no_proxy_hosts);
+        cmd.env("no_proxy", &no_proxy_hosts);
 
         // Set any extra environment variables
         for (key, value) in extra_env {
diff --git a/tests/common/mod.rs b/tests/common/mod.rs
index 0eecb5c9..7a015843 100644
--- a/tests/common/mod.rs
+++ b/tests/common/mod.rs
@@ -24,6 +24,7 @@ pub struct HttpjailCommand {
     args: Vec<String>,
     use_sudo: bool,
     weak_mode: bool,
+    env: Vec<(String, String)>,
 }
 
 impl HttpjailCommand {
@@ -33,6 +34,7 @@ impl HttpjailCommand {
             args: vec![],
             use_sudo: false,
             weak_mode: false,
+            env: vec![],
         }
     }
 
@@ -70,6 +72,12 @@ impl HttpjailCommand {
         self
     }
 
+    /// Set an environment variable for the httpjail process
+    pub fn env(mut self, key: &str, value: &str) -> Self {
+        self.env.push((key.to_string(), value.to_string()));
+        self
+    }
+
     /// Build and execute the command
     pub fn execute(mut self) -> Result<(i32, String, String), String> {
         // Ensure httpjail is built
@@ -104,12 +112,18 @@ impl HttpjailCommand {
             for arg in &self.args {
                 sudo_cmd.arg(arg);
             }
+            for (key, value) in &self.env {
+                sudo_cmd.env(key, value);
+            }
             sudo_cmd
         } else {
             let mut cmd = Command::new(&httpjail_path);
             for arg in &self.args {
                 cmd.arg(arg);
             }
+            for (key, value) in &self.env {
+                cmd.env(key, value);
+            }
             cmd
         };
 
diff --git a/tests/weak_integration.rs b/tests/weak_integration.rs
index 41cff5b8..28765d9c 100644
--- a/tests/weak_integration.rs
+++ b/tests/weak_integration.rs
@@ -123,6 +123,57 @@ fn test_weak_mode_allows_localhost() {
     }
 }
 
+#[test]
+fn test_weak_mode_appends_no_proxy() {
+    // Ensure existing NO_PROXY values are preserved and localhost entries appended
+    let result = HttpjailCommand::new()
+        .weak()
+        .rule("allow: .*")
+        .env("NO_PROXY", "example.com")
+        .verbose(2)
+        .command(vec!["env"])
+        .execute();
+
+    match result {
+        Ok((exit_code, stdout, _stderr)) => {
+            assert_eq!(exit_code, 0, "env command should succeed");
+
+            let mut found_upper = false;
+            let mut found_lower = false;
+
+            for line in stdout.lines() {
+                if let Some((key, value)) = line.split_once('=') {
+                    if key == "NO_PROXY" {
+                        found_upper = true;
+                        assert!(
+                            value.contains("example.com")
+                                && value.contains("localhost")
+                                && value.contains("127.0.0.1")
+                                && value.contains("::1"),
+                            "NO_PROXY missing expected entries: {}",
+                            value
+                        );
+                    } else if key == "no_proxy" {
+                        found_lower = true;
+                        assert!(
+                            value.contains("example.com")
+                                && value.contains("localhost")
+                                && value.contains("127.0.0.1")
+                                && value.contains("::1"),
+                            "no_proxy missing expected entries: {}",
+                            value
+                        );
+                    }
+                }
+            }
+
+            assert!(found_upper, "NO_PROXY variable not found");
+            assert!(found_lower, "no_proxy variable not found");
+        }
+        Err(e) => panic!("Failed to execute httpjail: {}", e),
+    }
+}
+
 // Simple server start function - we know the ports we're setting
 fn start_server(http_port: u16, https_port: u16) -> Result<std::process::Child, String> {
     let httpjail_path = build_httpjail()?;