diff --git a/crates/chat-cli/src/cli/chat/tools/execute/mod.rs b/crates/chat-cli/src/cli/chat/tools/execute/mod.rs index 2f97471761..f035f9e601 100644 --- a/crates/chat-cli/src/cli/chat/tools/execute/mod.rs +++ b/crates/chat-cli/src/cli/chat/tools/execute/mod.rs @@ -48,6 +48,11 @@ pub struct ExecuteCommand { impl ExecuteCommand { pub fn requires_acceptance(&self, allowed_commands: Option<&Vec>, allow_read_only: bool) -> bool { + // Always require acceptance for multi-line commands. + if self.command.contains("\n") || self.command.contains("\r") { + return true; + } + let default_arr = vec![]; let allowed_commands = allowed_commands.unwrap_or(&default_arr); @@ -64,7 +69,7 @@ impl ExecuteCommand { let Some(args) = shlex::split(&self.command) else { return true; }; - const DANGEROUS_PATTERNS: &[&str] = &["<(", "$(", "`", ">", "&&", "||", "&", ";"]; + const DANGEROUS_PATTERNS: &[&str] = &["<(", "$(", "`", ">", "&&", "||", "&", ";", "${", "\n", "\r", "IFS"]; if args .iter() @@ -106,6 +111,7 @@ impl ExecuteCommand { arg.contains("-exec") // includes -execdir || arg.contains("-delete") || arg.contains("-ok") // includes -okdir + || arg.contains("-fprint") // includes -fprint0 and -fprintf }) => { return true; @@ -114,7 +120,11 @@ impl ExecuteCommand { // Special casing for `grep`. -P flag for perl regexp has RCE issues, apparently // should not be supported within grep but is flagged as a possibility since this is perl // regexp. - if cmd == "grep" && cmd_args.iter().any(|arg| arg.contains("-P")) { + if cmd == "grep" + && cmd_args + .iter() + .any(|arg| arg.contains("-P") || arg.contains("--perl-regexp")) + { return true; } let is_cmd_read_only = READONLY_COMMANDS.contains(&cmd.as_str()); @@ -290,6 +300,14 @@ mod tests { ("cat <<< 'some string here' > myimportantfile", true), ("echo '\n#!/usr/bin/env bash\necho hello\n' > myscript.sh", true), ("cat < myimportantfile\nhello world\nEOF", true), + // newline checks + ("which ls\ntouch asdf", true), + ("which ls\rtouch asdf", true), + // $IFS check + ( + r#"IFS=';'; for cmd in "which ls;touch asdf"; do eval "$cmd"; done"#, + true, + ), // Safe piped commands ("find . -name '*.rs' | grep main", false), ("ls -la | grep .git", false), @@ -307,8 +325,12 @@ mod tests { true, ), ("find important-dir/ -name '*.txt'", false), + (r#"find / -fprintf "/path/to/file" -quit"#, true), + (r"find . -${t}exec touch asdf \{\} +", true), + (r"find . -${t:=exec} touch asdf2 \{\} +", true), // `grep` command arguments ("echo 'test data' | grep -P '(?{system(\"date\")})'", true), + ("echo 'test data' | grep --perl-regexp '(?{system(\"date\")})'", true), ]; for (cmd, expected) in cmds { let tool = serde_json::from_value::(serde_json::json!({