fix: add more safety checks for execute_bash readonly checks (#2605)

Author: brandonskiser

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<String>>, 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());
@@ -293,6 +303,14 @@ mod tests {
             ("cat <<< 'some string here' > myimportantfile", true),
             ("echo '\n#!/usr/bin/env bash\necho hello\n' > myscript.sh", true),
             ("cat <<EOF > 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),
@@ -310,8 +328,12 @@ mod tests {
                 true,
             ),
             ("find important-dir/ -name '*.txt'", false),
+            (r#"find / -fprintf "/path/to/file" <data-to-write> -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::<ExecuteCommand>(serde_json::json!({