Skip to content

Commit 2786f01

Browse files
authored
feat(cli) Bash pipe command safety (#927)
* feat(cli): Add find and grep as commands to the safe list Rational: these are common commands used by the CLI to determine the current state of the users workspace when understanding context for a task. * feat(cli): Support safe piped commands in execute_bash Allow piped commands that only use safe read-only operations like 'find | grep' or 'ls | grep'. Each command in the pipe chain is validated against the READONLY_COMMANDS list. 🤖 Assisted by [Amazon Q Developer](https://aws.amazon.com/q/developer)
1 parent 3545e45 commit 2786f01

File tree

1 file changed

+43
-6
lines changed

1 file changed

+43
-6
lines changed

crates/q_cli/src/cli/chat/tools/execute_bash.rs

+43-6
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use super::{
2424
};
2525
use crate::cli::chat::truncate_safe;
2626

27-
const READONLY_COMMANDS: &[&str] = &["ls", "cat", "echo", "pwd", "which", "head", "tail"];
27+
const READONLY_COMMANDS: &[&str] = &["ls", "cat", "echo", "pwd", "which", "head", "tail", "find", "grep"];
2828

2929
#[derive(Debug, Clone, Deserialize)]
3030
pub struct ExecuteBash {
@@ -37,19 +37,48 @@ impl ExecuteBash {
3737
return true;
3838
};
3939

40-
const DANGEROUS_PATTERNS: &[&str] = &["|", "<(", "$(", "`", ">", "&&", "||"];
40+
const DANGEROUS_PATTERNS: &[&str] = &["<(", "$(", "`", ">", "&&", "||"];
4141
if args
4242
.iter()
4343
.any(|arg| DANGEROUS_PATTERNS.iter().any(|p| arg.contains(p)))
4444
{
4545
return true;
4646
}
4747

48-
if let Some(cmd) = args.first() {
49-
!READONLY_COMMANDS.contains(&cmd.as_str())
50-
} else {
51-
true
48+
// Split commands by pipe and check each one
49+
let mut current_cmd = Vec::new();
50+
let mut all_commands = Vec::new();
51+
52+
for arg in args {
53+
if arg == "|" {
54+
if !current_cmd.is_empty() {
55+
all_commands.push(current_cmd);
56+
}
57+
current_cmd = Vec::new();
58+
} else if arg.contains("|") {
59+
// if pipe appears without spacing e.g. `echo myimportantfile|args rm` it won't get
60+
// parsed out, in this case - we want to verify before running
61+
return true;
62+
} else {
63+
current_cmd.push(arg);
64+
}
65+
}
66+
if !current_cmd.is_empty() {
67+
all_commands.push(current_cmd);
5268
}
69+
70+
// Check if each command in the pipe chain starts with a safe command
71+
for cmd_args in all_commands {
72+
if let Some(cmd) = cmd_args.first() {
73+
if !READONLY_COMMANDS.contains(&cmd.as_str()) {
74+
return true;
75+
}
76+
} else {
77+
return true;
78+
}
79+
}
80+
81+
false
5382
}
5483

5584
pub async fn invoke(&self, mut updates: impl Write) -> Result<InvokeOutput> {
@@ -252,6 +281,14 @@ mod tests {
252281
("cat <<< 'some string here' > myimportantfile", true),
253282
("echo '\n#!/usr/bin/env bash\necho hello\n' > myscript.sh", true),
254283
("cat <<EOF > myimportantfile\nhello world\nEOF", true),
284+
// Safe piped commands
285+
("find . -name '*.rs' | grep main", false),
286+
("ls -la | grep .git", false),
287+
("cat file.txt | grep pattern | head -n 5", false),
288+
// Unsafe piped commands
289+
("find . -name '*.rs' | rm", true),
290+
("ls -la | grep .git | rm -rf", true),
291+
("echo hello | sudo rm -rf /", true),
255292
];
256293
for (cmd, expected) in cmds {
257294
let tool = serde_json::from_value::<ExecuteBash>(serde_json::json!({

0 commit comments

Comments
 (0)