Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 129 additions & 1 deletion codex-rs/core/src/command_safety/windows_dangerous_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down Expand Up @@ -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;
Expand All @@ -110,14 +150,52 @@ fn is_dangerous_cmd(command: &[String]) -> bool {
let Some(first_cmd) = iter.next() else {
return false;
};
let remaining: Vec<String> = 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<String> = 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;
Expand Down Expand Up @@ -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(&[
Expand All @@ -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(&[
Expand Down
Loading