Fix flaky command safety test by using robust regex matching

Author: evansenter

src/tools/bash/safety.rs

@@ -10,17 +10,17 @@ use std::sync::LazyLock;
 pub static BLOCKED_PATTERNS: LazyLock<Vec<Regex>> = LazyLock::new(|| {
     vec![
         // Destructive filesystem operations
-        Regex::new(r"rm\s+(-[rfRF]+\s+)*[/~](\s|$)").unwrap(),
-        Regex::new(r"rm\s+(-[rfRF]+\s+)*/\*").unwrap(),
-        Regex::new(r"rm\s+(-[rfRF]+\s+)*~").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)rm\s+(-[rfRF]+\s+)*[/~](\s|$)").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)rm\s+(-[rfRF]+\s+)*/\*").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)rm\s+(-[rfRF]+\s+)*~").unwrap(),
         // Disk/device operations
-        Regex::new(r"dd\s+.*if=").unwrap(),
-        Regex::new(r"mkfs").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)dd\s+.*if=").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)mkfs").unwrap(),
         Regex::new(r">\s*/dev/sd").unwrap(),
         Regex::new(r">\s*/dev/nvme").unwrap(),
         // Permission bombs
-        Regex::new(r"chmod\s+(-[rR]+\s+)*777\s+/").unwrap(),
-        Regex::new(r"chown\s+(-[rR]+\s+)*.*\s+/").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)chmod\s+(-[rR]+\s+)*777\s+/").unwrap(),
+        Regex::new(r"(?m)(^|[;&|]\s*)chown\s+(-[rR]+\s+)*.*\s+/").unwrap(),
         // Fork bomb
         Regex::new(r":\(\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;").unwrap(),
         // Dangerous redirects
@@ -34,24 +34,18 @@ pub static BLOCKED_PATTERNS: LazyLock<Vec<Regex>> = LazyLock::new(|| {
 });
 
 /// Commands that require extra caution (requires user confirmation).
-pub static CAUTION_PATTERNS: LazyLock<Vec<&str>> = LazyLock::new(|| {
+pub static CAUTION_PATTERNS: LazyLock<Vec<Regex>> = LazyLock::new(|| {
     vec![
-        "sudo",
-        "su ",
-        "rm ",
-        "mv ",
-        "chmod",
-        "chown",
-        "kill",
-        "pkill",
-        "killall",
-        "git push --force",
-        "git push -f",
-        "git reset --hard",
-        "cargo publish",
-        "npm publish",
-        "docker rm",
-        "docker rmi",
+        // Dangerous binaries (word boundary safe)
+        // Matches start of line or after command separator (; | &)
+        Regex::new(r"(?m)(^|[;&|]\s*)(sudo|su|rm|mv|chmod|chown|kill|pkill|killall)(\s+|$)").unwrap(),
+        
+        // Dangerous subcommands
+        Regex::new(r"(?m)(^|[;&|]\s*)(docker\s+(rm|rmi)|cargo\s+publish|npm\s+publish)(\s+|$)").unwrap(),
+
+        // Dangerous flags
+        Regex::new(r"git\s+push\s+.*(-f|--force)").unwrap(),
+        Regex::new(r"git\s+reset\s+.*--hard").unwrap(),
     ]
 });
 
@@ -70,7 +64,7 @@ pub fn is_blocked(command: &str) -> Option<String> {
 pub fn needs_caution(command: &str) -> bool {
     CAUTION_PATTERNS
         .iter()
-        .any(|pattern| command.contains(pattern))
+        .any(|pattern| pattern.is_match(command))
 }
 
 #[cfg(test)]
@@ -92,6 +86,12 @@ mod tests {
         assert!(is_blocked("ls -l").is_none());
     }
 
+    #[test]
+    fn test_blocked_false_positives() {
+        assert!(is_blocked("echo harm -rf /").is_none(), "Should not block 'harm'");
+        assert!(is_blocked("echo farm -rf /*").is_none(), "Should not block 'farm'");
+    }
+
     #[test]
     fn test_caution_patterns() {
         assert!(needs_caution("sudo apt update"));
@@ -100,4 +100,21 @@ mod tests {
         assert!(needs_caution("git reset --hard HEAD"));
         assert!(!needs_caution("ls -l"));
     }
+
+    #[test]
+    fn test_caution_patterns_edge_cases() {
+        assert!(needs_caution("rm\tfile.txt"), "Should catch tab separator");
+        assert!(needs_caution("rm\nfile.txt"), "Should catch newline separator");
+        assert!(needs_caution("echo hi; rm file"), "Should catch rm after semicolon");
+        assert!(needs_caution("echo hi | rm file"), "Should catch rm after pipe");
+        assert!(needs_caution("rm"), "Should catch bare rm");
+    }
+
+    #[test]
+    fn test_caution_false_positives() {
+        assert!(!needs_caution("echo farm animal"), "Should not flag 'farm' as dangerous");
+        assert!(!needs_caution("echo supper"), "Should not flag 'supper' (contains 'su')");
+        assert!(!needs_caution("mkdir form"), "Should not flag 'form' (matches 'rm' substring)");
+        assert!(!needs_caution("echo remove"), "Should not flag 'remove'");
+    }
 }