Harden the terminal tool's permission system to reject commands containing shell substitutions and interpolations (`$VAR`, `${VAR}`, `$(…)`, backticks, `$((…))`, `<(…)`, `>(…)`) before they reach terminal creation. ## Changes ### Shell command parser (`shell_command_parser`) - Added structured terminal command-prefix extraction with env-var prefix support - Added parser-backed validation that classifies commands as Safe/Unsafe/Unknown - Extended normalized command extraction to include scalar env-var assignments in order - Preserved quoted assignment values when they contain whitespace or special characters ### Pattern extraction (`agent/pattern_extraction`) - Updated terminal pattern extraction to use structured parser output - Included env-var prefixes in generated allow patterns - Normalized regex token boundaries to `\s+` while preserving display whitespace ### Tool permissions (`agent/tool_permissions`) - Added invalid-terminal-command rejection for forbidden substitutions/interpolations - Added unconditional allow-all bypass (global default Allow, or terminal-specific Allow with empty patterns) - Preserved hardcoded denial precedence over allow-all ### Terminal tool (`agent/tools/terminal_tool`) - Updated tool description and input schema to explicitly prohibit shell substitutions - Added comprehensive SEC-264 regression test suite (20 new tests) covering: - All forbidden constructs (`${HOME}`, `$1`, `$?`, `$$`, `$@`, `$(whoami)`, backticks, `$((1+1))`, `<(ls)`, `>(cat)`, env-prefix variants, multiline, nested) - Allow-all exception paths (global and terminal-specific) - Hardcoded-denial precedence - Env-prefix permission flow (matching, value mismatch rejection, multiple assignments, quoted whitespace) Closes SEC-264 Release Notes: - Terminal tool permissions regexes can now match environment variables (e.g. `FOO=bar cmd arg1 arg2`) - If terminal tool permissions have active permissions regexes running on them, then bare interpolations like `$FOO` are disallowed for security, since regexes wouldn't be able to match on them.
Showing the changes to crates/shell_command_parser/src/shell_command_parser.rs (1 of 5 files). Show all
Unified · Split
| @@ -1,8 +1,25 @@ | |||
| 1 | use brush_parser::ast; | 1 | use brush_parser::ast; |
| 2 | use brush_parser::ast::SourceLocation; | ||
| 2 | use brush_parser::word::WordPiece; | 3 | use brush_parser::word::WordPiece; |
| 3 | use brush_parser::{Parser, ParserOptions, SourceInfo}; | 4 | use brush_parser::{Parser, ParserOptions, SourceInfo}; |
| 4 | use std::io::BufReader; | 5 | use std::io::BufReader; |
| 5 | 6 | ||
| 7 | #[derive(Debug, Clone, PartialEq, Eq)] | ||
| 8 | pub struct TerminalCommandPrefix { | ||
| 9 | pub normalized: String, | ||
| 10 | pub display: String, | ||
| 11 | pub tokens: Vec<String>, | ||
| 12 | pub command: String, | ||
| 13 | pub subcommand: Option<String>, | ||
| 14 | } | ||
| 15 | |||
| 16 | #[derive(Debug, Clone, PartialEq, Eq)] | ||
| 17 | pub enum TerminalCommandValidation { | ||
| 18 | Safe, | ||
| 19 | Unsafe, | ||
| 20 | Unsupported, | ||
| 21 | } | ||
| 22 | |||
| 6 | pub fn extract_commands(command: &str) -> Option<Vec<String>> { | 23 | pub fn extract_commands(command: &str) -> Option<Vec<String>> { |
| 7 | let reader = BufReader::new(command.as_bytes()); | 24 | let reader = BufReader::new(command.as_bytes()); |
| 8 | let options = ParserOptions::default(); | 25 | let options = ParserOptions::default(); |
| @@ -17,6 +34,444 @@ pub fn extract_commands(command: &str) -> Option<Vec<String>> { · 8 unmodified lines | |||
| 17 | Some(commands) | 34 | Some(commands) |
| 18 | } | 35 | } |
| 19 | 36 | ||
| 37 | pub fn extract_terminal_command_prefix(command: &str) -> Option<TerminalCommandPrefix> { | ||
| 38 | let reader = BufReader::new(command.as_bytes()); | ||
| 39 | let options = ParserOptions::default(); | ||
| 40 | let source_info = SourceInfo::default(); | ||
| 41 | let mut parser = Parser::new(reader, &options, &source_info); | ||
| 42 | |||
| 43 | let program = parser.parse_program().ok()?; | ||
| 44 | let simple_command = first_simple_command(&program)?; | ||
| 45 | |||
| 46 | let mut normalized_tokens = Vec::new(); | ||
| 47 | let mut display_start = None; | ||
| 48 | let mut display_end = None; | ||
| 49 | |||
| 50 | if let Some(prefix) = &simple_command.prefix { | ||
| 51 | for item in &prefix.0 { | ||
| 52 | if let ast::CommandPrefixOrSuffixItem::AssignmentWord(assignment, word) = item { | ||
| 53 | match normalize_assignment_for_command_prefix(assignment, word)? { | ||
| 54 | NormalizedAssignment::Included(normalized_assignment) => { | ||
| 55 | normalized_tokens.push(normalized_assignment); | ||
| 56 | update_display_bounds(&mut display_start, &mut display_end, word); | ||
| 57 | } | ||
| 58 | NormalizedAssignment::Skipped => {} | ||
| 59 | } | ||
| 60 | } | ||
| 61 | } | ||
| 62 | } | ||
| 63 | |||
| 64 | let command_word = simple_command.word_or_name.as_ref()?; | ||
| 65 | let command_name = normalize_word(command_word)?; | ||
| 66 | normalized_tokens.push(command_name.clone()); | ||
| 67 | update_display_bounds(&mut display_start, &mut display_end, command_word); | ||
| 68 | |||
| 69 | let mut subcommand = None; | ||
| 70 | if let Some(suffix) = &simple_command.suffix { | ||
| 71 | for item in &suffix.0 { | ||
| 72 | match item { | ||
| 73 | ast::CommandPrefixOrSuffixItem::IoRedirect(_) => continue, | ||
| 74 | ast::CommandPrefixOrSuffixItem::Word(word) => { | ||
| 75 | let normalized_word = normalize_word(word)?; | ||
| 76 | if !normalized_word.starts_with('-') { | ||
| 77 | subcommand = Some(normalized_word.clone()); | ||
| 78 | normalized_tokens.push(normalized_word); | ||
| 79 | update_display_bounds(&mut display_start, &mut display_end, word); | ||
| 80 | } | ||
| 81 | break; | ||
| 82 | } | ||
| 83 | _ => break, | ||
| 84 | } | ||
| 85 | } | ||
| 86 | } | ||
| 87 | |||
| 88 | let start = display_start?; | ||
| 89 | let end = display_end?; | ||
| 90 | let display = command.get(start..end)?.to_string(); | ||
| 91 | |||
| 92 | Some(TerminalCommandPrefix { | ||
| 93 | normalized: normalized_tokens.join(" "), | ||
| 94 | display, | ||
| 95 | tokens: normalized_tokens, | ||
| 96 | command: command_name, | ||
| 97 | subcommand, | ||
| 98 | }) | ||
| 99 | } | ||
| 100 | |||
| 101 | pub fn validate_terminal_command(command: &str) -> TerminalCommandValidation { | ||
| 102 | let reader = BufReader::new(command.as_bytes()); | ||
| 103 | let options = ParserOptions::default(); | ||
| 104 | let source_info = SourceInfo::default(); | ||
| 105 | let mut parser = Parser::new(reader, &options, &source_info); | ||
| 106 | |||
| 107 | let program = match parser.parse_program() { | ||
| 108 | Ok(program) => program, | ||
| 109 | Err(_) => return TerminalCommandValidation::Unsupported, | ||
| 110 | }; | ||
| 111 | |||
| 112 | match program_validation(&program) { | ||
| 113 | TerminalProgramValidation::Safe => TerminalCommandValidation::Safe, | ||
| 114 | TerminalProgramValidation::Unsafe => TerminalCommandValidation::Unsafe, | ||
| 115 | TerminalProgramValidation::Unsupported => TerminalCommandValidation::Unsupported, | ||
| 116 | } | ||
| 117 | } | ||
| 118 | |||
| 119 | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| 120 | enum TerminalProgramValidation { | ||
| 121 | Safe, | ||
| 122 | Unsafe, | ||
| 123 | Unsupported, | ||
| 124 | } | ||
| 125 | |||
| 126 | fn first_simple_command(program: &ast::Program) -> Option<&ast::SimpleCommand> { | ||
| 127 | let complete_command = program.complete_commands.first()?; | ||
| 128 | let compound_list_item = complete_command.0.first()?; | ||
| 129 | let command = compound_list_item.0.first.seq.first()?; | ||
| 130 | |||
| 131 | match command { | ||
| 132 | ast::Command::Simple(simple_command) => Some(simple_command), | ||
| 133 | _ => None, | ||
| 134 | } | ||
| 135 | } | ||
| 136 | |||
| 137 | fn update_display_bounds(start: &mut Option<usize>, end: &mut Option<usize>, word: &ast::Word) { | ||
| 138 | if let Some(location) = word.location() { | ||
| 139 | let word_start = location.start.index; | ||
| 140 | let word_end = location.end.index; | ||
| 141 | *start = Some(start.map_or(word_start, |current| current.min(word_start))); | ||
| 142 | *end = Some(end.map_or(word_end, |current| current.max(word_end))); | ||
| 143 | } | ||
| 144 | } | ||
| 145 | |||
| 146 | enum NormalizedAssignment { | ||
| 147 | Included(String), | ||
| 148 | Skipped, | ||
| 149 | } | ||
| 150 | |||
| 151 | fn normalize_assignment_for_command_prefix( | ||
| 152 | assignment: &ast::Assignment, | ||
| 153 | word: &ast::Word, | ||
| 154 | ) -> Option<NormalizedAssignment> { | ||
| 155 | let operator = if assignment.append { "+=" } else { "=" }; | ||
| 156 | let assignment_prefix = format!("{}{}", assignment.name, operator); | ||
| 157 | |||
| 158 | match &assignment.value { | ||
| 159 | ast::AssignmentValue::Scalar(value) => { | ||
| 160 | let normalized_value = normalize_word(value)?; | ||
| 161 | let raw_value = word.value.strip_prefix(&assignment_prefix)?; | ||
| 162 | let rendered_value = if shell_value_requires_quoting(&normalized_value) { | ||
| 163 | raw_value.to_string() | ||
| 164 | } else { | ||
| 165 | normalized_value | ||
| 166 | }; | ||
| 167 | |||
| 168 | Some(NormalizedAssignment::Included(format!( | ||
| 169 | "{assignment_prefix}{rendered_value}" | ||
| 170 | ))) | ||
| 171 | } | ||
| 172 | ast::AssignmentValue::Array(_) => Some(NormalizedAssignment::Skipped), | ||
| 173 | } | ||
| 174 | } | ||
| 175 | |||
| 176 | fn shell_value_requires_quoting(value: &str) -> bool { | ||
| 177 | value.chars().any(|character| { | ||
| 178 | character.is_whitespace() | ||
| 179 | || !matches!( | ||
| 180 | character, | ||
| 181 | 'a'..='z' | ||
| 182 | | 'A'..='Z' | ||
| 183 | | '0'..='9' | ||
| 184 | | '_' | ||
| 185 | | '@' | ||
| 186 | | '%' | ||
| 187 | | '+' | ||
| 188 | | '=' | ||
| 189 | | ':' | ||
| 190 | | ',' | ||
| 191 | | '.' | ||
| 192 | | '/' | ||
| 193 | | '-' | ||
| 194 | ) | ||
| 195 | }) | ||
| 196 | } | ||
| 197 | |||
| 198 | fn program_validation(program: &ast::Program) -> TerminalProgramValidation { | ||
| 199 | combine_validations( | ||
| 200 | program | ||
| 201 | .complete_commands | ||
| 202 | .iter() | ||
| 203 | .map(compound_list_validation), | ||
| 204 | ) | ||
| 205 | } | ||
| 206 | |||
| 207 | fn compound_list_validation(compound_list: &ast::CompoundList) -> TerminalProgramValidation { | ||
| 208 | combine_validations( | ||
| 209 | compound_list | ||
| 210 | .0 | ||
| 211 | .iter() | ||
| 212 | .map(|item| and_or_list_validation(&item.0)), | ||
| 213 | ) | ||
| 214 | } | ||
| 215 | |||
| 216 | fn and_or_list_validation(and_or_list: &ast::AndOrList) -> TerminalProgramValidation { | ||
| 217 | combine_validations( | ||
| 218 | std::iter::once(pipeline_validation(&and_or_list.first)).chain( | ||
| 219 | and_or_list.additional.iter().map(|and_or| match and_or { | ||
| 220 | ast::AndOr::And(pipeline) | ast::AndOr::Or(pipeline) => { | ||
| 221 | pipeline_validation(pipeline) | ||
| 222 | } | ||
| 223 | }), | ||
| 224 | ), | ||
| 225 | ) | ||
| 226 | } | ||
| 227 | |||
| 228 | fn pipeline_validation(pipeline: &ast::Pipeline) -> TerminalProgramValidation { | ||
| 229 | combine_validations(pipeline.seq.iter().map(command_validation)) | ||
| 230 | } | ||
| 231 | |||
| 232 | fn command_validation(command: &ast::Command) -> TerminalProgramValidation { | ||
| 233 | match command { | ||
| 234 | ast::Command::Simple(simple_command) => simple_command_validation(simple_command), | ||
| 235 | ast::Command::Compound(compound_command, redirect_list) => combine_validations( | ||
| 236 | std::iter::once(compound_command_validation(compound_command)) | ||
| 237 | .chain(redirect_list.iter().map(redirect_list_validation)), | ||
| 238 | ), | ||
| 239 | ast::Command::Function(function_definition) => { | ||
| 240 | function_body_validation(&function_definition.body) | ||
| 241 | } | ||
| 242 | ast::Command::ExtendedTest(test_expr) => extended_test_expr_validation(test_expr), | ||
| 243 | } | ||
| 244 | } | ||
| 245 | |||
| 246 | fn simple_command_validation(simple_command: &ast::SimpleCommand) -> TerminalProgramValidation { | ||
| 247 | combine_validations( | ||
| 248 | simple_command | ||
| 249 | .prefix | ||
| 250 | .iter() | ||
| 251 | .map(command_prefix_validation) | ||
| 252 | .chain(simple_command.word_or_name.iter().map(word_validation)) | ||
| 253 | .chain(simple_command.suffix.iter().map(command_suffix_validation)), | ||
| 254 | ) | ||
| 255 | } | ||
| 256 | |||
| 257 | fn command_prefix_validation(prefix: &ast::CommandPrefix) -> TerminalProgramValidation { | ||
| 258 | combine_validations(prefix.0.iter().map(prefix_or_suffix_item_validation)) | ||
| 259 | } | ||
| 260 | |||
| 261 | fn command_suffix_validation(suffix: &ast::CommandSuffix) -> TerminalProgramValidation { | ||
| 262 | combine_validations(suffix.0.iter().map(prefix_or_suffix_item_validation)) | ||
| 263 | } | ||
| 264 | |||
| 265 | fn prefix_or_suffix_item_validation( | ||
| 266 | item: &ast::CommandPrefixOrSuffixItem, | ||
| 267 | ) -> TerminalProgramValidation { | ||
| 268 | match item { | ||
| 269 | ast::CommandPrefixOrSuffixItem::IoRedirect(redirect) => io_redirect_validation(redirect), | ||
| 270 | ast::CommandPrefixOrSuffixItem::Word(word) => word_validation(word), | ||
| 271 | ast::CommandPrefixOrSuffixItem::AssignmentWord(assignment, word) => { | ||
| 272 | combine_validations([assignment_validation(assignment), word_validation(word)]) | ||
| 273 | } | ||
| 274 | ast::CommandPrefixOrSuffixItem::ProcessSubstitution(_, _) => { | ||
| 275 | TerminalProgramValidation::Unsafe | ||
| 276 | } | ||
| 277 | } | ||
| 278 | } | ||
| 279 | |||
| 280 | fn io_redirect_validation(redirect: &ast::IoRedirect) -> TerminalProgramValidation { | ||
| 281 | match redirect { | ||
| 282 | ast::IoRedirect::File(_, _, target) => match target { | ||
| 283 | ast::IoFileRedirectTarget::Filename(word) => word_validation(word), | ||
| 284 | ast::IoFileRedirectTarget::ProcessSubstitution(_, _) => { | ||
| 285 | TerminalProgramValidation::Unsafe | ||
| 286 | } | ||
| 287 | _ => TerminalProgramValidation::Safe, | ||
| 288 | }, | ||
| 289 | ast::IoRedirect::HereDocument(_, here_doc) => { | ||
| 290 | if here_doc.requires_expansion { | ||
| 291 | word_validation(&here_doc.doc) | ||
| 292 | } else { | ||
| 293 | TerminalProgramValidation::Safe | ||
| 294 | } | ||
| 295 | } | ||
| 296 | ast::IoRedirect::HereString(_, word) | ast::IoRedirect::OutputAndError(word, _) => { | ||
| 297 | word_validation(word) | ||
| 298 | } | ||
| 299 | } | ||
| 300 | } | ||
| 301 | |||
| 302 | fn assignment_validation(assignment: &ast::Assignment) -> TerminalProgramValidation { | ||
| 303 | match &assignment.value { | ||
| 304 | ast::AssignmentValue::Scalar(word) => word_validation(word), | ||
| 305 | ast::AssignmentValue::Array(words) => { | ||
| 306 | combine_validations(words.iter().flat_map(|(key, value)| { | ||
| 307 | key.iter() | ||
| 308 | .map(word_validation) | ||
| 309 | .chain(std::iter::once(word_validation(value))) | ||
| 310 | })) | ||
| 311 | } | ||
| 312 | } | ||
| 313 | } | ||
| 314 | |||
| 315 | fn word_validation(word: &ast::Word) -> TerminalProgramValidation { | ||
| 316 | let options = ParserOptions::default(); | ||
| 317 | let pieces = match brush_parser::word::parse(&word.value, &options) { | ||
| 318 | Ok(pieces) => pieces, | ||
| 319 | Err(_) => return TerminalProgramValidation::Unsupported, | ||
| 320 | }; | ||
| 321 | |||
| 322 | combine_validations( | ||
| 323 | pieces | ||
| 324 | .iter() | ||
| 325 | .map(|piece_with_source| word_piece_validation(&piece_with_source.piece)), | ||
| 326 | ) | ||
| 327 | } | ||
| 328 | |||
| 329 | fn word_piece_validation(piece: &WordPiece) -> TerminalProgramValidation { | ||
| 330 | match piece { | ||
| 331 | WordPiece::Text(_) | ||
| 332 | | WordPiece::SingleQuotedText(_) | ||
| 333 | | WordPiece::AnsiCQuotedText(_) | ||
| 334 | | WordPiece::EscapeSequence(_) | ||
| 335 | | WordPiece::TildePrefix(_) => TerminalProgramValidation::Safe, | ||
| 336 | WordPiece::DoubleQuotedSequence(pieces) | ||
| 337 | | WordPiece::GettextDoubleQuotedSequence(pieces) => combine_validations( | ||
| 338 | pieces | ||
| 339 | .iter() | ||
| 340 | .map(|inner| word_piece_validation(&inner.piece)), | ||
| 341 | ), | ||
| 342 | WordPiece::ParameterExpansion(_) | WordPiece::ArithmeticExpression(_) => { | ||
| 343 | TerminalProgramValidation::Unsafe | ||
| 344 | } | ||
| 345 | WordPiece::CommandSubstitution(command) | ||
| 346 | | WordPiece::BackquotedCommandSubstitution(command) => { | ||
| 347 | let reader = BufReader::new(command.as_bytes()); | ||
| 348 | let options = ParserOptions::default(); | ||
| 349 | let source_info = SourceInfo::default(); | ||
| 350 | let mut parser = Parser::new(reader, &options, &source_info); | ||
| 351 | |||
| 352 | match parser.parse_program() { | ||
| 353 | Ok(_) => TerminalProgramValidation::Unsafe, | ||
| 354 | Err(_) => TerminalProgramValidation::Unsupported, | ||
| 355 | } | ||
| 356 | } | ||
| 357 | } | ||
| 358 | } | ||
| 359 | |||
| 360 | fn compound_command_validation( | ||
| 361 | compound_command: &ast::CompoundCommand, | ||
| 362 | ) -> TerminalProgramValidation { | ||
| 363 | match compound_command { | ||
| 364 | ast::CompoundCommand::BraceGroup(brace_group) => { | ||
| 365 | compound_list_validation(&brace_group.list) | ||
| 366 | } | ||
| 367 | ast::CompoundCommand::Subshell(subshell) => compound_list_validation(&subshell.list), | ||
| 368 | ast::CompoundCommand::ForClause(for_clause) => combine_validations( | ||
| 369 | for_clause | ||
| 370 | .values | ||
| 371 | .iter() | ||
| 372 | .flat_map(|values| values.iter().map(word_validation)) | ||
| 373 | .chain(std::iter::once(do_group_validation(&for_clause.body))), | ||
| 374 | ), | ||
| 375 | ast::CompoundCommand::CaseClause(case_clause) => combine_validations( | ||
| 376 | std::iter::once(word_validation(&case_clause.value)) | ||
| 377 | .chain( | ||
| 378 | case_clause | ||
| 379 | .cases | ||
| 380 | .iter() | ||
| 381 | .flat_map(|item| item.cmd.iter().map(compound_list_validation)), | ||
| 382 | ) | ||
| 383 | .chain( | ||
| 384 | case_clause | ||
| 385 | .cases | ||
| 386 | .iter() | ||
| 387 | .flat_map(|item| item.patterns.iter().map(word_validation)), | ||
| 388 | ), | ||
| 389 | ), | ||
| 390 | ast::CompoundCommand::IfClause(if_clause) => combine_validations( | ||
| 391 | std::iter::once(compound_list_validation(&if_clause.condition)) | ||
| 392 | .chain(std::iter::once(compound_list_validation(&if_clause.then))) | ||
| 393 | .chain(if_clause.elses.iter().flat_map(|elses| { | ||
| 394 | elses.iter().flat_map(|else_item| { | ||
| 395 | else_item | ||
| 396 | .condition | ||
| 397 | .iter() | ||
| 398 | .map(compound_list_validation) | ||
| 399 | .chain(std::iter::once(compound_list_validation(&else_item.body))) | ||
| 400 | }) | ||
| 401 | })), | ||
| 402 | ), | ||
| 403 | ast::CompoundCommand::WhileClause(while_clause) | ||
| 404 | | ast::CompoundCommand::UntilClause(while_clause) => combine_validations([ | ||
| 405 | compound_list_validation(&while_clause.0), | ||
| 406 | do_group_validation(&while_clause.1), | ||
| 407 | ]), | ||
| 408 | ast::CompoundCommand::ArithmeticForClause(_) => TerminalProgramValidation::Unsafe, | ||
| 409 | ast::CompoundCommand::Arithmetic(_) => TerminalProgramValidation::Unsafe, | ||
| 410 | } | ||
| 411 | } | ||
| 412 | |||
| 413 | fn do_group_validation(do_group: &ast::DoGroupCommand) -> TerminalProgramValidation { | ||
| 414 | compound_list_validation(&do_group.list) | ||
| 415 | } | ||
| 416 | |||
| 417 | fn function_body_validation(function_body: &ast::FunctionBody) -> TerminalProgramValidation { | ||
| 418 | combine_validations( | ||
| 419 | std::iter::once(compound_command_validation(&function_body.0)) | ||
| 420 | .chain(function_body.1.iter().map(redirect_list_validation)), | ||
| 421 | ) | ||
| 422 | } | ||
| 423 | |||
| 424 | fn redirect_list_validation(redirect_list: &ast::RedirectList) -> TerminalProgramValidation { | ||
| 425 | combine_validations(redirect_list.0.iter().map(io_redirect_validation)) | ||
| 426 | } | ||
| 427 | |||
| 428 | fn extended_test_expr_validation( | ||
| 429 | test_expr: &ast::ExtendedTestExprCommand, | ||
| 430 | ) -> TerminalProgramValidation { | ||
| 431 | extended_test_expr_inner_validation(&test_expr.expr) | ||
| 432 | } | ||
| 433 | |||
| 434 | fn extended_test_expr_inner_validation(expr: &ast::ExtendedTestExpr) -> TerminalProgramValidation { | ||
| 435 | match expr { | ||
| 436 | ast::ExtendedTestExpr::Not(inner) | ast::ExtendedTestExpr::Parenthesized(inner) => { | ||
| 437 | extended_test_expr_inner_validation(inner) | ||
| 438 | } | ||
| 439 | ast::ExtendedTestExpr::And(left, right) | ast::ExtendedTestExpr::Or(left, right) => { | ||
| 440 | combine_validations([ | ||
| 441 | extended_test_expr_inner_validation(left), | ||
| 442 | extended_test_expr_inner_validation(right), | ||
| 443 | ]) | ||
| 444 | } | ||
| 445 | ast::ExtendedTestExpr::UnaryTest(_, word) => word_validation(word), | ||
| 446 | ast::ExtendedTestExpr::BinaryTest(_, left, right) => { | ||
| 447 | combine_validations([word_validation(left), word_validation(right)]) | ||
| 448 | } | ||
| 449 | } | ||
| 450 | } | ||
| 451 | |||
| 452 | fn combine_validations( | ||
| 453 | validations: impl IntoIterator<Item = TerminalProgramValidation>, | ||
| 454 | ) -> TerminalProgramValidation { | ||
| 455 | let mut saw_unsafe = false; | ||
| 456 | let mut saw_unsupported = false; | ||
| 457 | |||
| 458 | for validation in validations { | ||
| 459 | match validation { | ||
| 460 | TerminalProgramValidation::Unsupported => saw_unsupported = true, | ||
| 461 | TerminalProgramValidation::Unsafe => saw_unsafe = true, | ||
| 462 | TerminalProgramValidation::Safe => {} | ||
| 463 | } | ||
| 464 | } | ||
| 465 | |||
| 466 | if saw_unsafe { | ||
| 467 | TerminalProgramValidation::Unsafe | ||
| 468 | } else if saw_unsupported { | ||
| 469 | TerminalProgramValidation::Unsupported | ||
| 470 | } else { | ||
| 471 | TerminalProgramValidation::Safe | ||
| 472 | } | ||
| 473 | } | ||
| 474 | |||
| 20 | fn extract_commands_from_program(program: &ast::Program, commands: &mut Vec<String>) -> Option<()> { | 475 | fn extract_commands_from_program(program: &ast::Program, commands: &mut Vec<String>) -> Option<()> { |
| 21 | for complete_command in &program.complete_commands { | 476 | for complete_command in &program.complete_commands { |
| 22 | extract_commands_from_compound_list(complete_command, commands)?; | 477 | extract_commands_from_compound_list(complete_command, commands)?; |
| @@ -117,12 +572,26 @@ fn extract_commands_from_simple_command( · 94 unmodified lines | |||
| 117 | 572 | ||
| 118 | if let Some(prefix) = &simple_command.prefix { | 573 | if let Some(prefix) = &simple_command.prefix { |
| 119 | for item in &prefix.0 { | 574 | for item in &prefix.0 { |
| 120 | if let ast::CommandPrefixOrSuffixItem::IoRedirect(redirect) = item { | 575 | match item { |
| 121 | match normalize_io_redirect(redirect) { | 576 | ast::CommandPrefixOrSuffixItem::IoRedirect(redirect) => { |
| 122 | Some(RedirectNormalization::Normalized(s)) => redirects.push(s), | 577 | match normalize_io_redirect(redirect) { |
| 123 | Some(RedirectNormalization::Skip) => {} | 578 | Some(RedirectNormalization::Normalized(s)) => redirects.push(s), |
| 124 | None => return None, | 579 | Some(RedirectNormalization::Skip) => {} |
| 580 | None => return None, | ||
| 581 | } | ||
| 582 | } | ||
| 583 | ast::CommandPrefixOrSuffixItem::AssignmentWord(assignment, word) => { | ||
| 584 | match normalize_assignment_for_command_prefix(assignment, word)? { | ||
| 585 | NormalizedAssignment::Included(normalized_assignment) => { | ||
| 586 | words.push(normalized_assignment); | ||
| 587 | } | ||
| 588 | NormalizedAssignment::Skipped => {} | ||
| 589 | } | ||
| 590 | } | ||
| 591 | ast::CommandPrefixOrSuffixItem::Word(word) => { | ||
| 592 | words.push(normalize_word(word)?); | ||
| 125 | } | 593 | } |
| 594 | ast::CommandPrefixOrSuffixItem::ProcessSubstitution(_, _) => return None, | ||
| 126 | } | 595 | } |
| 127 | } | 596 | } |
| 128 | } | 597 | } |
| @@ -142,7 +611,15 @@ fn extract_commands_from_simple_command( · 13 unmodified lines | |||
| 142 | None => return None, | 611 | None => return None, |
| 143 | } | 612 | } |
| 144 | } | 613 | } |
| 145 | _ => {} | 614 | ast::CommandPrefixOrSuffixItem::AssignmentWord(assignment, word) => { |
| 615 | match normalize_assignment_for_command_prefix(assignment, word)? { | ||
| 616 | NormalizedAssignment::Included(normalized_assignment) => { | ||
| 617 | words.push(normalized_assignment); | ||
| 618 | } | ||
| 619 | NormalizedAssignment::Skipped => {} | ||
| 620 | } | ||
| 621 | } | ||
| 622 | ast::CommandPrefixOrSuffixItem::ProcessSubstitution(_, _) => {} | ||
| 146 | } | 623 | } |
| 147 | } | 624 | } |
| 148 | } | 625 | } |
| @@ -1061,4 +1538,220 @@ mod tests { · 912 unmodified lines | |||
| 1061 | let commands = extract_commands("cmd > /tmp/out 2>/dev/null").expect("parse failed"); | 1538 | let commands = extract_commands("cmd > /tmp/out 2>/dev/null").expect("parse failed"); |
| 1062 | assert_eq!(commands, vec!["cmd", "> /tmp/out"]); | 1539 | assert_eq!(commands, vec!["cmd", "> /tmp/out"]); |
| 1063 | } | 1540 | } |
| 1541 | |||
| 1542 | #[test] | ||
| 1543 | fn test_scalar_env_var_prefix_included_in_extracted_command() { | ||
| 1544 | let commands = extract_commands("PAGER=blah git status").expect("parse failed"); | ||
| 1545 | assert_eq!(commands, vec!["PAGER=blah git status"]); | ||
| 1546 | } | ||
| 1547 | |||
| 1548 | #[test] | ||
| 1549 | fn test_multiple_scalar_assignments_preserved_in_order() { | ||
| 1550 | let commands = extract_commands("A=1 B=2 git log").expect("parse failed"); | ||
| 1551 | assert_eq!(commands, vec!["A=1 B=2 git log"]); | ||
| 1552 | } | ||
| 1553 | |||
| 1554 | #[test] | ||
| 1555 | fn test_assignment_quoting_dropped_when_safe() { | ||
| 1556 | let commands = extract_commands("PAGER='curl' git log").expect("parse failed"); | ||
| 1557 | assert_eq!(commands, vec!["PAGER=curl git log"]); | ||
| 1558 | } | ||
| 1559 | |||
| 1560 | #[test] | ||
| 1561 | fn test_assignment_quoting_preserved_for_whitespace() { | ||
| 1562 | let commands = extract_commands("PAGER='less -R' git log").expect("parse failed"); | ||
| 1563 | assert_eq!(commands, vec!["PAGER='less -R' git log"]); | ||
| 1564 | } | ||
| 1565 | |||
| 1566 | #[test] | ||
| 1567 | fn test_assignment_quoting_preserved_for_semicolon() { | ||
| 1568 | let commands = extract_commands("PAGER='a;b' git log").expect("parse failed"); | ||
| 1569 | assert_eq!(commands, vec!["PAGER='a;b' git log"]); | ||
| 1570 | } | ||
| 1571 | |||
| 1572 | #[test] | ||
| 1573 | fn test_array_assignments_ignored_for_prefix_matching_output() { | ||
| 1574 | let commands = extract_commands("FOO=(a b) git status").expect("parse failed"); | ||
| 1575 | assert_eq!(commands, vec!["git status"]); | ||
| 1576 | } | ||
| 1577 | |||
| 1578 | #[test] | ||
| 1579 | fn test_extract_terminal_command_prefix_includes_env_var_prefix_and_subcommand() { | ||
| 1580 | let prefix = extract_terminal_command_prefix("PAGER=blah git log --oneline") | ||
| 1581 | .expect("expected terminal command prefix"); | ||
| 1582 | |||
| 1583 | assert_eq!( | ||
| 1584 | prefix, | ||
| 1585 | TerminalCommandPrefix { | ||
| 1586 | normalized: "PAGER=blah git log".to_string(), | ||
| 1587 | display: "PAGER=blah git log".to_string(), | ||
| 1588 | tokens: vec![ | ||
| 1589 | "PAGER=blah".to_string(), | ||
| 1590 | "git".to_string(), | ||
| 1591 | "log".to_string(), | ||
| 1592 | ], | ||
| 1593 | command: "git".to_string(), | ||
| 1594 | subcommand: Some("log".to_string()), | ||
| 1595 | } | ||
| 1596 | ); | ||
| 1597 | } | ||
| 1598 | |||
| 1599 | #[test] | ||
| 1600 | fn test_extract_terminal_command_prefix_preserves_required_assignment_quotes_in_display_and_normalized() | ||
| 1601 | { | ||
| 1602 | let prefix = extract_terminal_command_prefix("PAGER='less -R' git log") | ||
| 1603 | .expect("expected terminal command prefix"); | ||
| 1604 | |||
| 1605 | assert_eq!( | ||
| 1606 | prefix, | ||
| 1607 | TerminalCommandPrefix { | ||
| 1608 | normalized: "PAGER='less -R' git log".to_string(), | ||
| 1609 | display: "PAGER='less -R' git log".to_string(), | ||
| 1610 | tokens: vec![ | ||
| 1611 | "PAGER='less -R'".to_string(), | ||
| 1612 | "git".to_string(), | ||
| 1613 | "log".to_string(), | ||
| 1614 | ], | ||
| 1615 | command: "git".to_string(), | ||
| 1616 | subcommand: Some("log".to_string()), | ||
| 1617 | } | ||
| 1618 | ); | ||
| 1619 | } | ||
| 1620 | |||
| 1621 | #[test] | ||
| 1622 | fn test_extract_terminal_command_prefix_skips_redirects_before_subcommand() { | ||
| 1623 | let prefix = extract_terminal_command_prefix("git 2>/dev/null log --oneline") | ||
| 1624 | .expect("expected terminal command prefix"); | ||
| 1625 | |||
| 1626 | assert_eq!( | ||
| 1627 | prefix, | ||
| 1628 | TerminalCommandPrefix { | ||
| 1629 | normalized: "git log".to_string(), | ||
| 1630 | display: "git 2>/dev/null log".to_string(), | ||
| 1631 | tokens: vec!["git".to_string(), "log".to_string()], | ||
| 1632 | command: "git".to_string(), | ||
| 1633 | subcommand: Some("log".to_string()), | ||
| 1634 | } | ||
| 1635 | ); | ||
| 1636 | } | ||
| 1637 | |||
| 1638 | #[test] | ||
| 1639 | fn test_validate_terminal_command_rejects_parameter_expansion() { | ||
| 1640 | assert_eq!( | ||
| 1641 | validate_terminal_command("echo $HOME"), | ||
| 1642 | TerminalCommandValidation::Unsafe | ||
| 1643 | ); | ||
| 1644 | } | ||
| 1645 | |||
| 1646 | #[test] | ||
| 1647 | fn test_validate_terminal_command_rejects_braced_parameter_expansion() { | ||
| 1648 | assert_eq!( | ||
| 1649 | validate_terminal_command("echo ${HOME}"), | ||
| 1650 | TerminalCommandValidation::Unsafe | ||
| 1651 | ); | ||
| 1652 | } | ||
| 1653 | |||
| 1654 | #[test] | ||
| 1655 | fn test_validate_terminal_command_rejects_special_parameters() { | ||
| 1656 | assert_eq!( | ||
| 1657 | validate_terminal_command("echo $?"), | ||
| 1658 | TerminalCommandValidation::Unsafe | ||
| 1659 | ); | ||
| 1660 | assert_eq!( | ||
| 1661 | validate_terminal_command("echo $$"), | ||
| 1662 | TerminalCommandValidation::Unsafe | ||
| 1663 | ); | ||
| 1664 | assert_eq!( | ||
| 1665 | validate_terminal_command("echo $@"), | ||
| 1666 | TerminalCommandValidation::Unsafe | ||
| 1667 | ); | ||
| 1668 | } | ||
| 1669 | |||
| 1670 | #[test] | ||
| 1671 | fn test_validate_terminal_command_rejects_command_substitution() { | ||
| 1672 | assert_eq!( | ||
| 1673 | validate_terminal_command("echo $(whoami)"), | ||
| 1674 | TerminalCommandValidation::Unsafe | ||
| 1675 | ); | ||
| 1676 | } | ||
| 1677 | |||
| 1678 | #[test] | ||
| 1679 | fn test_validate_terminal_command_rejects_backticks() { | ||
| 1680 | assert_eq!( | ||
| 1681 | validate_terminal_command("echo `whoami`"), | ||
| 1682 | TerminalCommandValidation::Unsafe | ||
| 1683 | ); | ||
| 1684 | } | ||
| 1685 | |||
| 1686 | #[test] | ||
| 1687 | fn test_validate_terminal_command_rejects_arithmetic_expansion() { | ||
| 1688 | assert_eq!( | ||
| 1689 | validate_terminal_command("echo $((1 + 1))"), | ||
| 1690 | TerminalCommandValidation::Unsafe | ||
| 1691 | ); | ||
| 1692 | } | ||
| 1693 | |||
| 1694 | #[test] | ||
| 1695 | fn test_validate_terminal_command_rejects_process_substitution() { | ||
| 1696 | assert_eq!( | ||
| 1697 | validate_terminal_command("cat <(ls)"), | ||
| 1698 | TerminalCommandValidation::Unsafe | ||
| 1699 | ); | ||
| 1700 | assert_eq!( | ||
| 1701 | validate_terminal_command("ls >(cat)"), | ||
| 1702 | TerminalCommandValidation::Unsafe | ||
| 1703 | ); | ||
| 1704 | } | ||
| 1705 | |||
| 1706 | #[test] | ||
| 1707 | fn test_validate_terminal_command_rejects_forbidden_constructs_in_env_var_assignments() { | ||
| 1708 | assert_eq!( | ||
| 1709 | validate_terminal_command("PAGER=$HOME git log"), | ||
| 1710 | TerminalCommandValidation::Unsafe | ||
| 1711 | ); | ||
| 1712 | assert_eq!( | ||
| 1713 | validate_terminal_command("PAGER=$(whoami) git log"), | ||
| 1714 | TerminalCommandValidation::Unsafe | ||
| 1715 | ); | ||
| 1716 | } | ||
| 1717 | |||
| 1718 | #[test] | ||
| 1719 | fn test_validate_terminal_command_returns_unsupported_for_parse_failure() { | ||
| 1720 | assert_eq!( | ||
| 1721 | validate_terminal_command("echo $(ls &&)"), | ||
| 1722 | TerminalCommandValidation::Unsupported | ||
| 1723 | ); | ||
| 1724 | } | ||
| 1725 | |||
| 1726 | #[test] | ||
| 1727 | fn test_validate_terminal_command_rejects_substitution_in_case_pattern() { | ||
| 1728 | assert_ne!( | ||
| 1729 | validate_terminal_command("case x in $(echo y)) echo z;; esac"), | ||
| 1730 | TerminalCommandValidation::Safe | ||
| 1731 | ); | ||
| 1732 | } | ||
| 1733 | |||
| 1734 | #[test] | ||
| 1735 | fn test_validate_terminal_command_safe_case_clause_without_substitutions() { | ||
| 1736 | assert_eq!( | ||
| 1737 | validate_terminal_command("case x in foo) echo hello;; esac"), | ||
| 1738 | TerminalCommandValidation::Safe | ||
| 1739 | ); | ||
| 1740 | } | ||
| 1741 | |||
| 1742 | #[test] | ||
| 1743 | fn test_validate_terminal_command_rejects_substitution_in_arithmetic_for_clause() { | ||
| 1744 | assert_ne!( | ||
| 1745 | validate_terminal_command("for ((i=$(echo 0); i<3; i++)); do echo hello; done"), | ||
| 1746 | TerminalCommandValidation::Safe | ||
| 1747 | ); | ||
| 1748 | } | ||
| 1749 | |||
| 1750 | #[test] | ||
| 1751 | fn test_validate_terminal_command_rejects_arithmetic_for_clause_unconditionally() { | ||
| 1752 | assert_eq!( | ||
| 1753 | validate_terminal_command("for ((i=0; i<3; i++)); do echo hello; done"), | ||
| 1754 | TerminalCommandValidation::Unsafe | ||
| 1755 | ); | ||
| 1756 | } | ||
| 1064 | } | 1757 | } |