diff --git a/crates/tui/src/command_safety.rs b/crates/tui/src/command_safety.rs index 93a0a643..928682ec 100644 --- a/crates/tui/src/command_safety.rs +++ b/crates/tui/src/command_safety.rs @@ -590,6 +590,10 @@ pub fn analyze_command(command: &str) -> SafetyAnalysis { ); } + if let Some(analysis) = analyze_destructive_patterns(command) { + return analysis; + } + if command.contains("&&") || command.contains("||") || command.contains(';') { // Chains of known-safe commands (cargo/git/zig/npm/etc.) are // routine for build+test workflows. Instead of hard-blocking, @@ -622,7 +626,9 @@ pub fn analyze_command(command: &str) -> SafetyAnalysis { ); } - // Check for dangerous patterns first + // Check for dangerous patterns first. The token-aware pass above handles + // spacing and quoting variants; these literal patterns remain as a compact + // fallback for legacy shapes. for (pattern, reason) in DANGEROUS_PATTERNS { if command_lower.contains(&pattern.to_lowercase()) { return SafetyAnalysis::dangerous( @@ -717,6 +723,231 @@ pub fn analyze_command(command: &str) -> SafetyAnalysis { ) } +fn analyze_destructive_patterns(command: &str) -> Option { + if primary_shell_command_is(command, "eval") { + return Some(SafetyAnalysis::dangerous( + command, + vec!["Command invokes shell eval".to_string()], + vec!["Avoid evaluating dynamically generated shell input".to_string()], + )); + } + + if pipes_remote_content_to_shell(command) { + return Some(SafetyAnalysis::dangerous( + command, + vec!["Piping remote content directly to shell is dangerous".to_string()], + vec!["Download the script first and review it before execution".to_string()], + )); + } + + for segment in split_command_segments(command) { + let tokens = shell_words(&segment); + let Some(start) = primary_token_index(&tokens) else { + continue; + }; + match tokens[start].as_str() { + "rm" => { + if let Some(reason) = dangerous_rm_reason(&tokens[start + 1..]) { + return Some(SafetyAnalysis::dangerous( + command, + vec![reason], + vec!["Review the deletion target before retrying".to_string()], + )); + } + } + "find" => { + if let Some(analysis) = analyze_find_mutation(command, &tokens[start + 1..]) { + return Some(analysis); + } + } + _ => {} + } + } + + None +} + +fn split_command_segments(command: &str) -> Vec { + command + .replace("&&", "\n") + .replace("||", "\n") + .replace(';', "\n") + .split('\n') + .map(str::trim) + .filter(|segment| !segment.is_empty()) + .map(ToOwned::to_owned) + .collect() +} + +fn shell_words(segment: &str) -> Vec { + shlex::split(segment).unwrap_or_else(|| { + segment + .split_whitespace() + .map(|token| token.trim_matches(['"', '\'']).to_string()) + .collect() + }) +} + +fn primary_token_index(tokens: &[String]) -> Option { + let mut idx = 0; + while idx < tokens.len() { + let token = tokens[idx].as_str(); + if token == "env" { + idx += 1; + while idx < tokens.len() + && (tokens[idx].starts_with('-') || is_env_assignment(&tokens[idx])) + { + idx += 1; + } + continue; + } + if is_env_assignment(token) { + idx += 1; + continue; + } + return Some(idx); + } + None +} + +fn is_env_assignment(token: &str) -> bool { + let Some((name, _value)) = token.split_once('=') else { + return false; + }; + !name.is_empty() + && name + .chars() + .all(|ch| ch == '_' || ch.is_ascii_alphanumeric()) + && name + .chars() + .next() + .is_some_and(|ch| ch == '_' || ch.is_ascii_alphabetic()) +} + +fn primary_shell_command_is(command: &str, expected: &str) -> bool { + split_command_segments(command).into_iter().any(|segment| { + let tokens = shell_words(&segment); + primary_token_index(&tokens) + .and_then(|idx| tokens.get(idx)) + .is_some_and(|token| token == expected) + }) +} + +fn pipes_remote_content_to_shell(command: &str) -> bool { + split_command_segments(command).into_iter().any(|segment| { + let parts: Vec<&str> = segment.split('|').collect(); + if parts.len() < 2 { + return false; + } + parts.windows(2).any(|window| { + let left = window[0].to_ascii_lowercase(); + if !(left.contains("curl") || left.contains("wget")) { + return false; + } + let right_tokens = shell_words(window[1]); + primary_token_index(&right_tokens) + .and_then(|idx| right_tokens.get(idx)) + .is_some_and(|token| matches!(token.as_str(), "sh" | "bash" | "zsh")) + }) + }) +} + +fn dangerous_rm_reason(args: &[String]) -> Option { + let mut recursive = false; + let mut force = false; + let mut targets = Vec::new(); + + for arg in args { + match arg.as_str() { + "--" => continue, + "--recursive" | "--dir" => recursive = true, + "--force" => force = true, + flag if flag.starts_with('-') && !flag.starts_with("--") => { + recursive |= flag.chars().any(|ch| matches!(ch, 'r' | 'R')); + force |= flag.chars().any(|ch| ch == 'f'); + } + target => targets.push(target), + } + } + + if !(recursive || force) { + return None; + } + + for target in targets { + if is_root_delete_target(target) { + return Some("Recursive or forced deletion targets the root filesystem".to_string()); + } + if is_home_delete_target(target) { + return Some("Recursive or forced deletion targets the home directory".to_string()); + } + if target_contains_parent_escape(target) { + return Some("Recursive or forced deletion may escape the workspace".to_string()); + } + } + + None +} + +fn analyze_find_mutation(command: &str, args: &[String]) -> Option { + let has_delete = args.iter().any(|arg| arg == "-delete"); + let execs_rm = args + .windows(2) + .any(|pair| pair[0] == "-exec" && pair[1] == "rm"); + if !(has_delete || execs_rm) { + return None; + } + + let targets: Vec<&str> = args + .iter() + .take_while(|arg| !arg.starts_with('-')) + .map(String::as_str) + .collect(); + if targets.iter().any(|target| { + is_root_delete_target(target) + || is_home_delete_target(target) + || target_contains_parent_escape(target) + }) { + return Some(SafetyAnalysis::dangerous( + command, + vec!["find mutation targets a broad or external path".to_string()], + vec!["Restrict the find root to a workspace-relative path".to_string()], + )); + } + + Some(SafetyAnalysis::requires_approval( + command, + vec!["find command may delete files".to_string()], + )) +} + +fn is_root_delete_target(target: &str) -> bool { + let normalized = target.trim_matches(['"', '\'']).replace('\\', "/"); + normalized == "/" + || normalized == "/*" + || normalized == "//" + || normalized.starts_with("/*/") + || normalized.starts_with("/.") +} + +fn is_home_delete_target(target: &str) -> bool { + let normalized = target.trim_matches(['"', '\'']).replace('\\', "/"); + let lower = normalized.to_ascii_lowercase(); + lower == "~" + || lower.starts_with("~/") + || lower == "$home" + || lower.starts_with("$home/") + || lower == "${home}" + || lower.starts_with("${home}/") +} + +fn target_contains_parent_escape(target: &str) -> bool { + target + .replace('\\', "/") + .split('/') + .any(|component| component == "..") +} + /// Check if a command is known to be safe fn is_safe_command(command: &str) -> bool { let command_lower = command.to_lowercase(); @@ -955,6 +1186,52 @@ mod tests { ); } + #[test] + fn test_destructive_patterns_handle_spacing_and_quotes() { + assert_eq!(analyze_command("rm -rf /").level, SafetyLevel::Dangerous); + assert_eq!( + analyze_command("rm -rf \"/\"").level, + SafetyLevel::Dangerous + ); + assert_eq!(analyze_command("rm -fr -- /").level, SafetyLevel::Dangerous); + assert_eq!( + analyze_command("FOO=bar rm -rf $HOME").level, + SafetyLevel::Dangerous + ); + } + + #[test] + fn test_destructive_patterns_scan_chained_segments() { + assert_eq!( + analyze_command("echo ok; rm -rf /").level, + SafetyLevel::Dangerous + ); + } + + #[test] + fn test_find_delete_requires_approval_or_blocks_broad_roots() { + assert_eq!( + analyze_command("find / -delete").level, + SafetyLevel::Dangerous + ); + assert_eq!( + analyze_command("find . -delete").level, + SafetyLevel::RequiresApproval + ); + } + + #[test] + fn test_eval_invocation_is_blocked_without_substring_false_positive() { + assert_eq!( + analyze_command("eval $(echo test | base64 -d)").level, + SafetyLevel::Dangerous + ); + assert_ne!( + analyze_command("cargo run --bin deepseek -- eval").level, + SafetyLevel::Dangerous + ); + } + #[test] fn test_null_byte_is_blocked() { assert_eq!(