From 137c4661328b313a24176d63e3a3f96f8c0c504d Mon Sep 17 00:00:00 2001 From: HDCode Date: Sun, 28 Dec 2025 20:31:04 +0100 Subject: [PATCH 1/4] fix: require approval for force delete on Windows --- .../windows_dangerous_commands.rs | 146 +++++++++++++++++- 1 file changed, 142 insertions(+), 4 deletions(-) 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 d4b418d93a1..49fa25a15a5 100644 --- a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -82,6 +82,11 @@ fn is_dangerous_powershell(command: &[String]) -> bool { } } + // Check for force delete operations (e.g., Remove-Item -Force) + if has_force_delete_cmdlet(&tokens_lc) { + return true; + } + false } @@ -111,11 +116,24 @@ fn is_dangerous_cmd(command: &[String]) -> bool { return false; }; // Classic `cmd /c start https://...` ShellExecute path. - if !first_cmd.eq_ignore_ascii_case("start") { - return false; + if first_cmd.eq_ignore_ascii_case("start") { + let remaining: Vec = iter.cloned().collect(); + return args_have_url(&remaining); + } + + // Force delete: del /f, erase /f + if first_cmd.eq_ignore_ascii_case("del") || first_cmd.eq_ignore_ascii_case("erase") { + let remaining: Vec = iter.cloned().collect(); + return has_force_flag_cmd(&remaining); } - let remaining: Vec = iter.cloned().collect(); - args_have_url(&remaining) + + // Recursive directory removal: rd /s /q, rmdir /s /q + if first_cmd.eq_ignore_ascii_case("rd") || first_cmd.eq_ignore_ascii_case("rmdir") { + let remaining: Vec = iter.cloned().collect(); + return has_recursive_flag_cmd(&remaining) && has_quiet_flag_cmd(&remaining); + } + + false } fn is_direct_gui_launch(command: &[String]) -> bool { @@ -149,6 +167,35 @@ fn is_direct_gui_launch(command: &[String]) -> bool { false } +/// Check for PowerShell force delete cmdlets like `Remove-Item -Force`. +fn has_force_delete_cmdlet(tokens: &[String]) -> bool { + // PowerShell cmdlets/aliases for deletion + const DELETE_CMDLETS: &[&str] = &["remove-item", "ri", "rm", "del", "erase", "rd", "rmdir"]; + let has_delete = tokens.iter().any(|t| DELETE_CMDLETS.contains(&t.as_str())); + let has_force = tokens + .iter() + .any(|t| t == "-force" || t.starts_with("-force:")); + has_delete && has_force +} + +/// Check for /f or /F flag in CMD del/erase arguments. +fn has_force_flag_cmd(args: &[String]) -> bool { + args.iter() + .any(|a| a.eq_ignore_ascii_case("/f") || a.to_ascii_lowercase().contains("/f")) +} + +/// Check for /s or /S flag in CMD rd/rmdir arguments. +fn has_recursive_flag_cmd(args: &[String]) -> bool { + args.iter() + .any(|a| a.eq_ignore_ascii_case("/s") || a.to_ascii_lowercase().contains("/s")) +} + +/// Check for /q or /Q flag in CMD rd/rmdir arguments. +fn has_quiet_flag_cmd(args: &[String]) -> bool { + args.iter() + .any(|a| a.eq_ignore_ascii_case("/q") || a.to_ascii_lowercase().contains("/q")) +} + fn args_have_url(args: &[String]) -> bool { args.iter().any(|arg| looks_like_url(arg)) } @@ -313,4 +360,95 @@ mod tests { "." ]))); } + + // Force delete tests for PowerShell + + #[test] + fn powershell_remove_item_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item test -Force" + ]))); + } + + #[test] + fn powershell_remove_item_recurse_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item test -Recurse -Force" + ]))); + } + + #[test] + fn powershell_ri_alias_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "pwsh", + "-Command", + "ri test -Force" + ]))); + } + + #[test] + fn powershell_remove_item_without_force_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item test" + ]))); + } + + // Force delete tests for CMD + #[test] + fn cmd_del_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "del", "/f", "test.txt" + ]))); + } + + #[test] + fn cmd_erase_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "erase", "/f", "test.txt" + ]))); + } + + #[test] + fn cmd_del_without_force_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "del", "test.txt" + ]))); + } + + #[test] + fn cmd_rd_recursive_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "rd", "/s", "/q", "test" + ]))); + } + + #[test] + fn cmd_rd_without_quiet_is_not_flagged() { + assert!(!is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "rd", "/s", "test" + ]))); + } + + #[test] + fn cmd_rmdir_recursive_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "cmd", "/c", "rmdir", "/s", "/q", "test" + ]))); + } + + // Test exact scenario from issue #8567 + #[test] + fn powershell_remove_item_path_recurse_force_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item -Path 'test' -Recurse -Force" + ]))); + } } From 5e249d6e8a7679a074aa878b8360f3d9d52eb75b Mon Sep 17 00:00:00 2001 From: HDCode Date: Sun, 28 Dec 2025 20:43:13 +0100 Subject: [PATCH 2/4] fix: handle trailing punctuation in PowerShell force check --- .../windows_dangerous_commands.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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 49fa25a15a5..07c3574f307 100644 --- a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -172,9 +172,11 @@ fn has_force_delete_cmdlet(tokens: &[String]) -> bool { // PowerShell cmdlets/aliases for deletion const DELETE_CMDLETS: &[&str] = &["remove-item", "ri", "rm", "del", "erase", "rd", "rmdir"]; let has_delete = tokens.iter().any(|t| DELETE_CMDLETS.contains(&t.as_str())); - let has_force = tokens - .iter() - .any(|t| t == "-force" || t.starts_with("-force:")); + let has_force = tokens.iter().any(|t| { + // Handle -Force, -Force:, -Force; (trailing punctuation) + let trimmed = t.trim_end_matches(|c| c == ';' || c == ')' || c == '}'); + trimmed == "-force" || trimmed.starts_with("-force:") + }); has_delete && has_force } @@ -451,4 +453,13 @@ mod tests { "Remove-Item -Path 'test' -Recurse -Force" ]))); } + + #[test] + fn powershell_remove_item_force_with_semicolon_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "Remove-Item test -Force; Write-Host done" + ]))); + } } From 8b7511d858b27eef39b5808918dfd8a2c776bc06 Mon Sep 17 00:00:00 2001 From: HDCode Date: Sun, 28 Dec 2025 21:02:45 +0100 Subject: [PATCH 3/4] fix: include ']' in trailing punctuation trim and update comment --- .../core/src/command_safety/windows_dangerous_commands.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 07c3574f307..ff9bfeb466b 100644 --- a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -174,7 +174,9 @@ fn has_force_delete_cmdlet(tokens: &[String]) -> bool { let has_delete = tokens.iter().any(|t| DELETE_CMDLETS.contains(&t.as_str())); let has_force = tokens.iter().any(|t| { // Handle -Force, -Force:, -Force; (trailing punctuation) - let trimmed = t.trim_end_matches(|c| c == ';' || c == ')' || c == '}'); + // We use trim_end_matches for simplicity/performance. If this becomes insufficient + // (e.g. need to handle complex surrounding punctuation), consider using a Regex. + let trimmed = t.trim_end_matches(|c| c == ';' || c == ')' || c == '}' || c == ']'); trimmed == "-force" || trimmed.starts_with("-force:") }); has_delete && has_force From cd4d8a159ff28f7920fecad3f8d9aa0ab842b846 Mon Sep 17 00:00:00 2001 From: HDCode Date: Sun, 28 Dec 2025 21:15:59 +0100 Subject: [PATCH 4/4] test: add cases for force delete inside blocks and brackets --- .../windows_dangerous_commands.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 ff9bfeb466b..a96a1a78847 100644 --- a/codex-rs/core/src/command_safety/windows_dangerous_commands.rs +++ b/codex-rs/core/src/command_safety/windows_dangerous_commands.rs @@ -464,4 +464,22 @@ mod tests { "Remove-Item test -Force; Write-Host done" ]))); } + + #[test] + fn powershell_remove_item_force_inside_block_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "if ($true) { Remove-Item test -Force}" + ]))); + } + + #[test] + fn powershell_remove_item_force_inside_brackets_is_dangerous() { + assert!(is_dangerous_command_windows(&vec_str(&[ + "powershell", + "-Command", + "[void]( Remove-Item test -Force)]" + ]))); + } }