diff --git a/crates/astra-sandbox/src/bash_ast.rs b/crates/astra-sandbox/src/bash_ast.rs index 63c24c8005..5a310ed9a8 100644 --- a/crates/astra-sandbox/src/bash_ast.rs +++ b/crates/astra-sandbox/src/bash_ast.rs @@ -212,6 +212,49 @@ fn parse_plain_command(node: Node<'_>, source: &str) -> Option> { (!words.is_empty()).then_some(words) } +#[derive(Debug)] +enum CommandWord { + Literal(String), + Dynamic, +} + +impl CommandWord { + fn literal(&self) -> Option<&str> { + match self { + Self::Literal(value) => Some(value), + Self::Dynamic => None, + } + } +} + +fn command_words(node: Node<'_>, source: &str) -> Option> { + if node.kind() != "command" || node.has_error() { + return None; + } + let mut words = Vec::new(); + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + let word_node = match child.kind() { + "command_name" => child.named_child(0)?, + "word" | "number" | "string" | "raw_string" | "concatenation" => child, + "variable_assignment" + | "file_redirect" + | "heredoc_redirect" + | "herestring_redirect" => continue, + _ => { + words.push(CommandWord::Dynamic); + continue; + } + }; + words.push( + parse_plain_word(word_node, source) + .map(CommandWord::Literal) + .unwrap_or(CommandWord::Dynamic), + ); + } + (!words.is_empty()).then_some(words) +} + fn parse_plain_word(node: Node<'_>, source: &str) -> Option { match node.kind() { "word" | "number" => { @@ -270,7 +313,7 @@ fn decode_unquoted_shell_word(raw: &str) -> Option { while let Some(ch) = chars.next() { if ch == '\\' { value.push(chars.next()?); - } else if matches!(ch, '*' | '?' | '[' | ']') { + } else if matches!(ch, '*' | '?' | '[') { // Unquoted pathname/brace expansion means the source spelling is // not the argv Bash will execute. Escaped forms took the branch // above and are safe literal characters. @@ -346,23 +389,733 @@ fn decode_double_quoted_content(raw: &str) -> Option { /// and avoids false positives from string literals. /// Returns detected risks. If the shell cannot be parsed, returns an empty vector (no substring fallback). pub fn analyze_bash_risks_ast(command: &str) -> Vec { + analyze_bash_risks_ast_inner(command, 0) +} + +fn analyze_bash_risks_ast_inner(command: &str, shell_depth: usize) -> Vec { let Some(tree) = parse_bash(command) else { return Vec::new(); }; let root = tree.root_node(); - let mut ctx = RiskCtx::new(command); + let mut ctx = RiskCtx::with_shell_depth(command, shell_depth); + visit_node(root, &mut ctx); ctx.into_risks() } +enum NestedShellScript<'a> { + None, + Script(&'a str), + Ambiguous, +} + +const SHELL_LONG_OPTIONS: &[&str] = &[ + "--debug", + "--debugger", + "--dump-po-strings", + "--dump-strings", + "--help", + "--login", + "--noediting", + "--noprofile", + "--norc", + "--posix", + "--pretty-print", + "--restricted", + "--verbose", + "--version", +]; +const SHELL_LONG_OPTIONS_WITH_VALUE: &[&str] = &["--init-file", "--rcfile"]; +const SHELL_SHORT_OPTIONS: &str = "abefhiklmnprstuvxBCEHPTDqVE"; + +fn nested_shell_script(words: &[CommandWord]) -> NestedShellScript<'_> { + let index = match resolve_transparent_launcher(words, 0) { + Ok(Some(index)) => index, + Ok(None) => return NestedShellScript::None, + Err(()) => return NestedShellScript::Ambiguous, + }; + let Some(executable) = words + .get(index) + .and_then(CommandWord::literal) + .map(command_basename) + else { + return NestedShellScript::Ambiguous; + }; + if !matches!(executable.as_str(), "bash" | "sh" | "dash" | "zsh" | "ksh") { + return NestedShellScript::None; + } + + let mut argument_index = index + 1; + while let Some(word) = words.get(argument_index) { + let Some(argument) = word.literal() else { + return NestedShellScript::Ambiguous; + }; + if argument == "--" + || argument == "-" + || (!argument.starts_with('-') && !argument.starts_with('+')) + { + return NestedShellScript::None; + } + + if argument.starts_with("--") { + let (option, inline_value) = argument + .split_once('=') + .map_or((argument, false), |(option, _)| (option, true)); + if SHELL_LONG_OPTIONS.contains(&option) && !inline_value { + argument_index += 1; + continue; + } + if SHELL_LONG_OPTIONS_WITH_VALUE.contains(&option) { + if !inline_value { + if words.get(argument_index + 1).is_none() { + return NestedShellScript::Ambiguous; + } + argument_index += 1; + } + argument_index += 1; + continue; + } + return NestedShellScript::Ambiguous; + } + + let Some(flags) = argument.get(1..) else { + return NestedShellScript::Ambiguous; + }; + if flags.is_empty() + || flags + .chars() + .any(|flag| !SHELL_SHORT_OPTIONS.contains(flag) && !matches!(flag, 'c' | 'o' | 'O')) + { + return NestedShellScript::Ambiguous; + } + if argument.starts_with('+') && flags.contains('c') { + return NestedShellScript::Ambiguous; + } + let option_name_count = flags.matches(['o', 'O']).count(); + if flags.contains('c') { + return words + .get(argument_index + 1 + option_name_count) + .and_then(CommandWord::literal) + .map_or(NestedShellScript::Ambiguous, NestedShellScript::Script); + } + if words.len() < argument_index + 1 + option_name_count { + return NestedShellScript::Ambiguous; + } + argument_index += 1 + option_name_count; + } + NestedShellScript::None +} + +const DESTRUCTIVE_COMMANDS: &[&str] = &[ + "dd", + "mkswap", + "truncate", + "shred", + "wipefs", + "blkdiscard", + "fdisk", + "sfdisk", + "parted", + "cryptsetup", + "pvremove", + "vgremove", + "lvremove", + "zpool", + "zfs", + "shutdown", + "reboot", + "poweroff", + "halt", + "telinit", +]; + +enum DestructiveCommandResolution { + Safe, + Destructive(String), + Ambiguous, +} + +fn resolve_destructive_command( + words: &[CommandWord], + shell_depth: usize, +) -> DestructiveCommandResolution { + let index = match resolve_transparent_launcher(words, 0) { + Ok(Some(index)) => index, + Ok(None) => return DestructiveCommandResolution::Safe, + Err(()) => return DestructiveCommandResolution::Ambiguous, + }; + let Some(executable) = words.get(index).and_then(CommandWord::literal) else { + return DestructiveCommandResolution::Ambiguous; + }; + let executable = command_basename(executable); + if executable == "mkfs" || executable.starts_with("mkfs.") { + return DestructiveCommandResolution::Destructive("mkfs".to_string()); + } + if let Some(name) = DESTRUCTIVE_COMMANDS + .iter() + .copied() + .find(|candidate| executable.eq_ignore_ascii_case(candidate)) + { + return DestructiveCommandResolution::Destructive(name.to_string()); + } + match nested_shell_script(words) { + NestedShellScript::Script(_) if shell_depth >= 16 => { + return DestructiveCommandResolution::Ambiguous; + } + NestedShellScript::Script(script) => { + let nested_risks = analyze_bash_risks_ast_inner(script, shell_depth + 1); + if let Some(name) = nested_risks.into_iter().find_map(|risk| match risk { + CommandRisk::DestructiveCommand(name) => Some(name), + _ => None, + }) { + return DestructiveCommandResolution::Destructive(name); + } + } + NestedShellScript::Ambiguous => return DestructiveCommandResolution::Ambiguous, + NestedShellScript::None => {} + } + + match executable.as_str() { + "busybox" | "toybox" => resolve_multicall_applet(&words[index + 1..], shell_depth), + "xargs" => resolve_xargs_command(&words[index + 1..], shell_depth), + "find" => resolve_find_commands(&words[index + 1..], shell_depth), + _ => DestructiveCommandResolution::Safe, + } +} + +fn resolve_transparent_launcher( + words: &[CommandWord], + mut index: usize, +) -> Result, ()> { + loop { + let Some(word) = words.get(index) else { + return Ok(None); + }; + let executable = command_basename(word.literal().ok_or(())?); + index += 1; + match executable.as_str() { + "command" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new("pVv", "", &[], &[], &[]), + )? + else { + return Ok(None); + }; + index = next; + } + "builtin" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new("", "", &["--help"], &[], &[]), + )? + else { + return Ok(None); + }; + index = next; + } + "exec" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new("cl", "a", &["--help"], &[], &[]), + )? + else { + return Ok(None); + }; + index = next; + } + "nohup" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new("", "", &["--help", "--version"], &[], &[]), + )? + else { + return Ok(None); + }; + index = next; + } + "env" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "i0v", + "uCPa", + &[ + "--ignore-environment", + "--null", + "--debug", + "--list-signal-handling", + "--help", + "--version", + ], + &["--unset", "--chdir", "--path", "--argv0"], + &["--block-signal", "--default-signal", "--ignore-signal"], + ), + )? + else { + return Ok(None); + }; + index = next; + while words + .get(index) + .and_then(CommandWord::literal) + .is_some_and(is_assignment) + { + index += 1; + } + } + "sudo" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "ABbEHikKlNnPSseVv", + "CDghpRTUurt", + &[ + "--askpass", + "--background", + "--bell", + "--edit", + "--set-home", + "--help", + "--login", + "--remove-timestamp", + "--reset-timestamp", + "--list", + "--non-interactive", + "--preserve-groups", + "--stdin", + "--shell", + "--version", + "--validate", + ], + &[ + "--close-from", + "--chdir", + "--group", + "--host", + "--prompt", + "--chroot", + "--command-timeout", + "--other-user", + "--role", + "--type", + "--user", + ], + &["--preserve-env"], + ), + )? + else { + return Ok(None); + }; + index = next; + } + "doas" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new("Lns", "aCu", &[], &[], &[]), + )? + else { + return Ok(None); + }; + index = next; + } + "pkexec" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "", + "", + &["--disable-internal-agent", "--keep-cwd", "--version"], + &["--user"], + &[], + ), + )? + else { + return Ok(None); + }; + index = next; + } + "timeout" | "gtimeout" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "fpv", + "ks", + &[ + "--foreground", + "--preserve-status", + "--verbose", + "--help", + "--version", + ], + &["--kill-after", "--signal"], + &[], + ), + )? + else { + return Ok(None); + }; + let Some(next) = skip_launcher_operands(words, next, 1)? else { + return Ok(None); + }; + index = next; + } + "nice" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "", + "n", + &["--help", "--version"], + &["--adjustment"], + &[], + ) + .with_legacy_numeric_short_option(), + )? + else { + return Ok(None); + }; + index = next; + } + "ionice" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "thV", + "cnpPu", + &["--ignore", "--help", "--version"], + &["--class", "--classdata", "--pid", "--pgid", "--uid"], + &[], + ), + )? + else { + return Ok(None); + }; + index = next; + } + "setsid" => { + let Some(next) = skip_launcher_options( + words, + index, + LauncherOptionGrammar::new( + "cfwhV", + "", + &["--ctty", "--fork", "--wait", "--help", "--version"], + &[], + &[], + ), + )? + else { + return Ok(None); + }; + index = next; + } + _ => return Ok(Some(index - 1)), + } + } +} + +#[derive(Clone, Copy)] +struct LauncherOptionGrammar { + short_flags: &'static str, + short_options_with_value: &'static str, + long_flags: &'static [&'static str], + long_options_with_value: &'static [&'static str], + long_options_with_optional_value: &'static [&'static str], + legacy_numeric_short_option: bool, +} + +impl LauncherOptionGrammar { + const fn new( + short_flags: &'static str, + short_options_with_value: &'static str, + long_flags: &'static [&'static str], + long_options_with_value: &'static [&'static str], + long_options_with_optional_value: &'static [&'static str], + ) -> Self { + Self { + short_flags, + short_options_with_value, + long_flags, + long_options_with_value, + long_options_with_optional_value, + legacy_numeric_short_option: false, + } + } + + const fn with_legacy_numeric_short_option(mut self) -> Self { + self.legacy_numeric_short_option = true; + self + } +} + +fn skip_launcher_options( + words: &[CommandWord], + mut index: usize, + grammar: LauncherOptionGrammar, +) -> Result, ()> { + while let Some(word) = words.get(index) { + let argument = word.literal().ok_or(())?; + if argument == "--" { + return Ok((index + 1 < words.len()).then_some(index + 1)); + } + if !argument.starts_with('-') || argument == "-" { + return Ok(Some(index)); + } + + if argument.starts_with("--") { + let (option, inline_value) = argument + .split_once('=') + .map_or((argument, false), |(name, _)| (name, true)); + if grammar.long_flags.contains(&option) && !inline_value + || grammar.long_options_with_optional_value.contains(&option) + { + index += 1; + continue; + } + if !grammar.long_options_with_value.contains(&option) { + return Err(()); + } + index += 1; + if !inline_value { + if words.get(index).is_none() { + return Ok(None); + } + index += 1; + } + continue; + } + + if grammar.legacy_numeric_short_option + && argument.strip_prefix('-').is_some_and(|value| { + !value.is_empty() && value.chars().all(|ch| ch.is_ascii_digit()) + }) + { + index += 1; + continue; + } + + let mut flags = argument[1..].chars().peekable(); + if flags.peek().is_none() { + return Ok(Some(index)); + } + while let Some(flag) = flags.next() { + if grammar.short_flags.contains(flag) { + continue; + } + if !grammar.short_options_with_value.contains(flag) { + return Err(()); + } + if flags.peek().is_none() { + index += 1; + if words.get(index).is_none() { + return Ok(None); + } + } + break; + } + index += 1; + } + Ok(None) +} + +fn skip_launcher_operands( + words: &[CommandWord], + index: usize, + count: usize, +) -> Result, ()> { + let command_index = index.checked_add(count).ok_or(())?; + if command_index > words.len() { + return Ok(None); + } + Ok((command_index < words.len()).then_some(command_index)) +} + +fn resolve_multicall_applet( + words: &[CommandWord], + shell_depth: usize, +) -> DestructiveCommandResolution { + let Some(applet) = words.first().and_then(CommandWord::literal) else { + return if words.is_empty() { + DestructiveCommandResolution::Safe + } else { + DestructiveCommandResolution::Ambiguous + }; + }; + if applet.starts_with('-') { + return if matches!( + applet, + "--help" | "--list" | "--list-full" | "--install" | "--show" + ) { + DestructiveCommandResolution::Safe + } else { + DestructiveCommandResolution::Ambiguous + }; + } + resolve_destructive_command(words, shell_depth) +} + +fn resolve_xargs_command( + words: &[CommandWord], + shell_depth: usize, +) -> DestructiveCommandResolution { + const OPTIONS_WITH_VALUE: &[&str] = &[ + "-E", + "--eof", + "-I", + "--replace", + "-L", + "--max-lines", + "-n", + "--max-args", + "-P", + "--max-procs", + "-s", + "--max-chars", + "--process-slot-var", + "-a", + "--arg-file", + "-d", + "--delimiter", + ]; + const OPTIONS_WITH_ATTACHED_VALUE: &[&str] = &["-E", "-I", "-L", "-n", "-P", "-s", "-a", "-d"]; + const FLAGS: &[&str] = &[ + "-0", + "--null", + "-p", + "--interactive", + "-r", + "--no-run-if-empty", + "-t", + "--verbose", + "-x", + "--exit", + "--show-limits", + "--help", + "--version", + ]; + + let mut index = 0; + while let Some(word) = words.get(index) { + let Some(argument) = word.literal() else { + return DestructiveCommandResolution::Ambiguous; + }; + if argument == "--" { + index += 1; + break; + } + if !argument.starts_with('-') || argument == "-" { + break; + } + let option = argument.split_once('=').map_or(argument, |(name, _)| name); + if OPTIONS_WITH_VALUE.contains(&option) { + index += 1; + if !argument.contains('=') { + if words.get(index).is_none() { + return DestructiveCommandResolution::Ambiguous; + } + index += 1; + } + continue; + } + if OPTIONS_WITH_ATTACHED_VALUE + .iter() + .any(|prefix| argument.starts_with(prefix) && argument.len() > prefix.len()) + || FLAGS.contains(&argument) + || argument + .strip_prefix('-') + .is_some_and(|flags| flags.chars().all(|flag| "0prtx".contains(flag))) + { + index += 1; + continue; + } + return DestructiveCommandResolution::Ambiguous; + } + + if index == words.len() { + DestructiveCommandResolution::Safe + } else { + resolve_destructive_command(&words[index..], shell_depth) + } +} + +fn resolve_find_commands( + words: &[CommandWord], + shell_depth: usize, +) -> DestructiveCommandResolution { + let mut index = 0; + while index < words.len() { + let Some(argument) = words[index].literal() else { + index += 1; + continue; + }; + if !matches!(argument, "-exec" | "-execdir" | "-ok" | "-okdir") { + index += 1; + continue; + } + let command_start = index + 1; + let Some(command_end) = (command_start..words.len()).find(|candidate| { + words[*candidate] + .literal() + .is_some_and(|word| matches!(word, ";" | "+")) + }) else { + return DestructiveCommandResolution::Ambiguous; + }; + match resolve_destructive_command(&words[command_start..command_end], shell_depth) { + DestructiveCommandResolution::Safe => {} + result => return result, + } + index = command_end + 1; + } + DestructiveCommandResolution::Safe +} + +fn command_basename(raw: &str) -> String { + raw.rsplit(['/', '\\']) + .next() + .unwrap_or_default() + .to_ascii_lowercase() +} + +fn is_assignment(raw: &str) -> bool { + let Some((name, _)) = raw.split_once('=') else { + return false; + }; + let mut chars = name.chars(); + chars + .next() + .is_some_and(|first| first == '_' || first.is_ascii_alphabetic()) + && chars.all(|ch| ch == '_' || ch.is_ascii_alphanumeric()) +} + struct RiskCtx<'a> { src: &'a str, + shell_depth: usize, hits: Vec, } impl<'a> RiskCtx<'a> { fn new(src: &'a str) -> Self { - Self { src, hits: vec![] } + Self::with_shell_depth(src, 0) + } + + fn with_shell_depth(src: &'a str, shell_depth: usize) -> Self { + Self { + src, + shell_depth, + hits: vec![], + } } fn push(&mut self, risk: CommandRisk) { @@ -500,6 +1253,34 @@ fn analyze_redirection(node: Node<'_>, ctx: &mut RiskCtx<'_>) { } fn analyze_command_invocation(node: Node<'_>, ctx: &mut RiskCtx<'_>) { + if let Some(words) = command_words(node, ctx.src) { + match resolve_destructive_command(&words, ctx.shell_depth) { + DestructiveCommandResolution::Destructive(name) => { + ctx.push(CommandRisk::DestructiveCommand(name)); + } + DestructiveCommandResolution::Ambiguous => { + ctx.push(CommandRisk::RemoteCodeExecution); + } + DestructiveCommandResolution::Safe => {} + } + match nested_shell_script(&words) { + NestedShellScript::Script(script) if ctx.shell_depth < 16 => { + // Quoted `sh -c` input is a new shell program, unlike heredoc + // input to Python/Node. Parse it so wrappers cannot hide a + // real destructive command. + for risk in analyze_bash_risks_ast_inner(script, ctx.shell_depth + 1) { + ctx.push(risk); + } + } + NestedShellScript::Script(_) | NestedShellScript::Ambiguous => { + // An unbounded nesting depth or an option sequence whose + // command-string boundary is unclear cannot be authorized. + ctx.push(CommandRisk::RemoteCodeExecution); + } + NestedShellScript::None => {} + } + } + let Some(name) = command_name(node, ctx) else { return; }; @@ -785,6 +1566,149 @@ mod tests { assert!(risks.contains(&CommandRisk::EnvManipulation)); } + #[test] + fn destructive_commands_are_classified_from_command_nodes() { + for executable in + DESTRUCTIVE_COMMANDS + .iter() + .copied() + .chain(["mkfs", "mkfs.ext4", "mkfs.xfs"]) + { + let command = format!("/usr/sbin/{executable} --example"); + assert!( + analyze_bash_risks_ast(&command) + .iter() + .any(|risk| matches!(risk, CommandRisk::DestructiveCommand(_))), + "configured destructive executable must be detected: {command}" + ); + } + + for command in [ + "command dd if=/dev/zero of=/dev/sda", + "builtin dd if=/dev/zero of=/dev/sda", + "exec dd if=/dev/zero of=/dev/sda", + "exec -a alias dd if=/dev/zero of=/dev/sda", + "exec -cla alias dd if=/dev/zero of=/dev/sda", + "exec -a \"$alias\" dd if=/dev/zero of=/dev/sda", + "nohup dd if=/dev/zero of=/dev/sda", + "sudo wipefs -a /dev/sdb", + "sudo -D /tmp dd if=/dev/zero of=/dev/sda", + "doas wipefs -a /dev/sdb", + "doas -a passwd dd if=/dev/zero of=/dev/sda", + "pkexec wipefs -a /dev/sdb", + "pkexec --user root dd if=/dev/zero of=/dev/sda", + "env MODE=secure shred -u secrets.txt", + "env -u HOME dd if=/dev/zero of=/dev/sda", + "bash -lc 'dd if=/dev/zero of=/dev/sda'", + "bash -oc pipefail 'dd if=/dev/zero of=/dev/sda'", + "bash -oO pipefail extglob -c 'wipefs -a /dev/sdb'", + "sudo sh -c 'wipefs -a /dev/sdb'", + "bash --norc -c 'dd if=/dev/zero of=/dev/sda'", + "bash --rcfile /tmp/bashrc -c 'wipefs -a /dev/sdb'", + "sudo bash --norc -c 'dd if=/dev/zero of=/dev/sda'", + "env MODE=secure bash --rcfile /tmp/bashrc -c 'wipefs -a /dev/sdb'", + "busybox dd if=/dev/zero of=/dev/sda", + "toybox wipefs -a /dev/sdb", + "sudo busybox dd if=/dev/zero of=/dev/sda", + "timeout 5 dd if=/dev/zero of=/dev/sda", + "sudo timeout -s KILL 5 dd if=/dev/zero of=/dev/sda", + "nice -n 5 wipefs -a /dev/sdb", + "nice -5 dd if=/dev/zero of=/dev/sda", + "ionice -c 2 dd if=/dev/zero of=/dev/sda", + "setsid wipefs -a /dev/sdb", + "setsid env MODE=secure timeout 5 sudo dd if=/dev/zero of=/dev/sda", + "printf '%s\\n' data | xargs -n 1 dd if=/dev/zero of=/dev/sda", + "find . -exec dd if=/dev/zero of=/dev/sda {} \\;", + "find . -execdir sh -c 'wipefs -a /dev/sdb' {} \\;", + "printf data | xargs sh -c 'dd if=/dev/zero of=/dev/sda'", + ] { + assert!( + analyze_bash_risks_ast(command) + .iter() + .any(|risk| matches!(risk, CommandRisk::DestructiveCommand(_))), + "destructive command must be detected: {command}" + ); + } + } + + #[test] + fn destructive_words_in_data_are_not_commands() { + for command in [ + "echo dd", + "python3 -c 'dd = 1; print(dd)'", + "python3 <<'PY'\ndd = {'chart': 'bar'}\nprint(dd)\nPY", + "bash -c 'echo dd'", + "bash -oc pipefail 'echo dd'", + "bash --norc -c 'echo dd'", + "bash --rcfile /tmp/bashrc -c 'echo dd'", + "exec -a alias printf '%s\\n' dd", + "env -u HOME printf '%s\\n' dd", + "sudo -u root printf '%s\\n' dd", + "timeout 5 printf '%s\\n' dd", + "nice -n 5 printf '%s\\n' dd", + "ionice -c 2 printf '%s\\n' dd", + "setsid printf '%s\\n' dd", + "busybox echo dd", + "printf '%s\\n' dd | xargs printf '%s\\n'", + "printf '%s\\n' dd | xargs", + "find . -name dd -print", + ] { + assert!( + !analyze_bash_risks_ast(command) + .iter() + .any(|risk| matches!(risk, CommandRisk::DestructiveCommand(_))), + "data must not be classified as a destructive command: {command}" + ); + } + } + + #[test] + fn ambiguous_shell_options_fail_closed() { + for command in [ + "bash --unknown-option -c 'echo safe'", + "bash +c 'echo safe'", + "bash --rcfile", + ] { + assert!( + analyze_bash_risks_ast(command).contains(&CommandRisk::RemoteCodeExecution), + "ambiguous shell invocation must fail closed: {command}" + ); + } + } + + #[test] + fn dynamic_dispatched_executables_fail_closed() { + for command in [ + "tool=dd; \"$tool\" if=/dev/zero of=/dev/sda", + "busybox \"$tool\" if=/dev/zero of=/dev/sda", + "printf data | xargs \"$tool\" if=/dev/zero of=/dev/sda", + "find . -exec \"$tool\" if=/dev/zero of=/dev/sda {} \\;", + ] { + assert!( + analyze_bash_risks_ast(command).contains(&CommandRisk::RemoteCodeExecution), + "dynamic executable position must fail closed: {command}" + ); + } + } + + #[test] + fn ambiguous_launcher_options_fail_closed() { + for command in [ + "exec --future-option dd if=/dev/zero of=/dev/sda", + "sudo --future-option dd if=/dev/zero of=/dev/sda", + "env -S 'dd if=/dev/zero of=/dev/sda'", + "timeout --future-option 5 dd if=/dev/zero of=/dev/sda", + "nice --future-option dd if=/dev/zero of=/dev/sda", + "ionice --future-option dd if=/dev/zero of=/dev/sda", + "setsid --future-option dd if=/dev/zero of=/dev/sda", + ] { + assert!( + analyze_bash_risks_ast(command).contains(&CommandRisk::RemoteCodeExecution), + "launcher option with unproven arity must fail closed: {command}" + ); + } + } + #[test] fn chmod_setuid_variants() { for cmd in [ diff --git a/crates/astra-sandbox/src/command.rs b/crates/astra-sandbox/src/command.rs index 0a8549ecea..5d180a8a90 100644 --- a/crates/astra-sandbox/src/command.rs +++ b/crates/astra-sandbox/src/command.rs @@ -230,8 +230,8 @@ fn analyze_command_risks_with_workspace( ) -> Vec { let mut risks = Vec::new(); - // 1) AST-level analysis (best-effort). This avoids many string-literal false positives. - // If parsing fails, we still fall back to the legacy heuristic scanner below. + // AST owns command-invocation risks, including destructive executables. The + // remaining checks below are limited to path and shell-feature diagnostics. let ast_risks = super::bash_ast::analyze_bash_risks_ast(command); let ast_parsed = super::bash_ast::parse_bash(command).is_some(); risks.extend(ast_risks); @@ -259,10 +259,6 @@ fn analyze_command_risks_with_workspace( push_unique(&mut risks, CommandRisk::CredentialAccess(path)); } - if let Some(cmd) = destructive_command(&lower) { - push_unique(&mut risks, CommandRisk::DestructiveCommand(cmd.to_string())); - } - if let Some(target) = workspace_out_write_target(command, workspace_root) { push_unique(&mut risks, CommandRisk::WorkspaceOutWrite(target)); } @@ -324,26 +320,6 @@ fn analyze_command_risks_with_workspace( risks } -const DESTRUCTIVE_COMMANDS: &[&str] = &[ - "dd", - "mkfs", - "mkfs.ext4", - "mkfs.xfs", - "truncate", - "shred", - "wipefs", - "blkdiscard", - "fdisk", - "sfdisk", - "parted", - "cryptsetup", - "pvremove", - "vgremove", - "lvremove", - "zpool", - "zfs", -]; - const CREDENTIAL_PATH_PREFIXES: &[&str] = &[ "/.ssh/", "~/.ssh/", @@ -369,18 +345,6 @@ const CREDENTIAL_PATH_PREFIXES: &[&str] = &[ const CREDENTIAL_FILE_NAMES: &[&str] = &["id_rsa", "id_ed25519", "id_ecdsa", "id_ed25519_sk"]; -fn destructive_command(lower: &str) -> Option<&'static str> { - for command in DESTRUCTIVE_COMMANDS { - if lower - .split(|ch: char| !(ch.is_ascii_alphanumeric() || ch == '.' || ch == '-' || ch == '_')) - .any(|token| token == *command) - { - return Some(*command); - } - } - None -} - fn credential_access_target(lower: &str) -> Option { for fragment in lower.split(|ch: char| ch.is_whitespace() || [';', '|', '&'].contains(&ch)) { let token = normalize_shell_token(fragment); @@ -2142,14 +2106,15 @@ mod tests { } #[test] - fn concrete_hazards_inside_inline_source_remain_visible() { + fn inline_source_is_not_reinterpreted_as_shell_commands() { assert!( analyze_command_risks("python3 -c \"open('/etc/shadow').read()\"") .contains(&CommandRisk::SensitivePathAccess("/etc/".into())) ); assert!( - analyze_command_risks("node -e 'require(\"child_process\").exec(\"dd\")'") - .contains(&CommandRisk::DestructiveCommand("dd".into())) + !analyze_command_risks("node -e 'require(\"child_process\").exec(\"dd\")'") + .iter() + .any(|risk| matches!(risk, CommandRisk::DestructiveCommand(_))) ); } } diff --git a/crates/astra-tools/src/shell_ops.rs b/crates/astra-tools/src/shell_ops.rs index 09f19bfe1f..3910d2c5df 100644 --- a/crates/astra-tools/src/shell_ops.rs +++ b/crates/astra-tools/src/shell_ops.rs @@ -560,8 +560,8 @@ struct SearchIgnoreRule { /// boundary because command parsing cannot enumerate arbitrary writers. /// /// Layering: -/// 1. Local substring/heuristic rules (destructive `rm`, pipe-to-shell, netcat, etc.). -/// 2. [`analyze_command_risks`] (tree-sitter + legacy): any reported risk **blocks** except +/// 1. Local rules for shell syntax with no ordinary executable (`rm -rf`, fork bombs, etc.). +/// 2. [`analyze_command_risks`] (tree-sitter command analysis): any reported risk **blocks** except /// [`CommandRisk::PathTraversal`] and [`CommandRisk::NetworkAccess`], which are allowed here /// so normal `cd ../..` and `curl`/`wget` workflows remain usable (network still subject to /// sandbox/permissions elsewhere). All other sandbox risks (e.g. [`CommandRisk::Eval`], @@ -734,25 +734,10 @@ pub fn validate_execute_bash_command_in_workspace( } validate_bash_background_task_contract(cmd)?; let lower = cmd.to_ascii_lowercase(); - let blocked_substrings = [ - "mkfs", - "mkswap", - " wipefs", - " dd if=", - " dd of=", - "shutdown", - "reboot", - "poweroff", - "halt", - "telinit", - "kill -9", - "pkill", - "killall", - " :(){", - ":(){ :", - "fork bomb", - ]; - for pat in blocked_substrings { + // Fork bombs are shell syntax rather than an executable invocation, so + // they remain an explicit syntax check. Executable risks are owned by the + // AST analyzer below and must not scan heredoc/interpreter source text. + for pat in [" :(){", ":(){ :"] { if lower.contains(pat) { return Err(format!( "Error: bash command matches a blocked destructive pattern ({pat:?})" @@ -6152,6 +6137,23 @@ printf 'probe.txt:1:needle\n' fn validate_execute_bash_blocks_top5_security_risks() { for command in [ "dd if=/dev/zero of=/dev/sda", + "exec -a alias dd if=/dev/zero of=/dev/sda", + "sudo -D /tmp dd if=/dev/zero of=/dev/sda", + "doas -a passwd dd if=/dev/zero of=/dev/sda", + "pkexec --user root dd if=/dev/zero of=/dev/sda", + "env -u HOME dd if=/dev/zero of=/dev/sda", + "bash --norc -c 'dd if=/dev/zero of=/dev/sda'", + "env MODE=secure bash --rcfile /tmp/bashrc -c 'wipefs -a /dev/sdb'", + "busybox dd if=/dev/zero of=/dev/sda", + "timeout 5 dd if=/dev/zero of=/dev/sda", + "sudo timeout -s KILL 5 dd if=/dev/zero of=/dev/sda", + "nice -n 5 wipefs -a /dev/sdb", + "ionice -c 2 dd if=/dev/zero of=/dev/sda", + "setsid wipefs -a /dev/sdb", + "setsid env MODE=secure timeout 5 sudo dd if=/dev/zero of=/dev/sda", + "printf data | xargs dd if=/dev/zero of=/dev/sda", + "find . -exec wipefs -a /dev/sdb {} \\;", + "tool=dd; \"$tool\" if=/dev/zero of=/dev/sda", "cat ~/.ssh/id_rsa", "echo data > ../outside.txt", "eval \"echo hi\"", @@ -6164,10 +6166,38 @@ printf 'probe.txt:1:needle\n' } } + #[test] + fn validate_execute_bash_allows_destructive_words_inside_heredoc_data() { + let command = "python3 <<'PY'\ndd = {'chart': 'bar'}\nprint(dd)\nPY"; + assert!( + validate_execute_bash_command(command).is_ok(), + "Python identifiers in heredoc data must not be treated as shell commands" + ); + } + #[test] fn validate_execute_bash_allows_typical_build_commands() { assert!(validate_execute_bash_command("cargo test -p foo --quiet").is_ok()); assert!(validate_execute_bash_command("echo hello && ls").is_ok()); + assert!(validate_execute_bash_command("timeout 5 printf '%s\\n' dd").is_ok()); + assert!(validate_execute_bash_command("nice -n 5 printf '%s\\n' dd").is_ok()); + assert!(validate_execute_bash_command("ionice -c 2 printf '%s\\n' dd").is_ok()); + assert!(validate_execute_bash_command("setsid printf '%s\\n' dd").is_ok()); + } + + #[test] + fn validate_execute_bash_rejects_ambiguous_dispatcher_options() { + for command in [ + "timeout --future-option 5 dd if=/dev/zero of=/dev/sda", + "nice --future-option dd if=/dev/zero of=/dev/sda", + "ionice --future-option dd if=/dev/zero of=/dev/sda", + "setsid --future-option dd if=/dev/zero of=/dev/sda", + ] { + assert!( + validate_execute_bash_command(command).is_err(), + "unknown dispatcher option arity must fail closed: {command}" + ); + } } #[test]