diff --git a/src/security/policy.rs b/src/security/policy.rs index 180d9cc..3762fcf 100644 --- a/src/security/policy.rs +++ b/src/security/policy.rs @@ -184,6 +184,19 @@ fn contains_single_ampersand(s: &str) -> bool { false } +/// Strip safe stderr redirection patterns before policy checks. +/// +/// Removes `2>/dev/null`, `2> /dev/null`, and `2>&1` so they don't +/// trigger the generic `>` or `&` blockers. +fn strip_safe_stderr(s: &str) -> String { + let mut result = s.to_string(); + // Order matters: longest patterns first + for pat in ["2> /dev/null", "2>/dev/null", "2>&1"] { + result = result.replace(pat, ""); + } + result +} + impl SecurityPolicy { /// Classify command risk. Any high-risk segment marks the whole command high. pub fn command_risk_level(&self, command: &str) -> CommandRiskLevel { @@ -359,37 +372,46 @@ impl SecurityPolicy { /// - Blocks dangerous arguments (e.g. `find -exec`, `git config`) pub fn is_command_allowed(&self, command: &str) -> bool { if self.autonomy == AutonomyLevel::ReadOnly { + tracing::trace!(command, "Command blocked: read-only mode"); return false; } + // Strip safe stderr redirections (2>/dev/null, 2>&1) before + // operator checks so they don't trigger the generic `>` or `&` blockers. + let sanitized = strip_safe_stderr(command); + // Block subshell/expansion operators — these allow hiding arbitrary // commands inside an allowed command (e.g. `echo $(rm -rf /)`) - if command.contains('`') - || command.contains("$(") - || command.contains("${") - || command.contains("<(") - || command.contains(">(") + if sanitized.contains('`') + || sanitized.contains("$(") + || sanitized.contains("${") + || sanitized.contains("<(") + || sanitized.contains(">(") { + tracing::debug!(command, "Command blocked: subshell/expansion operator"); return false; } - // Block output redirections — they can write to arbitrary paths - if command.contains('>') { + // Block output redirections that write to arbitrary paths. + if sanitized.contains('>') { + tracing::debug!(command, "Command blocked: output redirection"); return false; } // Block `tee` — it can write to arbitrary files, bypassing the // redirect check above (e.g. `echo secret | tee /etc/crontab`) - if command + if sanitized .split_whitespace() .any(|w| w == "tee" || w.ends_with("/tee")) { + tracing::debug!(command, "Command blocked: tee can write arbitrary files"); return false; } // Block background command chaining (`&`), which can hide extra // sub-commands and outlive timeout expectations. Keep `&&` allowed. - if contains_single_ampersand(command) { + if contains_single_ampersand(&sanitized) { + tracing::debug!(command, "Command blocked: background & operator"); return false; } @@ -1107,6 +1129,22 @@ mod tests { let p = default_policy(); assert!(!p.is_command_allowed("echo secret > /etc/crontab")); assert!(!p.is_command_allowed("ls >> /tmp/exfil.txt")); + assert!(!p.is_command_allowed("cat file > /tmp/out")); + } + + #[test] + fn stderr_to_devnull_allowed() { + let p = SecurityPolicy { + allowed_commands: vec!["*".into()], + ..SecurityPolicy::default() + }; + assert!(p.is_command_allowed("ls -la /tmp/*.py 2>/dev/null")); + assert!(p.is_command_allowed("ls -la 2> /dev/null")); + assert!(p.is_command_allowed("grep pattern file 2>&1")); + assert!(p.is_command_allowed("cmd 2>/dev/null | grep foo")); + // Stdout redirect still blocked + assert!(!p.is_command_allowed("echo hello > /tmp/file")); + assert!(!p.is_command_allowed("echo hello 1> /tmp/file")); } #[test]