fix(cli): harden bash prefix approvals#30
Open
BunsDev wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the Bash “prefix allow” flow in the TUI/CLI by preventing session allowlist entries (and allowlist checks) from accidentally approving compound/redirected/substitution shell commands that should always require an explicit permission prompt.
Changes:
- Switch prefix recording to use the dialog’s explicit
suggested_prefix(instead of re-deriving the first whitespace token). - Add a shell-control scan plus bounded-prefix matching to decide whether a Bash command can skip the permission dialog.
- Update
claurst-tuiunit tests to validate the tighter allowlist behavior for chaining/redirection cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src-rust/crates/tui/src/app.rs |
Records suggested prefixes, adds shell-control scanning + bounded prefix matching, and updates tests for stricter allowlist behavior. |
src-rust/crates/cli/src/main.rs |
Avoids proposing a prefix for Bash commands containing obvious shell-control/substitution syntax and records the dialog-provided suggested prefix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5422
to
+5439
| 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 |
Comment on lines
5461
to
+5466
| 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)) |
Comment on lines
+7122
to
+7128
| // 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")); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Description
;,|,&,<,>, backticks,$(, newlines, etc.) and compute the dialogsuggested_prefixfrom the dialog payload when safe insrc-rust/crates/cli/src/main.rs.suggested_prefix(trimmed) into the session allowlist instead of re-deriving the first token, and only insert it if it does not itself contain shell-control syntax insrc-rust/crates/tui/src/app.rs.bash_command_has_shell_controlscan that rejects compound/redirected/substitution commands before skipping the permission dialog insrc-rust/crates/tui/src/app.rs.crates/tuito assert the tighter prefix behavior and to reject chained/redirection cases (e.g.git status; curl ...,git status > /tmp/..., and similar).Testing
cargo fmt --allto format changes (succeeded).cargo test --package claurst-tui bash_prefix_allowlistand the three updated tests passed locally (3 passed; 0 failed).cargo check,cargo clippy, andcargo test --workspacebut these were blocked by a missing system dependency (alsa.pcrequired byalsa-sys) in the execution environment, so full-workspace verification could not complete here.Codex Task