fix(core/shell): normalize raw shell script handling

This commit is contained in:
James Peter
2026-04-17 09:26:18 +10:00
parent a092ad44e2
commit 83b96a58bb
7 changed files with 148 additions and 167 deletions
+22 -25
View File
@@ -1,5 +1,6 @@
use super::*;
use serde_json::Value;
use crate::util::extract_shell_script;
use code_protocol::dynamic_tools::DynamicToolResponse;
use code_protocol::dynamic_tools::DynamicToolSpec;
use super::streaming::{
@@ -74,34 +75,12 @@ fn semantic_tokens(command: &[String]) -> Option<Vec<String>> {
if command.is_empty() {
return None;
}
if let Some(tokens) = shell_script_tokens(command) {
return Some(tokens);
if let Some((_, script)) = extract_shell_script(command) {
return Some(shlex_split(script).unwrap_or_else(|| vec![script.to_string()]));
}
Some(command.to_vec())
}
fn shell_script_tokens(command: &[String]) -> Option<Vec<String>> {
if command.len() == 3 && is_shell_wrapper(&command[0], &command[1]) {
if let Some(tokens) = shlex_split(&command[2]) {
return Some(tokens);
}
return Some(vec![command[2].clone()]);
}
None
}
fn is_shell_wrapper(shell: &str, flag: &str) -> bool {
let file_name = Path::new(shell)
.file_name()
.and_then(|s| s.to_str())
.unwrap_or(shell)
.to_ascii_lowercase();
matches!(
file_name.as_str(),
"bash" | "sh" | "zsh" | "ksh" | "fish" | "dash"
) && matches!(flag, "-lc" | "-c")
}
#[derive(Clone)]
pub(super) struct RunningExecMeta {
pub(super) sub_id: String,
@@ -349,7 +328,14 @@ pub(super) fn is_connectivity_error(err: &CodexErr) -> bool {
#[cfg(test)]
mod tests {
use super::is_connectivity_error;
use super::{FollowUpTurnAction, QueuedUserInput, State, take_follow_up_turn_action};
use super::{
ApprovedCommandMatchKind,
ApprovedCommandPattern,
FollowUpTurnAction,
QueuedUserInput,
State,
take_follow_up_turn_action,
};
use crate::error::CodexErr;
use code_protocol::models::{ContentItem, ResponseInputItem};
@@ -424,6 +410,17 @@ mod tests {
assert!(matches!(second, FollowUpTurnAction::QueuedUserInput(_)));
assert!(state.pending_user_input.is_empty());
}
#[test]
fn approved_command_prefix_matches_raw_shell_script_tokens() {
let pattern = ApprovedCommandPattern::new(
vec!["git".to_string(), "status".to_string()],
ApprovedCommandMatchKind::Prefix,
None,
);
assert!(pattern.matches(&["git status --short".to_string()]));
}
}
#[derive(Debug)]
+32 -21
View File
@@ -9675,12 +9675,12 @@ async fn handle_container_exec_with_params(
}
// If the argv is a shell wrapper, analyze and optionally strip `confirm:`.
// If the command is a shell script, analyze and optionally strip `confirm:`.
let mut params = params;
let seq_hint_for_exec = seq_hint;
let otel_event_manager = sess.client.get_otel_event_manager();
let tool_name = "local_shell";
if let Some((script_index, script)) = extract_shell_script_from_wrapper(&params.command) {
if let Some((script_index, script)) = extract_shell_script(&params.command) {
let trimmed = script.trim_start();
let confirm_prefixes = ["confirm:", "CONFIRM:"];
let has_confirm_prefix = confirm_prefixes
@@ -9848,8 +9848,8 @@ async fn handle_container_exec_with_params(
};
}
// If no shell wrapper, perform a lightweight argv inspection for sensitive git commands.
if extract_shell_script_from_wrapper(&params.command).is_none() {
// If no shell script is present, perform a lightweight argv inspection for sensitive git commands.
if extract_shell_script(&params.command).is_none() {
let joined = params.command.join(" ");
if !sess.confirm_guard.is_empty() {
if let Some(pattern) = sess.confirm_guard.matched_pattern(&joined) {
@@ -12768,20 +12768,8 @@ async fn handle_browser_history(sess: &Session, ctx: &ToolCallCtx, arguments: St
.await
}
fn extract_shell_script_from_wrapper(argv: &[String]) -> Option<(usize, String)> {
// Return (index_of_script, script) if argv matches: <shell> (-lc|-c) <script>
if argv.len() == 3 {
let shell = std::path::Path::new(&argv[0])
.file_name()
.and_then(|s| s.to_str())
.unwrap_or("");
let is_shell = matches!(shell, "bash" | "sh" | "zsh");
let is_flag = matches!(argv[1].as_str(), "-lc" | "-c");
if is_shell && is_flag {
return Some((2, argv[2].clone()));
}
}
None
fn extract_shell_script(argv: &[String]) -> Option<(usize, String)> {
crate::util::extract_shell_script(argv).map(|(index, script)| (index, script.to_string()))
}
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -12791,7 +12779,7 @@ struct CatWriteSuggestion {
}
fn detect_cat_write(argv: &[String]) -> Option<CatWriteSuggestion> {
if let Some((_, script)) = extract_shell_script_from_wrapper(argv) {
if let Some((_, script)) = extract_shell_script(argv) {
if script_contains_cat_write(&script) {
return Some(CatWriteSuggestion {
label: "original_script",
@@ -12961,7 +12949,7 @@ struct PythonWriteSuggestion {
}
fn detect_python_write(argv: &[String]) -> Option<PythonWriteSuggestion> {
if let Some((_, script)) = extract_shell_script_from_wrapper(argv) {
if let Some((_, script)) = extract_shell_script(argv) {
if script_contains_python_write(&script) {
return Some(PythonWriteSuggestion {
label: "original_script",
@@ -13045,7 +13033,7 @@ struct RedundantCdSuggestion {
fn detect_redundant_cd(argv: &[String], cwd: &Path) -> Option<RedundantCdSuggestion> {
let normalized_cwd = normalize_path(cwd);
if let Some((script_index, script)) = extract_shell_script_from_wrapper(argv) {
if let Some((script_index, script)) = extract_shell_script(argv) {
if let Some(suggestion) = detect_redundant_cd_in_shell(
argv,
script_index,
@@ -13236,6 +13224,16 @@ mod command_guard_detection_tests {
assert_eq!(suggestion.suggested, vec!["bash".to_string(), "-lc".to_string(), "ls".to_string()]);
}
#[test]
fn detects_raw_shell_script_redundant_cd() {
let cwd = PathBuf::from("/tmp/project");
let argv = vec!["cd /tmp/project && ls".to_string()];
let suggestion = detect_redundant_cd(&argv, &cwd).expect("should flag redundant cd");
assert_eq!(suggestion.label, "original_script");
assert_eq!(suggestion.suggested, vec!["ls".to_string()]);
}
#[test]
fn ignores_cd_to_different_directory() {
let cwd = PathBuf::from("/tmp/project");
@@ -13275,6 +13273,19 @@ mod command_guard_detection_tests {
.contains("cat <<'EOF' > code-rs/git-tooling/Cargo.toml"));
}
#[test]
fn detects_raw_shell_script_cat_heredoc_write() {
let argv = vec![
"cat <<'EOF' > code-rs/git-tooling/Cargo.toml\n[package]\nname = \"demo\"\nEOF".to_string(),
];
let suggestion = detect_cat_write(&argv).expect("should flag cat write");
assert_eq!(suggestion.label, "original_script");
assert!(suggestion
.original_value
.contains("cat <<'EOF' > code-rs/git-tooling/Cargo.toml"));
}
#[test]
fn allows_cat_heredoc_without_redirect() {
let argv = vec![
+54 -10
View File
@@ -1,5 +1,6 @@
use crate::bash::try_parse_bash;
use crate::bash::try_parse_word_only_commands_sequence;
use crate::util::extract_shell_script;
use crate::util::is_shell_like_executable;
use serde::Deserialize;
use serde::Serialize;
@@ -116,6 +117,16 @@ mod tests {
);
}
#[test]
fn raw_shell_script_git_status_is_read_command() {
assert_parsed(
&vec_str(&["git status"]),
vec![ParsedCommand::ReadCommand {
cmd: "git status".to_string(),
}],
);
}
#[test]
fn handles_git_pipe_wc() {
let inner = "git status | wc -l";
@@ -1049,7 +1060,7 @@ mod tests {
}
pub fn parse_command_impl(command: &[String]) -> Vec<ParsedCommand> {
if let Some(commands) = parse_bash_lc_commands(command) {
if let Some(commands) = parse_shell_script_commands(command) {
return commands;
}
@@ -1451,20 +1462,28 @@ fn parse_git_grep_query_and_path(args: &[String]) -> (Option<String>, Option<Str
(query, path)
}
fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
let [bash, flag, script] = original else {
return None;
fn parse_shell_script_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
let shell_flag = match original {
[script] => {
return parse_shell_script_text(script, None);
}
[bash, flag, _] if is_shell_executable(bash) && matches!(flag.as_str(), "-lc" | "-c") => {
Some(flag.as_str())
}
_ => None,
};
if !is_shell_executable(bash) || flag != "-lc" {
return None;
}
let (_, script) = extract_shell_script(original)?;
parse_shell_script_text(script, shell_flag)
}
fn parse_shell_script_text(script: &str, shell_flag: Option<&str>) -> Option<Vec<ParsedCommand>> {
if let Some(tree) = try_parse_bash(script) {
if let Some(all_commands) = try_parse_word_only_commands_sequence(&tree, script) {
if all_commands.is_empty() {
return None;
}
let script_tokens = shlex_split(script)
.unwrap_or_else(|| vec!["bash".to_string(), flag.clone(), script.clone()]);
.unwrap_or_else(|| fallback_shell_script_tokens(script, shell_flag));
// Strip small formatting helpers (e.g., head/tail/awk/wc/etc) so we
// bias toward the primary command when pipelines are present.
let had_multiple_commands = all_commands.len() > 1;
@@ -1508,7 +1527,7 @@ fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
});
if has_pipe && has_sed_n {
ParsedCommand::Read {
cmd: script.clone(),
cmd: script.to_string(),
name,
}
} else {
@@ -1561,7 +1580,32 @@ fn parse_bash_lc_commands(original: &[String]) -> Option<Vec<ParsedCommand>> {
return Some(commands);
}
}
None
let tokens = shlex_split(script).unwrap_or_else(|| vec![script.to_string()]);
let normalized = normalize_tokens(&tokens);
let parts = if contains_connectors(&normalized) {
split_on_connectors(&normalized)
} else {
vec![normalized.clone()]
};
let mut commands: Vec<ParsedCommand> = parts
.iter()
.map(|segment| summarize_main_tokens(segment))
.collect();
while let Some(next) = simplify_once(&commands) {
commands = next;
}
Some(commands)
}
fn fallback_shell_script_tokens(script: &str, shell_flag: Option<&str>) -> Vec<String> {
match shell_flag {
Some(flag) => vec!["bash".to_string(), flag.to_string(), script.to_string()],
None => vec![script.to_string()],
}
}
/// Return true if this looks like a small formatting helper in a pipeline.
+25 -4
View File
@@ -47,18 +47,25 @@ pub fn escape_command(command: &[String]) -> String {
try_join(command.iter().map(|s| s.as_str())).unwrap_or_else(|_| command.join(" "))
}
pub fn strip_bash_lc_and_escape(command: &[String]) -> String {
pub fn extract_shell_script(command: &[String]) -> Option<(usize, &str)> {
match command {
[script] => Some((0, script.as_str())),
[first, second, third]
if is_shell_like_executable(first)
&& second == "-lc" =>
&& matches!(second.as_str(), "-lc" | "-c") =>
{
strip_shell_wrapper_for_display(third)
Some((2, third.as_str()))
}
_ => escape_command(command),
_ => None,
}
}
pub fn strip_bash_lc_and_escape(command: &[String]) -> String {
extract_shell_script(command)
.map(|(_, script)| strip_shell_wrapper_for_display(script))
.unwrap_or_else(|| escape_command(command))
}
fn strip_shell_wrapper_for_display(script: &str) -> String {
unwrap_profile_wrapper(script).unwrap_or(script).to_string()
}
@@ -169,6 +176,20 @@ mod tests {
assert_eq!(strip_bash_lc_and_escape(&command), "sed -n '1,5p' file.txt");
}
#[test]
fn strip_bash_lc_and_escape_shows_raw_shell_script_without_quotes() {
let command = vec!["git status --short".to_string()];
assert_eq!(strip_bash_lc_and_escape(&command), "git status --short");
}
#[test]
fn strip_bash_lc_and_escape_shows_raw_multiline_shell_script_without_quotes() {
let command = vec!["cat <<'EOF'\nhello\nEOF".to_string()];
assert_eq!(strip_bash_lc_and_escape(&command), "cat <<'EOF'\nhello\nEOF");
}
#[test]
fn strip_bash_lc_and_escape_hides_multiline_profile_wrapper() {
let bashrc = home_rc_path(".bashrc");
+9 -68
View File
@@ -1,76 +1,10 @@
use std::path::Path;
use std::path::PathBuf;
use shlex::try_join;
pub(crate) fn escape_command(command: &[String]) -> String {
try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" "))
}
use code_core::util as core_util;
pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String {
match command {
// exactly three items
[first, second, third]
// first two must be "bash", "-lc"
if is_bash_like(first) && second == "-lc" =>
{
strip_shell_wrapper_for_display(third)
}
_ => escape_command(command),
}
}
fn strip_shell_wrapper_for_display(script: &str) -> String {
unwrap_profile_wrapper(script).unwrap_or(script).to_string()
}
fn unwrap_profile_wrapper(script: &str) -> Option<&str> {
let script = script.strip_prefix("set +m; ").unwrap_or(script);
let body = script.strip_prefix("source ")?;
let (rc_path, wrapped) = body.split_once(" && ")?;
if !looks_like_shell_rc_path(rc_path) {
return None;
}
wrapped
.strip_prefix('(')
.and_then(|inner| inner.strip_suffix(')'))
.or_else(|| {
wrapped
.strip_prefix("{\n")
.and_then(|inner| inner.strip_suffix("\n}"))
})
}
fn looks_like_shell_rc_path(path: &str) -> bool {
let Some(home) = std::env::var_os("HOME") else {
return false;
};
let home = Path::new(&home);
path == home.join(".bashrc").to_string_lossy() || path == home.join(".zshrc").to_string_lossy()
}
fn is_bash_like(cmd: &str) -> bool {
let trimmed = cmd.trim_matches('"').trim_matches('\'');
let lowered = Path::new(trimmed)
.file_name()
.and_then(|s| s.to_str())
.unwrap_or(trimmed)
.to_ascii_lowercase();
matches!(
lowered.as_str(),
"bash"
| "bash.exe"
| "sh"
| "sh.exe"
| "dash"
| "dash.exe"
| "zsh"
| "zsh.exe"
| "ksh"
| "ksh.exe"
| "busybox"
)
core_util::strip_bash_lc_and_escape(command)
}
/// If `path` is absolute and inside $HOME, return the part *after* the home
@@ -117,6 +51,13 @@ mod tests {
assert_eq!(strip_bash_lc_and_escape(&command), "sed -n '1,220p' file.txt");
}
#[test]
fn strip_bash_lc_and_escape_shows_raw_shell_script_without_quotes() {
let command = vec!["git status --short".to_string()];
assert_eq!(strip_bash_lc_and_escape(&command), "git status --short");
}
#[test]
fn strip_bash_lc_and_escape_hides_multiline_profile_wrapper() {
let bashrc = home_rc_path(".bashrc");
+2 -26
View File
@@ -8,6 +8,7 @@ use crate::history::state::{
ExploreSummary,
};
use code_core::parse_command::ParsedCommand;
use code_core::util::extract_shell_script;
use shlex::Shlex;
use std::path::{Component, Path};
@@ -696,32 +697,7 @@ fn command_string_for_annotation(original_command: &[String]) -> String {
}
fn unwrap_shell_script(command: &[String]) -> Option<String> {
for idx in 0..command.len() {
if is_shell_like(&command[idx]) {
let mut j = idx + 1;
while j < command.len() {
let arg = &command[j];
if arg == "-c" || arg == "-lc" {
return command.get(j + 1).cloned();
}
if arg.starts_with('-') {
j += 1;
continue;
}
break;
}
}
}
None
}
fn is_shell_like(token: &str) -> bool {
filter_command_name(token)
.map(|name| match name.as_str() {
"bash" | "sh" | "dash" | "zsh" | "ksh" | "busybox" => true,
_ => false,
})
.unwrap_or(false)
extract_shell_script(command).map(|(_, script)| script.to_string())
}
fn looks_like_line_filter(cmd: &str) -> bool {
+4 -13
View File
@@ -6,8 +6,8 @@
//! UI to Rust using [`ratatui`]. The goal is feature‑parity for the keyboard
//! driven workflow – a fully‑fledged visual match is not required.
use std::path::Path;
use std::path::PathBuf;
use code_core::util::extract_shell_script;
use code_core::protocol::Op;
use code_core::protocol::ReviewDecision;
use crossterm::event::KeyCode;
@@ -502,11 +502,11 @@ fn normalized_command_tokens(command: &[String]) -> Option<Vec<String>> {
return None;
}
if command.len() == 3 && is_shell_wrapper(&command[0], &command[1]) {
if let Some(script_tokens) = shlex_split(&command[2]) {
if let Some((_, script)) = extract_shell_script(command) {
if let Some(script_tokens) = shlex_split(script) {
return Some(script_tokens);
}
return Some(vec![command[2].clone()]);
return Some(vec![script.to_string()]);
}
Some(command.to_vec())
@@ -545,15 +545,6 @@ fn prefix_candidate(tokens: &[String]) -> Option<Vec<String>> {
}
}
fn is_shell_wrapper(shell: &str, flag: &str) -> bool {
let file_name = Path::new(shell)
.file_name()
.and_then(|s| s.to_str())
.unwrap_or(shell)
.to_ascii_lowercase();
matches!(file_name.as_str(), "bash" | "sh" | "zsh") && matches!(flag, "-lc" | "-c")
}
fn hotkey_suffix(key: KeyCode) -> String {
match key {
KeyCode::Char(c) => format!(" ({})", c.to_ascii_lowercase()),