openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Strengthen forced rm command detection (#33464)

Browse Source 
## Why

Forced `rm` commands can appear inside complex shell syntax or use option and wrapper variants that the existing dangerous-command heuristic does not recognize.

## What changed

- Detect literal forced `rm` invocations in control flow, substitutions, pipelines, traps, nested shells, and `sudo` or `env` wrappers.
- Recognize combined, separate, and long-form force options, including when they follow operands.
- Require approval for these commands, or reject them with a safer-alternative explanation when approvals are disabled, even when the sandbox is disabled.
- Preserve existing rejection reasons for other dangerous commands and explicit policy rules.

## Testing

Add unit, policy, and end-to-end coverage for forced `rm` variants, complex shell scripts, approval requests, and rejection messages.

GitOrigin-RevId: e1fabc55324e788bae00ab24aae50d089eb7b058
This commit is contained in:
Dylan Hurd
2026-07-16 01:12:14 +00:00
committed by copyberry
parent c7f21a852c
commit be39aab9f4
5 changed files with 604 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/core/src/exec_policy.rs
+96 -25
View File
@@ -24,7 +24,8 @@ use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::permissions::FileSystemSandboxKind;
use codex_protocol::protocol::AskForApproval;
use codex_shell_command::is_dangerous_command::command_might_be_dangerous;
use codex_shell_command::is_dangerous_command::DangerousCommandMatch;
use codex_shell_command::is_dangerous_command::dangerous_command_match;
use codex_shell_command::is_safe_command::is_known_safe_command;
use thiserror::Error;
use tokio::fs;
@@ -325,16 +326,34 @@ impl ExecPolicyManager {
match evaluation.decision {
Decision::Forbidden => ExecApprovalRequirement::Forbidden {
reason: derive_forbidden_reason(command, &evaluation),
reason: derive_forbidden_reason(
command,
&evaluation,
dangerous_command_match_for_heuristics(
&evaluation,
Decision::Forbidden,
command_origin,
),
),
},
Decision::Prompt => {
let prompt_is_rule = evaluation.matched_rules.iter().any(|rule_match| {
is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt
});
match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) {
Some(reason) => ExecApprovalRequirement::Forbidden {
Some(reason) if prompt_is_rule => ExecApprovalRequirement::Forbidden {
reason: reason.to_string(),
},
Some(reason) => ExecApprovalRequirement::Forbidden {
reason: derive_rejected_prompt_reason(
reason,
dangerous_command_match_for_heuristics(
&evaluation,
Decision::Prompt,
command_origin,
),
),
},
None => ExecApprovalRequirement::NeedsApproval {
reason: derive_prompt_reason(command, &evaluation),
proposed_execpolicy_amendment: requested_amendment.or_else(|| {
@@ -630,11 +649,46 @@ pub async fn load_exec_policy(config_stack: &ConfigLayerStack) -> Result<Policy,
Ok(policy.merge_overlay(requirements_policy.as_ref()))
}
fn dangerous_command_match_for_origin(
command: &[String],
command_origin: ExecPolicyCommandOrigin,
) -> Option<DangerousCommandMatch> {
match command_origin {
ExecPolicyCommandOrigin::Generic => dangerous_command_match(command),
#[cfg(windows)]
ExecPolicyCommandOrigin::PowerShell => {
codex_shell_command::is_dangerous_command::dangerous_powershell_words_match(command)
}
}
}
/// Extract DangerousCommandMatch from an Evaluation
fn dangerous_command_match_for_heuristics(
evaluation: &Evaluation,
decision: Decision,
command_origin: ExecPolicyCommandOrigin,
) -> Option<DangerousCommandMatch> {
evaluation
.matched_rules
.iter()
.find_map(|rule_match| match rule_match {
RuleMatch::HeuristicsRuleMatch {
command,
decision: matched_decision,
} if *matched_decision == decision => {
dangerous_command_match_for_origin(command, command_origin)
}
_ => None,
})
}
/// If a command is not matched by any execpolicy rule, derive a [`Decision`].
pub(crate) fn render_decision_for_unmatched_command(
command: &[String],
context: UnmatchedCommandContext<'_>,
) -> Decision {
let dangerous_command_match =
dangerous_command_match_for_origin(command, context.command_origin);
let UnmatchedCommandContext {
approval_policy,
permission_profile,
@@ -674,27 +728,10 @@ pub(crate) fn render_decision_for_unmatched_command(
// We prefer to prompt the user rather than outright forbid the command,
// but if the user has explicitly disabled prompts, we must
// forbid the command.
let command_is_dangerous = match command_origin {
ExecPolicyCommandOrigin::Generic => command_might_be_dangerous(command),
#[cfg(windows)]
ExecPolicyCommandOrigin::PowerShell => {
codex_shell_command::is_dangerous_command::is_dangerous_powershell_words(command)
}
};
if command_is_dangerous || windows_managed_fs_restrictions_without_sandbox_backend {
if dangerous_command_match.is_some() || windows_managed_fs_restrictions_without_sandbox_backend
{
return match approval_policy {
AskForApproval::Never => {
let sandbox_is_explicitly_disabled = matches!(
permission_profile,
PermissionProfile::Disabled | PermissionProfile::External { .. }
);
if sandbox_is_explicitly_disabled {
// If the sandbox is explicitly disabled, we should allow the command to run
Decision::Allow
} else {
Decision::Forbidden
}
}
AskForApproval::Never => Decision::Forbidden,
AskForApproval::OnRequest
| AskForApproval::UnlessTrusted
| AskForApproval::Granular(_) => Decision::Prompt,
@@ -955,7 +992,11 @@ fn render_shlex_command(args: &[String]) -> String {
/// Derive a string explaining why the command was forbidden. If `justification`
/// is set by the user, this can contain instructions with recommended
/// alternatives, for example.
fn derive_forbidden_reason(command_args: &[String], evaluation: &Evaluation) -> String {
fn derive_forbidden_reason(
command_args: &[String],
evaluation: &Evaluation,
dangerous_command_match: Option<DangerousCommandMatch>,
) -> String {
let command = render_shlex_command(command_args);
let most_specific_forbidden = evaluation
@@ -980,7 +1021,37 @@ fn derive_forbidden_reason(command_args: &[String], evaluation: &Evaluation) ->
let prefix = render_shlex_command(matched_prefix);
format!("`{command}` rejected: policy forbids commands starting with `{prefix}`")
}
None => format!("`{command}` rejected: blocked by policy"),
None => {
if let Some(dangerous_command_match) = dangerous_command_match {
let reason = dangerous_command_rejection_reason(dangerous_command_match);
format!("`{command}` rejected: {reason}")
} else {
format!("`{command}` rejected: blocked by policy")
}
}
}
}
fn derive_rejected_prompt_reason(
fallback_reason: &str,
dangerous_command_match: Option<DangerousCommandMatch>,
) -> String {
match dangerous_command_match {
Some(dangerous_command_match @ DangerousCommandMatch::ForcedRm) => {
dangerous_command_rejection_reason(dangerous_command_match).to_string()
}
Some(DangerousCommandMatch::Other) | None => fallback_reason.to_string(),
}
}
fn dangerous_command_rejection_reason(
dangerous_command_match: DangerousCommandMatch,
) -> &'static str {
match dangerous_command_match {
DangerousCommandMatch::ForcedRm => {
"rm -f style commands are not permitted. Use a safer approach"
}
DangerousCommandMatch::Other => "blocked by policy",
}
}
codex-rs/core/src/exec_policy_tests.rs
+105 -55
View File
@@ -224,7 +224,7 @@ async fn returns_empty_policy_when_no_policy_files_exist() {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec!["rm".to_string()],
decision: Decision::Allow
decision: Decision::Allow,
}],
},
policy.check_multiple(commands.iter(), &|_| Decision::Allow)
@@ -481,7 +481,7 @@ async fn ignores_policies_outside_policy_dir() {
decision: Decision::Allow,
matched_rules: vec![RuleMatch::HeuristicsRuleMatch {
command: vec!["ls".to_string()],
decision: Decision::Allow
decision: Decision::Allow,
}],
},
policy.check_multiple(command.iter(), &|_| Decision::Allow)
@@ -1346,6 +1346,28 @@ async fn exec_approval_requirement_rejects_unmatched_sandbox_escalation_when_gra
.await;
}
#[test]
fn other_danger_preserves_rejected_prompt_reason() {
assert_eq!(
derive_rejected_prompt_reason(
REJECT_SANDBOX_APPROVAL_REASON,
Some(DangerousCommandMatch::Other),
),
REJECT_SANDBOX_APPROVAL_REASON
);
}
#[test]
fn forced_rm_rejected_prompt_reason_does_not_repeat_command() {
assert_eq!(
derive_rejected_prompt_reason(
REJECT_SANDBOX_APPROVAL_REASON,
Some(DangerousCommandMatch::ForcedRm),
),
"rm -f style commands are not permitted. Use a safer approach"
);
}
#[tokio::test]
async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision() {
let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt")"#;
@@ -1384,7 +1406,7 @@ async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision()
}
#[tokio::test]
async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled() {
async fn forced_rm_preserves_rule_rejection_when_granular_rules_are_disabled() {
let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt")"#;
let mut parser = PolicyParser::new();
parser
@@ -1394,7 +1416,7 @@ async fn mixed_rule_and_sandbox_prompt_rejects_when_granular_rules_are_disabled(
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"git status && madeup-cmd".to_string(),
"git status && rm -rf /tmp/example".to_string(),
];
let requirement = manager
@@ -2015,10 +2037,85 @@ async fn dangerous_rm_rf_requires_approval_in_danger_full_access() {
.await;
}
#[tokio::test]
async fn dangerous_rm_rf_in_shell_loop_requires_approval_in_danger_full_access() {
let command = vec_str(&[
"bash",
"-lc",
"for target in /tmp/a /tmp/b; do rm -rf \"$target\"; done",
]);
assert_exec_approval_requirement_for_command(
ExecApprovalRequirementScenario {
policy_src: None,
command: command.clone(),
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::Disabled,
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
},
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)),
},
)
.await;
}
fn vec_str(items: &[&str]) -> Vec<String> {
items.iter().map(std::string::ToString::to_string).collect()
}
#[tokio::test]
async fn forced_rm_requires_approval_or_specific_rejection_on_all_platforms() {
let policy = ExecPolicyManager::new(Arc::new(Policy::empty()));
let permissions = SandboxPermissions::UseDefault;
let dangerous_command = vec_str(&["rm", "-rf", "/important/data"]);
assert_eq!(
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[
"rm",
"-rf",
"/important/data",
]))),
},
policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::read_only(),
windows_sandbox_level: WindowsSandboxLevel::Disabled,
sandbox_permissions: permissions,
prefix_rule: None,
})
.await,
r#"On all platforms, a forbidden command should require approval
(unless AskForApproval::Never is specified)."#
);
// A dangerous command should be forbidden if the user has specified
// AskForApproval::Never.
assert_eq!(
ExecApprovalRequirement::Forbidden {
reason: "`rm -rf /important/data` rejected: rm -f style commands are not permitted. Use a safer approach"
.to_string(),
},
policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::Never,
permission_profile: PermissionProfile::read_only(),
windows_sandbox_level: WindowsSandboxLevel::Disabled,
sandbox_permissions: permissions,
prefix_rule: None,
})
.await,
r#"On all platforms, a forbidden command should require approval
(unless AskForApproval::Never is specified)."#
);
}
/// Note this test behaves differently on Windows because it exercises an
/// `if cfg!(windows)` code path in render_decision_for_unmatched_command().
#[tokio::test]
@@ -2074,55 +2171,10 @@ async fn verify_approval_requirement_for_unsafe_powershell_command() {
.await,
"{pwsh_approval_reason}"
);
// This is flagged as a dangerous command on all platforms.
let dangerous_command = vec_str(&["rm", "-rf", "/important/data"]);
assert_eq!(
ExecApprovalRequirement::NeedsApproval {
reason: None,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec_str(&[
"rm",
"-rf",
"/important/data",
]))),
},
policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::OnRequest,
permission_profile: PermissionProfile::read_only(),
windows_sandbox_level: WindowsSandboxLevel::Disabled,
sandbox_permissions: permissions,
prefix_rule: None,
})
.await,
r#"On all platforms, a forbidden command should require approval
(unless AskForApproval::Never is specified)."#
);
// A dangerous command should be forbidden if the user has specified
// AskForApproval::Never.
assert_eq!(
ExecApprovalRequirement::Forbidden {
reason: "`rm -rf /important/data` rejected: blocked by policy".to_string(),
},
policy
.create_exec_approval_requirement_for_command(ExecApprovalRequest {
command: &dangerous_command,
approval_policy: AskForApproval::Never,
permission_profile: PermissionProfile::read_only(),
windows_sandbox_level: WindowsSandboxLevel::Disabled,
sandbox_permissions: permissions,
prefix_rule: None,
})
.await,
r#"On all platforms, a forbidden command should require approval
(unless AskForApproval::Never is specified)."#
);
}
#[tokio::test]
async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
async fn dangerous_command_forbidden_when_sandbox_is_explicitly_disabled() {
let command = vec_str(&["rm", "-rf", "/tmp/nonexistent"]);
assert_exec_approval_requirement_for_command(
ExecApprovalRequirementScenario {
@@ -2135,11 +2187,9 @@ async fn dangerous_command_allowed_when_sandbox_is_explicitly_disabled() {
sandbox_permissions: SandboxPermissions::UseDefault,
prefix_rule: None,
},
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: Some(ExecPolicyAmendment {
command: vec_str(&["rm", "-rf", "/tmp/nonexistent"]),
}),
ExecApprovalRequirement::Forbidden {
reason: "`rm -rf /tmp/nonexistent` rejected: rm -f style commands are not permitted. Use a safer approach"
.to_string(),
},
)
.await;
codex-rs/core/tests/suite/exec_policy.rs
+144
View File
@@ -9,7 +9,9 @@ use codex_protocol::config_types::Settings;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::GranularApprovalConfig;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::user_input::UserInput;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
@@ -27,6 +29,8 @@ use serde_json::Value;
use serde_json::json;
use std::fs;
const COMPLEX_FORCED_RM_COMMAND: &str = "for target in \"\"; do rm -rf \"$target\"; done";
fn collaboration_mode_for_model(model: String) -> CollaborationMode {
CollaborationMode {
mode: ModeKind::Default,
@@ -90,6 +94,146 @@ fn assert_no_matched_rules_invariant(output_item: &Value) {
);
}
#[tokio::test]
async fn granular_complex_forced_rm_denial_explains_why_the_command_was_rejected() -> Result<()> {
skip_if_target_windows!(Ok(()), "uses a POSIX shell command fixture");
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_with_auto_env(&server).await?;
let call_id = "forced-rm-denied";
let args = json!({
"command": COMPLEX_FORCED_RM_COMMAND,
"timeout_ms": 1_000,
});
mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-forced-rm-1"),
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
ev_completed("resp-forced-rm-1"),
]),
)
.await;
let results_mock = mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-forced-rm-1", "done"),
ev_completed("resp-forced-rm-2"),
]),
)
.await;
submit_user_turn(
&test,
"run the forced rm loop",
AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: false,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}),
PermissionProfile::read_only(),
/*collaboration_mode*/ None,
)
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let output_item = results_mock.single_request().function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("function call output should include a string output payload");
assert!(
output.contains("rm -f style commands are not permitted. Use a safer approach"),
"unexpected output: {output}"
);
Ok(())
}
#[tokio::test]
async fn granular_complex_forced_rm_requests_approval_when_allowed() -> Result<()> {
skip_if_target_windows!(Ok(()), "uses a POSIX shell command fixture");
let server = start_mock_server().await;
let mut builder = test_codex();
let test = builder.build_with_auto_env(&server).await?;
let call_id = "forced-rm-approval";
let args = json!({
"command": COMPLEX_FORCED_RM_COMMAND,
"timeout_ms": 1_000,
});
mount_sse_once(
&server,
sse(vec![
ev_response_created("resp-forced-rm-approval-1"),
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
ev_completed("resp-forced-rm-approval-1"),
]),
)
.await;
mount_sse_once(
&server,
sse(vec![
ev_assistant_message("msg-forced-rm-approval-1", "done"),
ev_completed("resp-forced-rm-approval-2"),
]),
)
.await;
submit_user_turn(
&test,
"run the forced rm loop",
AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}),
PermissionProfile::read_only(),
/*collaboration_mode*/ None,
)
.await?;
let approval_event = wait_for_event(&test.codex, |event| {
matches!(
event,
EventMsg::ExecApprovalRequest(_) | EventMsg::TurnComplete(_)
)
})
.await;
let EventMsg::ExecApprovalRequest(approval) = approval_event else {
panic!("expected forced rm to request approval before turn completion");
};
assert_eq!(
approval.command.last().map(String::as_str),
Some(COMPLEX_FORCED_RM_COMMAND)
);
test.codex
.submit(Op::ExecApproval {
id: approval.effective_approval_id(),
turn_id: None,
decision: ReviewDecision::Denied,
})
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
Ok(())
}
#[cfg(windows)]
#[tokio::test]
async fn unified_exec_disabled_windows_sandbox_rejects_managed_read_only_command() -> Result<()> {
codex-rs/shell-command/src/bash.rs
+73
View File
@@ -123,6 +123,39 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option<Vec<Vec<Strin
parse_shell_script_into_commands(script)
}
/// Extracts the literal portions of command invocations from a shell script.
///
/// Unlike [`parse_shell_lc_plain_commands`], this accepts complex shell syntax
/// and returns the statically known words from every command node in a valid
/// syntax tree. Dynamic words and redirections are omitted. This is suitable
/// for identifying dangerous literal commands, but must not be used to prove
/// that a command is safe.
pub(crate) fn parse_shell_lc_literal_commands(command: &[String]) -> Option<Vec<Vec<String>>> {
let (_, script) = extract_bash_command(command)?;
let tree = try_parse_shell(script)?;
let root = tree.root_node();
if root.has_error() {
return None;
}
let mut commands = Vec::new();
let mut stack = vec![root];
while let Some(node) = stack.pop() {
if node.kind() == "command"
&& let Some(command) = parse_literal_command_from_node(node, script)
{
commands.push(command);
}
let mut cursor = node.walk();
for child in node.named_children(&mut cursor) {
stack.push(child);
}
}
Some(commands)
}
/// Returns the parsed argv for a single shell command in a here-doc style
/// script (`<<`), as long as the script contains exactly one command node.
pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option<Vec<String>> {
@@ -201,6 +234,46 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
Some(words)
}
fn parse_literal_command_from_node(cmd: Node<'_>, src: &str) -> Option<Vec<String>> {
if cmd.kind() != "command" {
return None;
}
let mut words = Vec::new();
let mut found_command_name = false;
let mut cursor = cmd.walk();
for child in cmd.named_children(&mut cursor) {
if child.kind() == "command_name" {
let command_name = parse_literal_shell_word(child.named_child(0)?, src)?;
words.push(command_name);
found_command_name = true;
} else if found_command_name && let Some(word) = parse_literal_shell_word(child, src) {
words.push(word);
}
}
found_command_name.then_some(words)
}
fn parse_literal_shell_word(node: Node<'_>, src: &str) -> Option<String> {
match node.kind() {
"word" | "number" if is_literal_word_or_number(node) => {
Some(node.utf8_text(src.as_bytes()).ok()?.to_owned())
}
"string" => parse_double_quoted_string(node, src),
"raw_string" => parse_raw_string(node, src),
"concatenation" => {
let mut concatenated = String::new();
let mut cursor = node.walk();
for part in node.named_children(&mut cursor) {
concatenated.push_str(&parse_literal_shell_word(part, src)?);
}
(!concatenated.is_empty()).then_some(concatenated)
}
_ => None,
}
}
fn parse_heredoc_command_words(cmd: Node<'_>, src: &str) -> Option<Vec<String>> {
if cmd.kind() != "command" {
return None;
codex-rs/shell-command/src/command_safety/is_dangerous_command.rs
+186 -33
View File
@@ -1,45 +1,69 @@
use crate::bash::parse_shell_lc_plain_commands;
use crate::bash::parse_shell_lc_literal_commands;
use std::path::Path;
#[cfg(windows)]
#[path = "windows_dangerous_commands.rs"]
mod windows_dangerous_commands;
pub fn command_might_be_dangerous(command: &[String]) -> bool {
/// Identifies the dangerous-command rule matched by a command invocation.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum DangerousCommandMatch {
/// An `rm` invocation includes the force option.
ForcedRm,
/// Another dangerous-command rule matched.
Other,
}
const MAX_DANGEROUS_COMMAND_WRAPPER_DEPTH: usize = 8;
/// Returns the dangerous-command rule matched by an already-tokenized command.
pub fn dangerous_command_match(command: &[String]) -> Option<DangerousCommandMatch> {
dangerous_command_match_with_depth(command, /*wrapper_depth*/ 0)
}
fn dangerous_command_match_with_depth(
command: &[String],
wrapper_depth: usize,
) -> Option<DangerousCommandMatch> {
if wrapper_depth > MAX_DANGEROUS_COMMAND_WRAPPER_DEPTH {
return None;
}
if let Some(dangerous_match) = dangerous_command_match_for_exec(command, wrapper_depth) {
return Some(dangerous_match);
}
// Support shell scripts where any literal command might be dangerous,
// including commands nested in control flow or substitutions.
if let Some(dangerous_match) = parse_shell_lc_literal_commands(command).and_then(|commands| {
commands
.iter()
.find_map(|command| dangerous_command_match_with_depth(command, wrapper_depth + 1))
}) {
return Some(dangerous_match);
}
#[cfg(windows)]
{
if windows_dangerous_commands::is_dangerous_command_windows(command) {
return true;
return Some(DangerousCommandMatch::Other);
}
}
if is_dangerous_to_call_with_exec(command) {
return true;
}
// Support `bash -lc "<script>"` where the any part of the script might contain a dangerous command.
if let Some(all_commands) = parse_shell_lc_plain_commands(command)
&& all_commands
.iter()
.any(|cmd| is_dangerous_to_call_with_exec(cmd))
{
return true;
}
false
None
}
/// Returns whether already-tokenized PowerShell words should be treated as
/// dangerous by the Windows unmatched-command heuristics.
pub fn is_dangerous_powershell_words(command: &[String]) -> bool {
/// Returns the dangerous-command rule matched by tokenized PowerShell words.
pub fn dangerous_powershell_words_match(command: &[String]) -> Option<DangerousCommandMatch> {
#[cfg(windows)]
{
windows_dangerous_commands::is_dangerous_powershell_words(command)
.then_some(DangerousCommandMatch::Other)
}
#[cfg(not(windows))]
{
let _ = command;
false
None
}
}
@@ -142,23 +166,89 @@ pub(crate) fn find_git_subcommand<'a>(
None
}
fn is_dangerous_to_call_with_exec(command: &[String]) -> bool {
let cmd0 = command.first().map(String::as_str);
fn dangerous_command_match_for_exec(
command: &[String],
wrapper_depth: usize,
) -> Option<DangerousCommandMatch> {
let cmd0 = command
.first()
.and_then(|command| executable_name_lookup_key(command));
match cmd0 {
Some("rm") => matches!(command.get(1).map(String::as_str), Some("-f" | "-rf")),
match cmd0.as_deref() {
Some("rm") if rm_args_include_force_option(&command[1..]) => {
Some(DangerousCommandMatch::ForcedRm)
}
// for sudo <cmd> simply do the check for <cmd>
Some("sudo") => is_dangerous_to_call_with_exec(&command[1..]),
// For sudo <cmd>, simply check <cmd>.
Some("sudo") => dangerous_command_match_with_depth(&command[1..], wrapper_depth + 1),
// Skip environment assignments before checking the command run by env.
Some("env") => dangerous_command_match_for_env(command, wrapper_depth),
// A trap action is shell source stored in the first operand.
Some("trap") => dangerous_command_match_for_trap(command, wrapper_depth),
// ── anything else ─────────────────────────────────────────────────
_ => false,
_ => None,
}
}
fn dangerous_command_match_for_env(
command: &[String],
wrapper_depth: usize,
) -> Option<DangerousCommandMatch> {
let mut command_index = 1;
while let Some(argument) = command.get(command_index) {
if argument == "--" {
command_index += 1;
break;
}
if matches!(argument.as_str(), "-i" | "--ignore-environment")
|| argument
.split_once('=')
.is_some_and(|(name, _)| !name.is_empty() && !name.starts_with('-'))
{
command_index += 1;
continue;
}
break;
}
dangerous_command_match_with_depth(&command[command_index..], wrapper_depth + 1)
}
fn dangerous_command_match_for_trap(
command: &[String],
wrapper_depth: usize,
) -> Option<DangerousCommandMatch> {
let mut action_index = 1;
if command
.get(action_index)
.is_some_and(|argument| argument == "--")
{
action_index += 1;
}
let action = command
.get(action_index)
.filter(|action| !action.starts_with('-'))?;
let shell_command = vec!["sh".to_string(), "-c".to_string(), action.clone()];
dangerous_command_match_with_depth(&shell_command, wrapper_depth + 1)
}
fn rm_args_include_force_option(args: &[String]) -> bool {
args.iter()
.take_while(|arg| arg.as_str() != "--")
.any(|arg| {
arg == "--force"
|| arg
.strip_prefix('-')
.is_some_and(|flags| !flags.starts_with('-') && flags.contains('f'))
})
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
fn vec_str(items: &[&str]) -> Vec<String> {
items.iter().map(std::string::ToString::to_string).collect()
@@ -166,22 +256,85 @@ mod tests {
#[test]
fn rm_rf_is_dangerous() {
assert!(command_might_be_dangerous(&vec_str(&["rm", "-rf", "/"])));
assert_eq!(
dangerous_command_match(&vec_str(&["rm", "-rf", "/"])),
Some(DangerousCommandMatch::ForcedRm)
);
}
#[test]
fn rm_f_is_dangerous() {
assert!(command_might_be_dangerous(&vec_str(&["rm", "-f", "/"])));
assert_eq!(
dangerous_command_match(&vec_str(&["rm", "-f", "/"])),
Some(DangerousCommandMatch::ForcedRm)
);
}
#[test]
fn direct_powershell_words_reuse_windows_dangerous_detection() {
fn forced_rm_variants_are_dangerous() {
for command in [
vec_str(&["/bin/rm", "-fr", "/tmp/example"]),
vec_str(&["rm", "-r", "-f", "/tmp/example"]),
vec_str(&["rm", "--force", "/tmp/example"]),
vec_str(&["rm", "/tmp/example", "-f"]),
vec_str(&["sudo", "rm", "-rf", "/tmp/example"]),
vec_str(&["env", "TARGET=/tmp/example", "rm", "-rf", "/tmp/example"]),
] {
assert_eq!(
dangerous_command_match(&command),
Some(DangerousCommandMatch::ForcedRm),
"{command:?}"
);
}
}
#[test]
fn forced_rm_in_complex_shell_syntax_is_dangerous() {
for script in [
"printf x | rm -rf /tmp/example",
"if test -d /tmp/example; then rm --force /tmp/example; fi",
"rm -rf \"$TARGET\" >/dev/null",
"for target in /tmp/a /tmp/b; do rm -r -f \"$target\"; done",
"echo \"$(rm -rf /tmp/example)\"",
"bash -c 'rm -rf /tmp/example'",
"trap 'rm -rf /tmp/example' EXIT",
"for a in '-C5a25KeRr' '--' '--json' '--bogus'; do HOME=$(mktemp -d) MDE_URL=http://127.0.0.1:1 MDE_TOKEN=x node cli/mde.cjs ls \"$a\" >/tmp/mde-review-out 2>/tmp/mde-review-err; code=$?; printf '%s\\t%s\\t%s\\n' \"$a\" \"$code\" \"$(tr '\\n' ' ' </tmp/mde-review-err)\"; rm -rf \"$HOME\"; done",
] {
let command = vec_str(&["bash", "-lc", script]);
assert_eq!(
dangerous_command_match(&command),
Some(DangerousCommandMatch::ForcedRm),
"{script}"
);
}
}
#[test]
fn non_forced_or_non_literal_rm_is_not_dangerous() {
for command in [
vec_str(&["rm", "-r", "/tmp/example"]),
vec_str(&["rm", "--", "-f"]),
vec_str(&["bash", "-lc", "echo 'rm -rf /tmp/example'"]),
vec_str(&["bash", "-lc", "cmd=rm; $cmd -rf /tmp/example"]),
vec_str(&["bash", "-lc", "if then rm -rf /tmp/example"]),
vec_str(&["env", "TARGET=/tmp/example", "rm", "-r", "/tmp/example"]),
vec_str(&["bash", "-lc", "trap 'echo rm -rf /tmp/example' EXIT"]),
] {
assert_eq!(dangerous_command_match(&command), None, "{command:?}");
}
}
#[test]
fn direct_powershell_words_return_other_match_on_windows() {
let command = vec_str(&["Remove-Item", "test", "-Force"]);
if cfg!(windows) {
assert!(is_dangerous_powershell_words(&command));
assert_eq!(
dangerous_powershell_words_match(&command),
Some(DangerousCommandMatch::Other)
);
} else {
assert!(!is_dangerous_powershell_words(&command));
assert_eq!(dangerous_powershell_words_match(&command), None);
}
}
}