Skip to content

Commit d426f19

Browse files
authored
fix(cli): harden bash prefix approvals (#30)
Verified locally before merge: cargo test --package claurst-tui bash_prefix_allowlist -- --nocapture; cargo check -p claurst --no-default-features; git diff --check origin/main...HEAD. Note: cargo check reported the known pre-existing claurst-tui::rustle warning.
1 parent 35d46d1 commit d426f19

2 files changed

Lines changed: 98 additions & 25 deletions

File tree

src-rust/crates/cli/src/main.rs

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1589,11 +1589,19 @@ fn permission_request_from_core(
15891589

15901590
match (tool_name.as_str(), pending.request.path.clone()) {
15911591
("Bash", Some(command)) => {
1592-
let suggested_prefix = command
1593-
.split_whitespace()
1594-
.next()
1595-
.filter(|prefix| !prefix.is_empty())
1596-
.map(|prefix| format!("{} ", prefix));
1592+
let suggested_prefix = if command
1593+
.chars()
1594+
.any(|c| matches!(c, ';' | '|' | '&' | '<' | '>' | '\n' | '\r' | '`'))
1595+
|| command.contains("$(")
1596+
{
1597+
None
1598+
} else {
1599+
let mut words = command.split_whitespace();
1600+
words.next().map(|first| match words.next() {
1601+
Some(second) => format!("{} {}", first, second),
1602+
None => first.to_string(),
1603+
})
1604+
};
15971605
claurst_tui::dialogs::PermissionRequest::bash(
15981606
tool_use_id,
15991607
tool_name,
@@ -2761,9 +2769,12 @@ async fn run_interactive(
27612769
.and_then(|p| p.request.path.clone());
27622770
let bash_prefix = if should_record_bash_prefix {
27632771
match &pr.kind {
2764-
claurst_tui::dialogs::PermissionDialogKind::Bash { command, .. } => {
2765-
let first_word = command.split_whitespace().next().unwrap_or("").to_string();
2766-
if first_word.is_empty() { None } else { Some(first_word) }
2772+
claurst_tui::dialogs::PermissionDialogKind::Bash { suggested_prefix, .. } => {
2773+
suggested_prefix
2774+
.as_deref()
2775+
.map(str::trim)
2776+
.filter(|prefix| !prefix.is_empty())
2777+
.map(str::to_string)
27672778
}
27682779
_ => None,
27692780
}

src-rust/crates/tui/src/app.rs

Lines changed: 79 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5324,23 +5324,78 @@ impl App {
53245324
if selected_key != Some('P') {
53255325
return;
53265326
}
5327-
if let PermissionDialogKind::Bash { command, .. } = &pr.kind {
5328-
// Always normalize to the first whitespace-delimited word so
5329-
// that the allowlist check in `bash_command_allowed_by_prefix`
5330-
// (which also uses `split_whitespace().next()`) matches correctly.
5331-
let first_word = command.split_whitespace().next().unwrap_or("").to_string();
5332-
if !first_word.is_empty() {
5333-
self.bash_prefix_allowlist.insert(first_word);
5327+
if let PermissionDialogKind::Bash { suggested_prefix, .. } = &pr.kind {
5328+
if let Some(prefix) = suggested_prefix
5329+
.as_deref()
5330+
.map(str::trim)
5331+
.filter(|p| !p.is_empty())
5332+
{
5333+
if !Self::bash_command_has_shell_control(prefix) {
5334+
self.bash_prefix_allowlist.insert(prefix.to_string());
5335+
}
53345336
}
53355337
}
53365338
}
53375339

5340+
fn bash_command_has_shell_control(command: &str) -> bool {
5341+
let mut chars = command.chars().peekable();
5342+
let mut in_single_quote = false;
5343+
let mut in_double_quote = false;
5344+
let mut escaped = false;
5345+
5346+
while let Some(c) = chars.next() {
5347+
if escaped {
5348+
escaped = false;
5349+
continue;
5350+
}
5351+
5352+
// In bash, backslash does not escape characters inside single quotes.
5353+
if c == '\\' && !in_single_quote {
5354+
escaped = true;
5355+
continue;
5356+
}
5357+
5358+
match c {
5359+
'\'' if !in_double_quote => in_single_quote = !in_single_quote,
5360+
'"' if !in_single_quote => in_double_quote = !in_double_quote,
5361+
';' | '|' | '&' | '<' | '>' | '\n' | '\r' if !in_single_quote && !in_double_quote => {
5362+
return true;
5363+
}
5364+
'`' if !in_single_quote => return true,
5365+
'$' if !in_single_quote && chars.peek() == Some(&'(') => return true,
5366+
_ => {}
5367+
}
5368+
}
5369+
5370+
// Treat unterminated quotes / dangling escapes as unsafe.
5371+
escaped || in_single_quote || in_double_quote
5372+
}
5373+
5374+
fn bash_prefix_matches_command(prefix: &str, command: &str) -> bool {
5375+
let prefix = prefix.trim();
5376+
let command = command.trim_start();
5377+
if prefix.is_empty() || !command.starts_with(prefix) {
5378+
return false;
5379+
}
5380+
5381+
let rest = &command[prefix.len()..];
5382+
rest.is_empty()
5383+
|| rest
5384+
.chars()
5385+
.next()
5386+
.map(|c| c.is_ascii_whitespace())
5387+
.unwrap_or(false)
5388+
}
5389+
53385390
/// Returns `true` if the given bash `command` is covered by the session-local
5339-
/// prefix allowlist (i.e. its first word matches an entry in
5340-
/// `bash_prefix_allowlist`). Used by callers to skip the permission dialog.
5391+
/// prefix allowlist. Shell compound commands are never allowed by prefix;
5392+
/// they must go through the normal permission dialog.
53415393
pub fn bash_command_allowed_by_prefix(&self, command: &str) -> bool {
5342-
let first_word = command.split_whitespace().next().unwrap_or("");
5343-
!first_word.is_empty() && self.bash_prefix_allowlist.contains(first_word)
5394+
!Self::bash_command_has_shell_control(command)
5395+
&& self
5396+
.bash_prefix_allowlist
5397+
.iter()
5398+
.any(|prefix| Self::bash_prefix_matches_command(prefix, command))
53445399
}
53455400

53465401
// ---- Advanced mouse interaction helpers --------------------------------
@@ -7112,7 +7167,7 @@ mod tests {
71127167
"Bash".to_string(),
71137168
"This will execute a shell command.".to_string(),
71147169
"git status".to_string(),
7115-
Some("git".to_string()),
7170+
Some("git status".to_string()),
71167171
);
71177172
app.permission_request = Some(pr);
71187173

@@ -7125,11 +7180,17 @@ mod tests {
71257180
};
71267181
app.handle_permission_key(key);
71277182

7128-
// Dialog should be dismissed and "git" added to the allowlist.
7183+
// Dialog should be dismissed and the suggested prefix added to the allowlist.
71297184
assert!(app.permission_request.is_none());
71307185
assert!(app.bash_command_allowed_by_prefix("git status"));
7131-
assert!(app.bash_command_allowed_by_prefix("git push origin main"));
7132-
// Other commands should NOT be allowed.
7186+
assert!(app.bash_command_allowed_by_prefix("git status --short"));
7187+
// Other commands and compound shell commands should NOT be allowed.
7188+
assert!(!app.bash_command_allowed_by_prefix("git push origin main"));
7189+
assert!(!app.bash_command_allowed_by_prefix("git status; curl https://example.com"));
7190+
assert!(!app.bash_command_allowed_by_prefix(
7191+
"git status ; curl https://example.com"
7192+
));
7193+
assert!(!app.bash_command_allowed_by_prefix("git status > /tmp/status.txt"));
71337194
assert!(!app.bash_command_allowed_by_prefix("rm -rf /tmp"));
71347195
}
71357196

@@ -7144,7 +7205,7 @@ mod tests {
71447205
"Bash".to_string(),
71457206
"This will execute a shell command.".to_string(),
71467207
"cargo build".to_string(),
7147-
Some("cargo".to_string()),
7208+
Some("cargo build".to_string()),
71487209
);
71497210
// Navigate to the prefix option (index 3 in a 5-option dialog).
71507211
pr.selected_option = 3;
@@ -7160,7 +7221,8 @@ mod tests {
71607221
app.handle_permission_key(key);
71617222

71627223
assert!(app.permission_request.is_none());
7163-
assert!(app.bash_command_allowed_by_prefix("cargo test"));
7224+
assert!(app.bash_command_allowed_by_prefix("cargo build --workspace"));
7225+
assert!(!app.bash_command_allowed_by_prefix("cargo test"));
71647226
assert!(!app.bash_command_allowed_by_prefix("make build"));
71657227
}
71667228

0 commit comments

Comments
 (0)