From ba350bdd9027f5a8fed0eaa0704c2fc832a08f60 Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 06:49:45 -0500 Subject: [PATCH 1/2] fix(cli): harden bash prefix approvals --- src-rust/crates/cli/src/main.rs | 27 +++++++--- src-rust/crates/tui/src/app.rs | 94 +++++++++++++++++++++++++++------ 2 files changed, 96 insertions(+), 25 deletions(-) diff --git a/src-rust/crates/cli/src/main.rs b/src-rust/crates/cli/src/main.rs index 044c904..1985026 100644 --- a/src-rust/crates/cli/src/main.rs +++ b/src-rust/crates/cli/src/main.rs @@ -1588,11 +1588,19 @@ fn permission_request_from_core( match (tool_name.as_str(), pending.request.path.clone()) { ("Bash", Some(command)) => { - let suggested_prefix = command - .split_whitespace() - .next() - .filter(|prefix| !prefix.is_empty()) - .map(|prefix| format!("{} ", prefix)); + let suggested_prefix = if command + .chars() + .any(|c| matches!(c, ';' | '|' | '&' | '<' | '>' | '\n' | '\r' | '`')) + || command.contains("$(") + { + None + } else { + let mut words = command.split_whitespace(); + words.next().map(|first| match words.next() { + Some(second) => format!("{} {}", first, second), + None => first.to_string(), + }) + }; claurst_tui::dialogs::PermissionRequest::bash( tool_use_id, tool_name, @@ -2760,9 +2768,12 @@ async fn run_interactive( .and_then(|p| p.request.path.clone()); let bash_prefix = if should_record_bash_prefix { match &pr.kind { - claurst_tui::dialogs::PermissionDialogKind::Bash { command, .. } => { - let first_word = command.split_whitespace().next().unwrap_or("").to_string(); - if first_word.is_empty() { None } else { Some(first_word) } + claurst_tui::dialogs::PermissionDialogKind::Bash { suggested_prefix, .. } => { + suggested_prefix + .as_deref() + .map(str::trim) + .filter(|prefix| !prefix.is_empty()) + .map(str::to_string) } _ => None, } diff --git a/src-rust/crates/tui/src/app.rs b/src-rust/crates/tui/src/app.rs index 464f757..3901ec7 100644 --- a/src-rust/crates/tui/src/app.rs +++ b/src-rust/crates/tui/src/app.rs @@ -5394,23 +5394,76 @@ impl App { if selected_key != Some('P') { return; } - if let PermissionDialogKind::Bash { command, .. } = &pr.kind { - // Always normalize to the first whitespace-delimited word so - // that the allowlist check in `bash_command_allowed_by_prefix` - // (which also uses `split_whitespace().next()`) matches correctly. - let first_word = command.split_whitespace().next().unwrap_or("").to_string(); - if !first_word.is_empty() { - self.bash_prefix_allowlist.insert(first_word); + if let PermissionDialogKind::Bash { suggested_prefix, .. } = &pr.kind { + if let Some(prefix) = suggested_prefix + .as_deref() + .map(str::trim) + .filter(|p| !p.is_empty()) + { + if !Self::bash_command_has_shell_control(prefix) { + self.bash_prefix_allowlist.insert(prefix.to_string()); + } } } } + fn bash_command_has_shell_control(command: &str) -> bool { + let mut chars = command.chars().peekable(); + let mut in_single_quote = false; + let mut in_double_quote = false; + let mut escaped = false; + + while let Some(c) = chars.next() { + if escaped { + escaped = false; + continue; + } + + if c == '\\' { + escaped = true; + continue; + } + + match c { + '\'' if !in_double_quote => in_single_quote = !in_single_quote, + '"' if !in_single_quote => in_double_quote = !in_double_quote, + ';' | '|' | '&' | '<' | '>' | '\n' | '\r' if !in_single_quote && !in_double_quote => { + return true; + } + '`' if !in_single_quote => return true, + '$' if !in_single_quote && chars.peek() == Some(&'(') => return true, + _ => {} + } + } + + false + } + + fn bash_prefix_matches_command(prefix: &str, command: &str) -> bool { + let prefix = prefix.trim(); + let command = command.trim_start(); + if prefix.is_empty() || !command.starts_with(prefix) { + return false; + } + + let rest = &command[prefix.len()..]; + rest.is_empty() + || rest + .chars() + .next() + .map(|c| c.is_ascii_whitespace()) + .unwrap_or(false) + } + /// Returns `true` if the given bash `command` is covered by the session-local - /// prefix allowlist (i.e. its first word matches an entry in - /// `bash_prefix_allowlist`). Used by callers to skip the permission dialog. + /// prefix allowlist. Shell compound commands are never allowed by prefix; + /// they must go through the normal permission dialog. pub fn bash_command_allowed_by_prefix(&self, command: &str) -> bool { - let first_word = command.split_whitespace().next().unwrap_or(""); - !first_word.is_empty() && self.bash_prefix_allowlist.contains(first_word) + !Self::bash_command_has_shell_control(command) + && self + .bash_prefix_allowlist + .iter() + .any(|prefix| Self::bash_prefix_matches_command(prefix, command)) } // ---- Advanced mouse interaction helpers -------------------------------- @@ -7049,7 +7102,7 @@ mod tests { "Bash".to_string(), "This will execute a shell command.".to_string(), "git status".to_string(), - Some("git".to_string()), + Some("git status".to_string()), ); app.permission_request = Some(pr); @@ -7062,11 +7115,17 @@ mod tests { }; app.handle_permission_key(key); - // Dialog should be dismissed and "git" added to the allowlist. + // Dialog should be dismissed and the suggested prefix added to the allowlist. assert!(app.permission_request.is_none()); assert!(app.bash_command_allowed_by_prefix("git status")); - assert!(app.bash_command_allowed_by_prefix("git push origin main")); - // Other commands should NOT be allowed. + assert!(app.bash_command_allowed_by_prefix("git status --short")); + // Other commands and compound shell commands should NOT be allowed. + assert!(!app.bash_command_allowed_by_prefix("git push origin main")); + assert!(!app.bash_command_allowed_by_prefix("git status; curl https://example.com")); + assert!(!app.bash_command_allowed_by_prefix( + "git status ; curl https://example.com" + )); + assert!(!app.bash_command_allowed_by_prefix("git status > /tmp/status.txt")); assert!(!app.bash_command_allowed_by_prefix("rm -rf /tmp")); } @@ -7081,7 +7140,7 @@ mod tests { "Bash".to_string(), "This will execute a shell command.".to_string(), "cargo build".to_string(), - Some("cargo".to_string()), + Some("cargo build".to_string()), ); // Navigate to the prefix option (index 3 in a 5-option dialog). pr.selected_option = 3; @@ -7097,7 +7156,8 @@ mod tests { app.handle_permission_key(key); assert!(app.permission_request.is_none()); - assert!(app.bash_command_allowed_by_prefix("cargo test")); + assert!(app.bash_command_allowed_by_prefix("cargo build --workspace")); + assert!(!app.bash_command_allowed_by_prefix("cargo test")); assert!(!app.bash_command_allowed_by_prefix("make build")); } From e041e1dcf9564a44da2f3aa8849ec075a2ba4b7f Mon Sep 17 00:00:00 2001 From: Val Alexander Date: Wed, 3 Jun 2026 17:25:00 -0500 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src-rust/crates/tui/src/app.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src-rust/crates/tui/src/app.rs b/src-rust/crates/tui/src/app.rs index 3901ec7..b559783 100644 --- a/src-rust/crates/tui/src/app.rs +++ b/src-rust/crates/tui/src/app.rs @@ -5419,7 +5419,8 @@ impl App { continue; } - if c == '\\' { + // In bash, backslash does not escape characters inside single quotes. + if c == '\\' && !in_single_quote { escaped = true; continue; } @@ -5436,7 +5437,8 @@ impl App { } } - false + // Treat unterminated quotes / dangling escapes as unsafe. + escaped || in_single_quote || in_double_quote } fn bash_prefix_matches_command(prefix: &str, command: &str) -> bool {