diff --git a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs index d4b418d93a..553d4ff7a1 100644 --- a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -41,6 +41,13 @@ fn is_dangerous_powershell(command: &[String]) -> bool { .collect(); let has_url = args_have_url(&parsed.tokens); + // Keep parity with Unix-style `rm -f` checks: in PowerShell, `Remove-Item` (and + // common aliases like `rm`) becomes meaningfully more dangerous when `-Force` + // (or `-f`) is present. + if is_powershell_force_delete(&tokens_lc) { + return true; + } + if has_url && tokens_lc.iter().any(|t| { matches!( @@ -85,6 +92,39 @@ fn is_dangerous_powershell(command: &[String]) -> bool { false } +fn is_powershell_force_delete(tokens_lc: &[String]) -> bool { + let Some(first) = tokens_lc.first() else { + return false; + }; + + // `rm`, `ri`, `del`, `erase` are common aliases for `Remove-Item`. + if !matches!( + first.as_str(), + "remove-item" | "rm" | "ri" | "del" | "erase" + ) { + return false; + } + + tokens_lc.iter().any(|t| { + // Common truthy forms. + if matches!(t.as_str(), "-force" | "-f" | "-rf" | "-fr") { + return true; + } + + // Only treat explicit truthy assignments as force. Avoid flagging `-Force:$false`. + let value = if let Some(v) = t.strip_prefix("-force:") { + v + } else if let Some(v) = t.strip_prefix("-f:") { + v + } else { + return false; + }; + + let value = value.trim(); + value.eq_ignore_ascii_case("true") || value.eq_ignore_ascii_case("$true") || value == "1" + }) +} + fn is_dangerous_cmd(command: &[String]) -> bool { let Some((exe, rest)) = command.split_first() else { return false; @@ -110,14 +150,52 @@ fn is_dangerous_cmd(command: &[String]) -> bool { let Some(first_cmd) = iter.next() else { return false; }; + let remaining: Vec = iter.cloned().collect(); + + if is_cmd_force_delete(first_cmd, &remaining) { + return true; + } + // Classic `cmd /c start https://...` ShellExecute path. if !first_cmd.eq_ignore_ascii_case("start") { return false; } - let remaining: Vec = iter.cloned().collect(); + args_have_url(&remaining) } +fn is_cmd_force_delete(first_cmd_lc: &str, cmd_args: &[String]) -> bool { + // Helper: detect cmd-style switches, including grouped forms like `/q/f`. + let has_switch = |needle: &str| { + let needle = needle.to_ascii_lowercase(); + cmd_args.iter().any(|a| { + let lower = a.to_ascii_lowercase(); + if !(lower.starts_with('/') || lower.starts_with('-')) { + return false; + } + + // Strip the prefix and split on additional `/` in grouped forms. + let rest = lower.trim_start_matches(['/', '-']); + rest.split('/').any(|seg| { + let seg = seg.trim(); + seg == needle || seg.starts_with(&format!("{needle}:")) + }) + }) + }; + + // File deletes: `del/erase /f` forces deletion of read-only files. + if matches!(first_cmd_lc, "del" | "erase") { + return has_switch("f"); + } + + // Directory deletes: `rd/rmdir /s` removes directory trees. + if matches!(first_cmd_lc, "rd" | "rmdir") { + return has_switch("s"); + } + + false +} + fn is_direct_gui_launch(command: &[String]) -> bool { let Some((exe, rest)) = command.split_first() else { return false; @@ -288,6 +366,42 @@ mod tests { ]))); } + #[test] + fn powershell_remove_item_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item -Force foo.txt" + ]))); + } + + #[test] + fn powershell_rm_f_alias_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "rm -f foo.txt" + ]))); + } + + #[test] + fn powershell_remove_item_without_force_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item foo.txt" + ]))); + } + + #[test] + fn powershell_remove_item_force_false_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item -Force:$false foo.txt" + ]))); + } + #[test] fn cmd_start_with_url_is_dangerous() { assert!(is_dangerous_command_windows(&vec_str(&[ @@ -298,6 +412,20 @@ mod tests { ]))); } + #[test] + fn cmd_del_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "del", "/f", "foo.txt" + ]))); + } + + #[test] + fn cmd_del_without_force_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "del", "foo.txt" + ]))); + } + #[test] fn msedge_with_url_is_dangerous() { assert!(is_dangerous_command_windows(&vec_str(&[