Merge pull request #2796 from zeroclaw-labs/issue-2779-shell-redirect-policy-dev

fix(shell): add configurable redirect policy and strip mode

src/config/schema.rs

@@ -1,6 +1,6 @@
 use crate::config::traits::ChannelConfig;
 use crate::providers::{is_glm_alias, is_zai_alias};
-use crate::security::{AutonomyLevel, DomainMatcher};
+use crate::security::{AutonomyLevel, DomainMatcher, ShellRedirectPolicy};
 use anyhow::{Context, Result};
 use directories::UserDirs;
 use schemars::JsonSchema;
@@ -2297,6 +2297,13 @@ pub struct AutonomyConfig {
     #[serde(default = "default_true")]
     pub block_high_risk_commands: bool,
 
+    /// Redirect handling mode for shell commands.
+    ///
+    /// - `block` (default): reject unquoted redirects.
+    /// - `strip`: normalize common stderr/null redirects before execution.
+    #[serde(default)]
+    pub shell_redirect_policy: ShellRedirectPolicy,
+
     /// Additional environment variables allowed for shell tool subprocesses.
     ///
     /// These names are explicitly allowlisted and merged with the built-in safe
@@ -2451,6 +2458,7 @@ impl Default for AutonomyConfig {
             max_cost_per_day_cents: 500,
             require_approval_for_medium_risk: true,
             block_high_risk_commands: true,
+            shell_redirect_policy: ShellRedirectPolicy::Block,
             shell_env_passthrough: vec![],
             auto_approve: default_auto_approve(),
             always_ask: default_always_ask(),
@@ -6776,6 +6784,7 @@ mod tests {
         assert_eq!(a.max_cost_per_day_cents, 500);
         assert!(a.require_approval_for_medium_risk);
         assert!(a.block_high_risk_commands);
+        assert_eq!(a.shell_redirect_policy, ShellRedirectPolicy::Block);
         assert!(a.shell_env_passthrough.is_empty());
         assert!(a.non_cli_excluded_tools.contains(&"shell".to_string()));
         assert!(a.non_cli_excluded_tools.contains(&"delegate".to_string()));
@@ -6798,6 +6807,7 @@ always_ask = []
 allowed_roots = []
 "#;
         let parsed: AutonomyConfig = toml::from_str(raw).unwrap();
+        assert_eq!(parsed.shell_redirect_policy, ShellRedirectPolicy::Block);
         assert!(parsed.non_cli_excluded_tools.contains(&"shell".to_string()));
         assert!(parsed
             .non_cli_excluded_tools
@@ -6963,6 +6973,7 @@ default_temperature = 0.7
                 max_cost_per_day_cents: 1000,
                 require_approval_for_medium_risk: false,
                 block_high_risk_commands: true,
+                shell_redirect_policy: ShellRedirectPolicy::Strip,
                 shell_env_passthrough: vec!["DATABASE_URL".into()],
                 auto_approve: vec!["file_read".into()],
                 always_ask: vec![],

src/cron/scheduler.rs

@@ -414,6 +414,8 @@ async fn run_job_command_with_timeout(
     job: &CronJob,
     timeout: Duration,
 ) -> (bool, String) {
+    let effective_command = security.apply_shell_redirect_policy(&job.command);
+
     if !security.can_act() {
         return (
             false,
@@ -428,7 +430,7 @@ async fn run_job_command_with_timeout(
         );
     }
 
-    if !security.is_command_allowed(&job.command) {
+    if !security.is_command_allowed(&effective_command) {
         return (
             false,
             format!(
@@ -438,7 +440,7 @@ async fn run_job_command_with_timeout(
         );
     }
 
-    if let Some(path) = security.forbidden_path_argument(&job.command) {
+    if let Some(path) = security.forbidden_path_argument(&effective_command) {
         return (
             false,
             format!("blocked by security policy: forbidden path argument: {path}"),
@@ -454,7 +456,7 @@ async fn run_job_command_with_timeout(
 
     let child = match Command::new("sh")
         .arg("-lc")
-        .arg(&job.command)
+        .arg(&effective_command)
         .current_dir(&config.workspace_dir)
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
@@ -491,7 +493,7 @@ mod tests {
     use super::*;
     use crate::config::Config;
     use crate::cron::{self, DeliveryConfig};
-    use crate::security::SecurityPolicy;
+    use crate::security::{SecurityPolicy, ShellRedirectPolicy};
     use chrono::{Duration as ChronoDuration, Utc};
     use std::sync::OnceLock;
     use tempfile::TempDir;
@@ -694,6 +696,20 @@ mod tests {
         assert!(output.contains("command not allowed"));
     }
 
+    #[tokio::test]
+    async fn run_job_command_strip_policy_normalizes_common_stderr_redirects() {
+        let tmp = TempDir::new().unwrap();
+        let mut config = test_config(&tmp).await;
+        config.autonomy.allowed_commands = vec!["echo".into()];
+        config.autonomy.shell_redirect_policy = ShellRedirectPolicy::Strip;
+        let job = test_job("echo scheduler-strip 2>&1");
+        let security = SecurityPolicy::from_config(&config.autonomy, &config.workspace_dir);
+
+        let (success, output) = run_job_command(&config, &security, &job).await;
+        assert!(success);
+        assert!(output.contains("scheduler-strip"));
+    }
+
     #[tokio::test]
     async fn run_job_command_blocks_readonly_mode() {
         let tmp = TempDir::new().unwrap();

src/security/mod.rs

@@ -54,7 +54,7 @@ pub use estop::{EstopLevel, EstopManager, EstopState, ResumeSelector};
 pub use otp::OtpValidator;
 #[allow(unused_imports)]
 pub use pairing::PairingGuard;
-pub use policy::{AutonomyLevel, SecurityPolicy};
+pub use policy::{AutonomyLevel, SecurityPolicy, ShellRedirectPolicy};
 #[allow(unused_imports)]
 pub use secrets::SecretStore;
 #[allow(unused_imports)]

src/security/policy.rs

@@ -32,6 +32,25 @@ impl std::str::FromStr for AutonomyLevel {
     }
 }
 
+/// Policy for handling shell redirect operators.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
+#[serde(rename_all = "snake_case")]
+pub enum ShellRedirectPolicy {
+    /// Block redirect operators (`<`, `>`, `>>`, etc.) in unquoted shell input.
+    #[default]
+    Block,
+    /// Strip common LLM-generated stderr/null redirects before validation/execution.
+    ///
+    /// Supported normalization:
+    /// - `2>&1`, `1>&2`, `>&1`
+    /// - `2>/dev/null`, `2>>/dev/null`, `>/dev/null`, `>>/dev/null`
+    /// - `&>/dev/null`, `>&/dev/null`
+    /// - `|&` -> `|`
+    ///
+    /// Other redirect forms remain blocked by command policy.
+    Strip,
+}
+
 /// Risk score for shell command execution.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum CommandRiskLevel {
@@ -105,6 +124,7 @@ pub struct SecurityPolicy {
     pub max_cost_per_day_cents: u32,
     pub require_approval_for_medium_risk: bool,
     pub block_high_risk_commands: bool,
+    pub shell_redirect_policy: ShellRedirectPolicy,
     pub shell_env_passthrough: Vec<String>,
     pub tracker: ActionTracker,
 }
@@ -157,6 +177,7 @@ impl Default for SecurityPolicy {
             max_cost_per_day_cents: 500,
             require_approval_for_medium_risk: true,
             block_high_risk_commands: true,
+            shell_redirect_policy: ShellRedirectPolicy::Block,
             shell_env_passthrough: vec![],
             tracker: ActionTracker::new(),
         }
@@ -419,6 +440,179 @@ fn contains_unquoted_char(command: &str, target: char) -> bool {
     false
 }
 
+fn is_token_boundary_char(ch: char) -> bool {
+    ch.is_whitespace() || matches!(ch, ';' | '\n' | '|' | '&' | ')' | '(')
+}
+
+fn starts_with_literal(chars: &[char], start: usize, literal: &str) -> bool {
+    let literal_chars: Vec<char> = literal.chars().collect();
+    chars
+        .get(start..start + literal_chars.len())
+        .is_some_and(|slice| slice == literal_chars)
+}
+
+fn consume_stream_merge_redirect(chars: &[char], start: usize) -> Option<usize> {
+    // Matches:
+    // - 2>&1
+    // - 1>&2
+    // - >&1
+    let mut i = start;
+    while i < chars.len() && chars[i].is_ascii_digit() {
+        i += 1;
+    }
+    if i >= chars.len() || chars[i] != '>' {
+        return None;
+    }
+    i += 1;
+    while i < chars.len() && chars[i].is_whitespace() {
+        i += 1;
+    }
+    if i >= chars.len() || chars[i] != '&' {
+        return None;
+    }
+    i += 1;
+    while i < chars.len() && chars[i].is_whitespace() {
+        i += 1;
+    }
+    let fd_start = i;
+    while i < chars.len() && chars[i].is_ascii_digit() {
+        i += 1;
+    }
+    if i == fd_start {
+        return None;
+    }
+    Some(i - start)
+}
+
+fn consume_dev_null_redirect(chars: &[char], start: usize) -> Option<usize> {
+    // Matches:
+    // - [fd]>/dev/null
+    // - [fd]>>/dev/null
+    // - [fd]< /dev/null
+    // - &>/dev/null
+    // - >&/dev/null
+    let mut i = start;
+    if chars[i] == '&' {
+        i += 1;
+        if i >= chars.len() || chars[i] != '>' {
+            return None;
+        }
+        i += 1;
+    } else {
+        while i < chars.len() && chars[i].is_ascii_digit() {
+            i += 1;
+        }
+        if i >= chars.len() || !matches!(chars[i], '>' | '<') {
+            return None;
+        }
+        let op = chars[i];
+        i += 1;
+        if op == '>' && i < chars.len() && chars[i] == '>' {
+            i += 1;
+        }
+        if op == '>' && i < chars.len() && chars[i] == '&' {
+            i += 1;
+        }
+    }
+
+    while i < chars.len() && chars[i].is_whitespace() {
+        i += 1;
+    }
+    if !starts_with_literal(chars, i, "/dev/null") {
+        return None;
+    }
+    i += "/dev/null".chars().count();
+    if i < chars.len() && !is_token_boundary_char(chars[i]) {
+        return None;
+    }
+    Some(i - start)
+}
+
+fn strip_supported_redirects(command: &str) -> String {
+    let chars: Vec<char> = command.chars().collect();
+    let mut out = String::with_capacity(command.len());
+    let mut quote = QuoteState::None;
+    let mut escaped = false;
+    let mut i = 0usize;
+
+    while i < chars.len() {
+        let ch = chars[i];
+        match quote {
+            QuoteState::Single => {
+                if ch == '\'' {
+                    quote = QuoteState::None;
+                }
+                out.push(ch);
+                i += 1;
+            }
+            QuoteState::Double => {
+                if escaped {
+                    escaped = false;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+                if ch == '\\' {
+                    escaped = true;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+                if ch == '"' {
+                    quote = QuoteState::None;
+                }
+                out.push(ch);
+                i += 1;
+            }
+            QuoteState::None => {
+                if escaped {
+                    escaped = false;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+                if ch == '\\' {
+                    escaped = true;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+                if ch == '\'' {
+                    quote = QuoteState::Single;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+                if ch == '"' {
+                    quote = QuoteState::Double;
+                    out.push(ch);
+                    i += 1;
+                    continue;
+                }
+
+                // Normalize `|&` to `|` since shell tool already captures stderr.
+                if ch == '|' && chars.get(i + 1).is_some_and(|next| *next == '&') {
+                    out.push('|');
+                    i += 2;
+                    continue;
+                }
+
+                if let Some(consumed) = consume_stream_merge_redirect(&chars, i)
+                    .or_else(|| consume_dev_null_redirect(&chars, i))
+                {
+                    i += consumed;
+                    continue;
+                }
+
+                out.push(ch);
+                i += 1;
+            }
+        }
+    }
+
+    out
+}
+
 /// Detect unquoted shell variable expansions like `$HOME`, `$1`, `$?`.
 ///
 /// Escaped dollars (`\$`) are ignored. Variables inside single quotes are
@@ -561,6 +755,14 @@ fn is_allowlist_entry_match(allowed: &str, executable: &str, executable_base: &s
 }
 
 impl SecurityPolicy {
+    /// Apply configured redirect policy to a shell command before validation/execution.
+    pub fn apply_shell_redirect_policy(&self, command: &str) -> String {
+        match self.shell_redirect_policy {
+            ShellRedirectPolicy::Block => command.to_string(),
+            ShellRedirectPolicy::Strip => strip_supported_redirects(command),
+        }
+    }
+
     // ── Risk Classification ──────────────────────────────────────────────
     // Risk is assessed per-segment (split on shell operators), and the
     // highest risk across all segments wins. This prevents bypasses like
@@ -688,15 +890,17 @@ impl SecurityPolicy {
         command: &str,
         approved: bool,
     ) -> Result<CommandRiskLevel, String> {
-        if !self.is_command_allowed(command) {
+        let effective_command = self.apply_shell_redirect_policy(command);
+
+        if !self.is_command_allowed(&effective_command) {
             return Err(format!("Command not allowed by security policy: {command}"));
         }
 
-        if let Some(path) = self.forbidden_path_argument(command) {
+        if let Some(path) = self.forbidden_path_argument(&effective_command) {
             return Err(format!("Path blocked by security policy: {path}"));
         }
 
-        let risk = self.command_risk_level(command);
+        let risk = self.command_risk_level(&effective_command);
 
         if risk == CommandRiskLevel::High {
             if self.block_high_risk_commands {
@@ -1088,6 +1292,7 @@ impl SecurityPolicy {
             max_cost_per_day_cents: autonomy_config.max_cost_per_day_cents,
             require_approval_for_medium_risk: autonomy_config.require_approval_for_medium_risk,
             block_high_risk_commands: autonomy_config.block_high_risk_commands,
+            shell_redirect_policy: autonomy_config.shell_redirect_policy,
             shell_env_passthrough: autonomy_config.shell_env_passthrough.clone(),
             tracker: ActionTracker::new(),
         }
@@ -1451,6 +1656,7 @@ mod tests {
             max_cost_per_day_cents: 1000,
             require_approval_for_medium_risk: false,
             block_high_risk_commands: false,
+            shell_redirect_policy: ShellRedirectPolicy::Strip,
             shell_env_passthrough: vec!["DATABASE_URL".into()],
             ..crate::config::AutonomyConfig::default()
         };
@@ -1465,6 +1671,7 @@ mod tests {
         assert_eq!(policy.max_cost_per_day_cents, 1000);
         assert!(!policy.require_approval_for_medium_risk);
         assert!(!policy.block_high_risk_commands);
+        assert_eq!(policy.shell_redirect_policy, ShellRedirectPolicy::Strip);
         assert_eq!(policy.shell_env_passthrough, vec!["DATABASE_URL"]);
         assert_eq!(policy.workspace_dir, PathBuf::from("/tmp/test-workspace"));
     }
@@ -1509,6 +1716,7 @@ mod tests {
         assert!(p.max_cost_per_day_cents > 0);
         assert!(p.require_approval_for_medium_risk);
         assert!(p.block_high_risk_commands);
+        assert_eq!(p.shell_redirect_policy, ShellRedirectPolicy::Block);
         assert!(p.shell_env_passthrough.is_empty());
     }
 
@@ -1701,6 +1909,60 @@ mod tests {
         assert!(!p.is_command_allowed("cat</etc/passwd"));
     }
 
+    #[test]
+    fn strip_policy_normalizes_common_redirect_patterns() {
+        let p = SecurityPolicy {
+            shell_redirect_policy: ShellRedirectPolicy::Strip,
+            ..default_policy()
+        };
+
+        let merged = p.apply_shell_redirect_policy("echo hello 2>&1");
+        assert!(!merged.contains("2>&1"));
+        assert!(merged.contains("echo hello"));
+
+        let devnull = p.apply_shell_redirect_policy("echo hello 2>/dev/null");
+        assert!(!devnull.contains("/dev/null"));
+        assert!(devnull.contains("echo hello"));
+
+        let pipeline = p.apply_shell_redirect_policy("echo hello |& cat");
+        assert!(!pipeline.contains("|&"));
+        assert!(pipeline.contains("| cat"));
+
+        let quoted = p.apply_shell_redirect_policy("echo '2>&1' \"|&\" '2>/dev/null'");
+        assert_eq!(quoted, "echo '2>&1' \"|&\" '2>/dev/null'");
+    }
+
+    #[test]
+    fn strip_policy_allows_normalized_stderr_redirects() {
+        let p = SecurityPolicy {
+            shell_redirect_policy: ShellRedirectPolicy::Strip,
+            allowed_commands: vec!["echo".into()],
+            ..default_policy()
+        };
+
+        assert!(p
+            .validate_command_execution("echo hello 2>&1", false)
+            .is_ok());
+        assert!(p
+            .validate_command_execution("echo hello 2>/dev/null", false)
+            .is_ok());
+    }
+
+    #[test]
+    fn strip_policy_keeps_unsupported_redirects_blocked() {
+        let p = SecurityPolicy {
+            shell_redirect_policy: ShellRedirectPolicy::Strip,
+            ..default_policy()
+        };
+
+        assert!(p
+            .validate_command_execution("echo hello > out.txt", false)
+            .is_err());
+        assert!(p
+            .validate_command_execution("cat </etc/passwd", false)
+            .is_err());
+    }
+
     #[test]
     fn quoted_ampersand_and_redirect_literals_are_not_treated_as_operators() {
         let p = default_policy();

src/tools/process.rs

@@ -80,6 +80,7 @@ impl ProcessTool {
             .get("command")
             .and_then(|v| v.as_str())
             .ok_or_else(|| anyhow::anyhow!("Missing 'command' parameter for spawn action"))?;
+        let effective_command = self.security.apply_shell_redirect_policy(command);
 
         // Check concurrent running process count.
         {
@@ -126,7 +127,7 @@ impl ProcessTool {
             });
         }
 
-        if let Some(path) = self.security.forbidden_path_argument(command) {
+        if let Some(path) = self.security.forbidden_path_argument(&effective_command) {
             return Ok(ToolResult {
                 success: false,
                 output: String::new(),
@@ -145,7 +146,7 @@ impl ProcessTool {
         // Build command via runtime adapter.
         let mut cmd = match self
             .runtime
-            .build_shell_command(command, &self.security.workspace_dir)
+            .build_shell_command(&effective_command, &self.security.workspace_dir)
         {
             Ok(cmd) => cmd,
             Err(e) => {

src/tools/shell.rs

@@ -138,6 +138,7 @@ impl Tool for ShellTool {
     async fn execute(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> {
         let command = extract_command_argument(&args)
             .ok_or_else(|| anyhow::anyhow!("Missing 'command' parameter"))?;
+        let effective_command = self.security.apply_shell_redirect_policy(&command);
         let approved = args
             .get("approved")
             .and_then(|v| v.as_bool())
@@ -162,7 +163,7 @@ impl Tool for ShellTool {
             }
         }
 
-        if let Some(path) = self.security.forbidden_path_argument(&command) {
+        if let Some(path) = self.security.forbidden_path_argument(&effective_command) {
             return Ok(ToolResult {
                 success: false,
                 output: String::new(),
@@ -183,7 +184,7 @@ impl Tool for ShellTool {
         // (CWE-200), then re-add only safe, functional variables.
         let mut cmd = match self
             .runtime
-            .build_shell_command(&command, &self.security.workspace_dir)
+            .build_shell_command(&effective_command, &self.security.workspace_dir)
         {
             Ok(cmd) => cmd,
             Err(e) => {
@@ -228,7 +229,7 @@ impl Tool for ShellTool {
 
                 if let Some(detector) = &self.syscall_detector {
                     let _ = detector.inspect_command_output(
-                        &command,
+                        &effective_command,
                         &stdout,
                         &stderr,
                         output.status.code(),
@@ -266,7 +267,9 @@ mod tests {
     use super::*;
     use crate::config::{AuditConfig, SyscallAnomalyConfig};
     use crate::runtime::{NativeRuntime, RuntimeAdapter};
-    use crate::security::{AutonomyLevel, SecurityPolicy, SyscallAnomalyDetector};
+    use crate::security::{
+        AutonomyLevel, SecurityPolicy, ShellRedirectPolicy, SyscallAnomalyDetector,
+    };
     use tempfile::TempDir;
 
     fn test_security(autonomy: AutonomyLevel) -> Arc<SecurityPolicy> {
@@ -277,6 +280,18 @@ mod tests {
         })
     }
 
+    fn test_security_with_redirect_policy(
+        autonomy: AutonomyLevel,
+        shell_redirect_policy: ShellRedirectPolicy,
+    ) -> Arc<SecurityPolicy> {
+        Arc::new(SecurityPolicy {
+            autonomy,
+            workspace_dir: std::env::temp_dir(),
+            shell_redirect_policy,
+            ..SecurityPolicy::default()
+        })
+    }
+
     fn test_runtime() -> Arc<dyn RuntimeAdapter> {
         Arc::new(NativeRuntime::new())
     }
@@ -487,6 +502,56 @@ mod tests {
             .contains("not allowed"));
     }
 
+    #[tokio::test]
+    async fn shell_strip_policy_allows_common_stderr_redirects() {
+        let tool = ShellTool::new(
+            test_security_with_redirect_policy(
+                AutonomyLevel::Supervised,
+                ShellRedirectPolicy::Strip,
+            ),
+            test_runtime(),
+        );
+
+        let merged = tool
+            .execute(json!({"command": "echo redirect-ok 2>&1"}))
+            .await
+            .expect("2>&1 should be normalized under strip policy");
+        assert!(merged.success);
+        assert!(merged.output.contains("redirect-ok"));
+
+        let devnull = tool
+            .execute(json!({"command": "ls definitely_missing_shell_redirect 2>/dev/null"}))
+            .await
+            .expect("2>/dev/null should be normalized under strip policy");
+        assert!(!devnull.success);
+        assert!(devnull
+            .error
+            .as_deref()
+            .unwrap_or("")
+            .contains("definitely_missing_shell_redirect"));
+    }
+
+    #[tokio::test]
+    async fn shell_strip_policy_still_blocks_unsupported_redirects() {
+        let tool = ShellTool::new(
+            test_security_with_redirect_policy(
+                AutonomyLevel::Supervised,
+                ShellRedirectPolicy::Strip,
+            ),
+            test_runtime(),
+        );
+        let result = tool
+            .execute(json!({"command": "echo blocked > out.txt"}))
+            .await
+            .expect("unsupported redirect should still be blocked");
+        assert!(!result.success);
+        assert!(result
+            .error
+            .as_deref()
+            .unwrap_or("")
+            .contains("not allowed"));
+    }
+
     fn test_security_with_env_cmd() -> Arc<SecurityPolicy> {
         Arc::new(SecurityPolicy {
             autonomy: AutonomyLevel::Supervised,