fix(security): scrub credential env vars from ShellExecutor subprocess environment (#2449)

Zeph's ShellExecutor inherited the full parent process environment when
spawning bash subcommands, exposing any credentials present in the
process env (e.g. vars set by the user's shell profile) to arbitrary
shell commands executed by the agent.

Add `env_blocklist: Vec<String>` to `ShellConfig` (default-on, covers
ZEPH_*, AWS_*, AZURE_*, GCP_*, GOOGLE_*, OPENAI_*, ANTHROPIC_*, HF_*,
HUGGING*). In `execute_bash`, iterate `std::env::vars()` and call
`cmd.env_remove()` for any key matching a blocklist prefix before
spawning. Skill `extra_env` vars are injected after scrubbing and are
unaffected. Configurable via `[tools.shell] env_blocklist`.

Closes #2449

Author: bug-ops

CHANGELOG.md

@@ -35,6 +35,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - fix(acp): discovery endpoint already reflects `ProtocolVersion::LATEST` — confirmed fixed in PR #2423; no code change required (closes #2412)
 - fix(security): extend MCP env var blocklist — `PATH`, `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, `NO_PROXY`, `BASH_ENV`, `ENV`, `PYTHONPATH`, `NODE_PATH`, `RUBYLIB` are now stripped from ACP-provided env vars for MCP stdio child processes (closes #2437)
 - fix(tools): `AuditLogger::log` now emits `tracing::error!` when `serde_json` serialization fails instead of silently dropping the audit entry (closes #2438)
+- fix(security): scrub credential env vars (`ZEPH_*`, `AWS_*`, `ANTHROPIC_*`, `OPENAI_*`, `AZURE_*`, `GCP_*`, `GOOGLE_*`, `HF_*`, `HUGGING*`) from `ShellExecutor` subprocess environment to prevent exfiltration via shell commands; configurable via `[tools.shell] env_blocklist` (closes #2449)
 
 ### Added (tests)
 

crates/zeph-tools/src/config.rs

@@ -337,6 +337,28 @@ pub struct ShellConfig {
     pub allow_network: bool,
     #[serde(default = "default_confirm_patterns")]
     pub confirm_patterns: Vec<String>,
+    /// Environment variable name prefixes to strip from subprocess environment.
+    /// Variables whose names start with any of these prefixes are removed before
+    /// spawning shell commands. Default covers common credential naming conventions.
+    #[serde(default = "ShellConfig::default_env_blocklist")]
+    pub env_blocklist: Vec<String>,
+}
+
+impl ShellConfig {
+    #[must_use]
+    pub fn default_env_blocklist() -> Vec<String> {
+        vec![
+            "ZEPH_".into(),
+            "AWS_".into(),
+            "AZURE_".into(),
+            "GCP_".into(),
+            "GOOGLE_".into(),
+            "OPENAI_".into(),
+            "ANTHROPIC_".into(),
+            "HF_".into(),
+            "HUGGING".into(),
+        ]
+    }
 }
 
 /// Configuration for audit logging of tool executions.
@@ -379,6 +401,7 @@ impl Default for ShellConfig {
             allowed_paths: Vec::new(),
             allow_network: true,
             confirm_patterns: default_confirm_patterns(),
+            env_blocklist: Self::default_env_blocklist(),
         }
     }
 }

crates/zeph-tools/src/shell/mod.rs

@@ -98,6 +98,7 @@ pub struct ShellExecutor {
     blocked_commands: Vec<String>,
     allowed_paths: Vec<PathBuf>,
     confirm_patterns: Vec<String>,
+    env_blocklist: Vec<String>,
     audit_logger: Option<Arc<AuditLogger>>,
     tool_event_tx: Option<ToolEventTx>,
     permission_policy: Option<PermissionPolicy>,
@@ -145,6 +146,7 @@ impl ShellExecutor {
             blocked_commands: blocked,
             allowed_paths,
             confirm_patterns: config.confirm_patterns.clone(),
+            env_blocklist: config.env_blocklist.clone(),
             audit_logger: None,
             tool_event_tx: None,
             permission_policy: None,
@@ -278,6 +280,7 @@ impl ShellExecutor {
             self.tool_event_tx.as_ref(),
             self.cancel_token.as_ref(),
             skill_env_snapshot.as_ref(),
+            &self.env_blocklist,
         )
         .await;
         if exit_code == 130
@@ -932,6 +935,7 @@ async fn execute_bash(
     event_tx: Option<&ToolEventTx>,
     cancel_token: Option<&CancellationToken>,
     extra_env: Option<&std::collections::HashMap<String, String>>,
+    env_blocklist: &[String],
 ) -> (String, i32) {
     use std::process::Stdio;
     use tokio::io::{AsyncBufReadExt, BufReader};
@@ -943,6 +947,16 @@ async fn execute_bash(
         .arg(code)
         .stdout(Stdio::piped())
         .stderr(Stdio::piped());
+
+    for (key, _) in std::env::vars() {
+        if env_blocklist
+            .iter()
+            .any(|prefix| key.starts_with(prefix.as_str()))
+        {
+            cmd.env_remove(&key);
+        }
+    }
+
     if let Some(env) = extra_env {
         cmd.envs(env);
     }

crates/zeph-tools/src/shell/tests.rs

@@ -11,6 +11,7 @@ fn default_config() -> ShellConfig {
         allowed_paths: Vec::new(),
         allow_network: true,
         confirm_patterns: Vec::new(),
+        env_blocklist: ShellConfig::default_env_blocklist(),
     }
 }
 
@@ -60,15 +61,23 @@ fn unclosed_block_ignored() {
 #[cfg(not(target_os = "windows"))]
 async fn execute_simple_command() {
     let (result, code) =
-        execute_bash("echo hello", Duration::from_secs(30), None, None, None).await;
+        execute_bash("echo hello", Duration::from_secs(30), None, None, None, &[]).await;
     assert!(result.contains("hello"));
     assert_eq!(code, 0);
 }
 
 #[tokio::test]
 #[cfg(not(target_os = "windows"))]
 async fn execute_stderr_output() {
-    let (result, _) = execute_bash("echo err >&2", Duration::from_secs(30), None, None, None).await;
+    let (result, _) = execute_bash(
+        "echo err >&2",
+        Duration::from_secs(30),
+        None,
+        None,
+        None,
+        &[],
+    )
+    .await;
     assert!(result.contains("[stderr]"));
     assert!(result.contains("err"));
 }
@@ -82,6 +91,7 @@ async fn execute_stdout_and_stderr_combined() {
         None,
         None,
         None,
+        &[],
     )
     .await;
     assert!(result.contains("out"));
@@ -93,7 +103,7 @@ async fn execute_stdout_and_stderr_combined() {
 #[tokio::test]
 #[cfg(not(target_os = "windows"))]
 async fn execute_empty_output() {
-    let (result, code) = execute_bash("true", Duration::from_secs(30), None, None, None).await;
+    let (result, code) = execute_bash("true", Duration::from_secs(30), None, None, None, &[]).await;
     assert_eq!(result, "(no output)");
     assert_eq!(code, 0);
 }
@@ -899,6 +909,7 @@ async fn execute_bash_injects_extra_env() {
         None,
         None,
         Some(&env),
+        &[],
     )
     .await;
     assert_eq!(code, 0);
@@ -912,11 +923,8 @@ async fn shell_executor_set_skill_env_injects_vars() {
 
     let config = ShellConfig {
         timeout: 5,
-        allowed_commands: vec![],
-        blocked_commands: vec![],
-        allowed_paths: vec![],
-        confirm_patterns: vec![],
         allow_network: false,
+        ..default_config()
     };
 
     let executor = ShellExecutor::new(&config);
@@ -935,7 +943,7 @@ async fn shell_executor_set_skill_env_injects_vars() {
 #[cfg(unix)]
 #[tokio::test]
 async fn execute_bash_error_handling() {
-    let (result, code) = execute_bash("false", Duration::from_secs(5), None, None, None).await;
+    let (result, code) = execute_bash("false", Duration::from_secs(5), None, None, None, &[]).await;
     assert_eq!(result, "(no output)");
     assert_eq!(code, 1);
 }
@@ -949,6 +957,7 @@ async fn execute_bash_command_not_found() {
         None,
         None,
         None,
+        &[],
     )
     .await;
     assert!(result.contains("[stderr]") || result.contains("[error]"));
@@ -1056,6 +1065,7 @@ async fn cancel_token_kills_child_process() {
         None,
         Some(&token),
         None,
+        &[],
     )
     .await;
     assert_eq!(code, 130);
@@ -1065,7 +1075,8 @@ async fn cancel_token_kills_child_process() {
 #[tokio::test]
 #[cfg(not(target_os = "windows"))]
 async fn cancel_token_none_does_not_cancel() {
-    let (result, code) = execute_bash("echo ok", Duration::from_secs(5), None, None, None).await;
+    let (result, code) =
+        execute_bash("echo ok", Duration::from_secs(5), None, None, None, &[]).await;
     assert_eq!(code, 0);
     assert!(result.contains("ok"));
 }
@@ -1082,8 +1093,15 @@ async fn cancel_kills_child_process_group() {
         tokio::time::sleep(Duration::from_millis(200)).await;
         token_clone.cancel();
     });
-    let (result, code) =
-        execute_bash(&script, Duration::from_secs(30), None, Some(&token), None).await;
+    let (result, code) = execute_bash(
+        &script,
+        Duration::from_secs(30),
+        None,
+        Some(&token),
+        None,
+        &[],
+    )
+    .await;
     assert_eq!(code, 130);
     assert!(result.contains("[cancelled]"));
     // Wait briefly, then verify the subprocess did NOT create the marker file
@@ -1550,3 +1568,128 @@ fn classify_exit_0_returns_none() {
 fn classify_exit_1_generic_returns_none() {
     assert_eq!(classify_shell_exit(1, "some other error"), None);
 }
+
+// --- env_blocklist / scrubbing tests ---
+
+#[cfg(unix)]
+#[allow(unsafe_code)]
+#[tokio::test]
+async fn env_blocklist_strips_sensitive_vars() {
+    // Set a fake sensitive env var in the current process
+    unsafe { std::env::set_var("ZEPH_SECRET_TEST_VAR", "should-not-leak") };
+    let blocklist = vec!["ZEPH_".to_owned()];
+    let (result, code) = execute_bash(
+        "echo ${ZEPH_SECRET_TEST_VAR:-absent}",
+        Duration::from_secs(5),
+        None,
+        None,
+        None,
+        &blocklist,
+    )
+    .await;
+    unsafe { std::env::remove_var("ZEPH_SECRET_TEST_VAR") };
+    assert_eq!(code, 0);
+    assert!(
+        result.contains("absent"),
+        "ZEPH_ var should have been stripped, got: {result}"
+    );
+}
+
+#[cfg(unix)]
+#[tokio::test]
+async fn env_blocklist_preserves_safe_vars() {
+    let blocklist = vec!["ZEPH_".to_owned()];
+    // PATH and HOME are always set in the test environment; verify they are inherited.
+    let (result, code) = execute_bash(
+        "echo ${PATH:+present}",
+        Duration::from_secs(5),
+        None,
+        None,
+        None,
+        &blocklist,
+    )
+    .await;
+    assert_eq!(code, 0);
+    assert!(
+        result.contains("present"),
+        "PATH should be preserved, got: {result}"
+    );
+}
+
+#[cfg(unix)]
+#[tokio::test]
+async fn env_blocklist_extra_env_still_injected() {
+    // Even with a blocklist active, skill-provided extra_env vars must be passed through.
+    let blocklist = vec!["ZEPH_".to_owned()];
+    let mut extra = std::collections::HashMap::new();
+    extra.insert("SKILL_TEST_VAR".to_owned(), "skill-value".to_owned());
+    let (result, code) = execute_bash(
+        "echo $SKILL_TEST_VAR",
+        Duration::from_secs(5),
+        None,
+        None,
+        Some(&extra),
+        &blocklist,
+    )
+    .await;
+    assert_eq!(code, 0);
+    assert!(
+        result.contains("skill-value"),
+        "skill extra_env should be injected, got: {result}"
+    );
+}
+
+#[cfg(unix)]
+#[allow(unsafe_code)]
+#[tokio::test]
+async fn env_blocklist_multiple_prefixes() {
+    unsafe {
+        std::env::set_var("AWS_SECRET_ACCESS_KEY", "aws-secret");
+        std::env::set_var("OPENAI_API_KEY", "openai-secret");
+    }
+    let blocklist = vec!["AWS_".to_owned(), "OPENAI_".to_owned()];
+    let (result, code) = execute_bash(
+        "echo ${AWS_SECRET_ACCESS_KEY:-absent1} ${OPENAI_API_KEY:-absent2}",
+        Duration::from_secs(5),
+        None,
+        None,
+        None,
+        &blocklist,
+    )
+    .await;
+    unsafe {
+        std::env::remove_var("AWS_SECRET_ACCESS_KEY");
+        std::env::remove_var("OPENAI_API_KEY");
+    }
+    assert_eq!(code, 0);
+    assert!(
+        result.contains("absent1"),
+        "AWS_ var should be stripped, got: {result}"
+    );
+    assert!(
+        result.contains("absent2"),
+        "OPENAI_ var should be stripped, got: {result}"
+    );
+}
+
+#[cfg(unix)]
+#[allow(unsafe_code)]
+#[tokio::test]
+async fn empty_env_blocklist_passes_all_vars() {
+    unsafe { std::env::set_var("ZEPH_EMPTY_BLOCKLIST_TEST", "visible") };
+    let (result, code) = execute_bash(
+        "echo ${ZEPH_EMPTY_BLOCKLIST_TEST:-absent}",
+        Duration::from_secs(5),
+        None,
+        None,
+        None,
+        &[],
+    )
+    .await;
+    unsafe { std::env::remove_var("ZEPH_EMPTY_BLOCKLIST_TEST") };
+    assert_eq!(code, 0);
+    assert!(
+        result.contains("visible"),
+        "empty blocklist should pass all vars, got: {result}"
+    );
+}