diff --git a/codex-rs/core/src/bash.rs b/codex-rs/core/src/bash.rs index bb0ae7fe90e..fcb8f9ae174 100644 --- a/codex-rs/core/src/bash.rs +++ b/codex-rs/core/src/bash.rs @@ -119,6 +119,23 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option Option> { + let (_, script) = extract_bash_command(command)?; + let tree = try_parse_shell(script)?; + let root = tree.root_node(); + if root.has_error() { + return None; + } + if !has_named_descendant_kind(root, "heredoc_redirect") { + return None; + } + + let command_node = find_single_command_node(root)?; + parse_heredoc_command_words(command_node, script) +} + fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option> { if cmd.kind() != "command" { return None; @@ -177,6 +194,94 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option, src: &str) -> Option> { + if cmd.kind() != "command" { + return None; + } + + let mut words = Vec::new(); + let mut cursor = cmd.walk(); + for child in cmd.named_children(&mut cursor) { + match child.kind() { + "command_name" => { + let word_node = child.named_child(0)?; + if !matches!(word_node.kind(), "word" | "number") + || !is_literal_word_or_number(word_node) + { + return None; + } + words.push(word_node.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + "word" | "number" => { + if !is_literal_word_or_number(child) { + return None; + } + words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned()); + } + // Allow shell constructs that attach IO to a single command without + // changing argv matching semantics for the executable prefix. + "variable_assignment" | "comment" => {} + kind if is_allowed_heredoc_attachment_kind(kind) => {} + _ => return None, + } + } + + if words.is_empty() { None } else { Some(words) } +} + +fn is_literal_word_or_number(node: Node<'_>) -> bool { + if !matches!(node.kind(), "word" | "number") { + return false; + } + let mut cursor = node.walk(); + node.named_children(&mut cursor).next().is_none() +} + +fn has_named_descendant_kind(node: Node<'_>, kind: &str) -> bool { + let mut stack = vec![node]; + while let Some(current) = stack.pop() { + if current.kind() == kind { + return true; + } + let mut cursor = current.walk(); + for child in current.named_children(&mut cursor) { + stack.push(child); + } + } + false +} + +fn is_allowed_heredoc_attachment_kind(kind: &str) -> bool { + matches!( + kind, + "heredoc_body" + | "simple_heredoc_body" + | "heredoc_redirect" + | "herestring_redirect" + | "file_redirect" + | "redirected_statement" + ) +} + +fn find_single_command_node(root: Node<'_>) -> Option> { + let mut stack = vec![root]; + let mut single_command = None; + while let Some(node) = stack.pop() { + if node.kind() == "command" { + if single_command.is_some() { + return None; + } + single_command = Some(node); + } + + let mut cursor = node.walk(); + for child in node.named_children(&mut cursor) { + stack.push(child); + } + } + single_command +} + fn parse_double_quoted_string(node: Node, src: &str) -> Option { if node.kind() != "string" { return None; @@ -375,4 +480,86 @@ mod tests { assert!(parse_seq("rg -g\"$(pwd)\" pattern").is_none()); assert!(parse_seq("rg -g\"$(echo '*.py')\" pattern").is_none()); } + + #[test] + fn parse_shell_lc_single_command_prefix_supports_heredoc() { + let command = vec![ + "zsh".to_string(), + "-lc".to_string(), + "python3 <<'PY'\nprint('hello')\nPY".to_string(), + ]; + let parsed = parse_shell_lc_single_command_prefix(&command); + assert_eq!(parsed, Some(vec!["python3".to_string()])); + + let command_unquoted = vec![ + "zsh".to_string(), + "-lc".to_string(), + "python3 << PY\nprint('hello')\nPY".to_string(), + ]; + let parsed_unquoted = parse_shell_lc_single_command_prefix(&command_unquoted); + assert_eq!(parsed_unquoted, Some(vec!["python3".to_string()])); + } + + #[test] + fn parse_shell_lc_single_command_prefix_rejects_multi_command_scripts() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 <<'PY'\nprint('hello')\nPY\necho done".to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } + + #[test] + fn parse_shell_lc_single_command_prefix_rejects_non_heredoc_redirects() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "echo hello > /tmp/out.txt".to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } + + #[test] + fn parse_shell_lc_single_command_prefix_accepts_heredoc_with_extra_redirect() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 <<'PY' > /tmp/out.txt\nprint('hello')\nPY".to_string(), + ]; + assert_eq!( + parse_shell_lc_single_command_prefix(&command), + Some(vec!["python3".to_string()]) + ); + } + + #[test] + fn parse_shell_lc_single_command_prefix_rejects_herestring_with_substitution() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + r#"python3 <<< "$(rm -rf /)""#.to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } + + #[test] + fn parse_shell_lc_single_command_prefix_rejects_arithmetic_shift_non_heredoc_script() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "echo $((1<<2))".to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } + + #[test] + fn parse_shell_lc_single_command_prefix_rejects_heredoc_command_with_word_expansion() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 $((1<<2)) <<'PY'\nprint('hello')\nPY".to_string(), + ]; + assert_eq!(parse_shell_lc_single_command_prefix(&command), None); + } } diff --git a/codex-rs/core/src/command_canonicalization.rs b/codex-rs/core/src/command_canonicalization.rs new file mode 100644 index 00000000000..0708e41e193 --- /dev/null +++ b/codex-rs/core/src/command_canonicalization.rs @@ -0,0 +1,130 @@ +use crate::bash::extract_bash_command; +use crate::bash::parse_shell_lc_plain_commands; +use crate::powershell::extract_powershell_command; + +const CANONICAL_BASH_SCRIPT_PREFIX: &str = "__codex_shell_script__"; +const CANONICAL_POWERSHELL_SCRIPT_PREFIX: &str = "__codex_powershell_script__"; + +/// Canonicalize command argv for approval-cache matching. +/// +/// This keeps approval decisions stable across wrapper-path differences (for +/// example `/bin/bash -lc` vs `bash -lc`) and across shell wrapper tools while +/// preserving exact script text for complex scripts where we cannot safely +/// recover a tokenized command sequence. +pub(crate) fn canonicalize_command_for_approval(command: &[String]) -> Vec { + if let Some(commands) = parse_shell_lc_plain_commands(command) + && let [single_command] = commands.as_slice() + { + return single_command.clone(); + } + + if let Some((_shell, script)) = extract_bash_command(command) { + let shell_mode = command.get(1).cloned().unwrap_or_default(); + return vec![ + CANONICAL_BASH_SCRIPT_PREFIX.to_string(), + shell_mode, + script.to_string(), + ]; + } + + if let Some((_shell, script)) = extract_powershell_command(command) { + return vec![ + CANONICAL_POWERSHELL_SCRIPT_PREFIX.to_string(), + script.to_string(), + ]; + } + + command.to_vec() +} + +#[cfg(test)] +mod tests { + use super::canonicalize_command_for_approval; + use pretty_assertions::assert_eq; + + #[test] + fn canonicalizes_word_only_shell_scripts_to_inner_command() { + let command_a = vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "cargo test -p codex-core".to_string(), + ]; + let command_b = vec![ + "bash".to_string(), + "-lc".to_string(), + "cargo test -p codex-core".to_string(), + ]; + + assert_eq!( + canonicalize_command_for_approval(&command_a), + vec![ + "cargo".to_string(), + "test".to_string(), + "-p".to_string(), + "codex-core".to_string(), + ] + ); + assert_eq!( + canonicalize_command_for_approval(&command_a), + canonicalize_command_for_approval(&command_b) + ); + } + + #[test] + fn canonicalizes_heredoc_scripts_to_stable_script_key() { + let script = "python3 <<'PY'\nprint('hello')\nPY"; + let command_a = vec![ + "/bin/zsh".to_string(), + "-lc".to_string(), + script.to_string(), + ]; + let command_b = vec!["zsh".to_string(), "-lc".to_string(), script.to_string()]; + + assert_eq!( + canonicalize_command_for_approval(&command_a), + vec![ + "__codex_shell_script__".to_string(), + "-lc".to_string(), + script.to_string(), + ] + ); + assert_eq!( + canonicalize_command_for_approval(&command_a), + canonicalize_command_for_approval(&command_b) + ); + } + + #[test] + fn canonicalizes_powershell_wrappers_to_stable_script_key() { + let script = "Write-Host hi"; + let command_a = vec![ + "powershell.exe".to_string(), + "-NoProfile".to_string(), + "-Command".to_string(), + script.to_string(), + ]; + let command_b = vec![ + "powershell".to_string(), + "-Command".to_string(), + script.to_string(), + ]; + + assert_eq!( + canonicalize_command_for_approval(&command_a), + vec![ + "__codex_powershell_script__".to_string(), + script.to_string(), + ] + ); + assert_eq!( + canonicalize_command_for_approval(&command_a), + canonicalize_command_for_approval(&command_b) + ); + } + + #[test] + fn preserves_non_shell_commands() { + let command = vec!["cargo".to_string(), "fmt".to_string()]; + assert_eq!(canonicalize_command_for_approval(&command), command); + } +} diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 5e73fb0735a..514b4f0af59 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -25,6 +25,7 @@ use tokio::fs; use tokio::task::spawn_blocking; use crate::bash::parse_shell_lc_plain_commands; +use crate::bash::parse_shell_lc_single_command_prefix; use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ExecApprovalRequirement; use shlex::try_join as shlex_try_join; @@ -121,8 +122,11 @@ impl ExecPolicyManager { prefix_rule, } = req; let exec_policy = self.current(); - let commands = - parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]); + let (commands, used_heredoc_fallback) = commands_for_exec_policy(command); + // Keep heredoc prefix parsing for rule evaluation so existing + // allow/prompt/forbidden rules still apply, but avoid auto-derived + // amendments when only the heredoc fallback parser matched. + let auto_amendment_allowed = !used_heredoc_fallback; let exec_policy_fallback = |cmd: &[String]| { render_decision_for_unmatched_command( approval_policy, @@ -149,9 +153,13 @@ impl ExecPolicyManager { ExecApprovalRequirement::NeedsApproval { reason: derive_prompt_reason(command, &evaluation), proposed_execpolicy_amendment: requested_amendment.or_else(|| { - try_derive_execpolicy_amendment_for_prompt_rules( - &evaluation.matched_rules, - ) + if auto_amendment_allowed { + try_derive_execpolicy_amendment_for_prompt_rules( + &evaluation.matched_rules, + ) + } else { + None + } }), } } @@ -161,9 +169,11 @@ impl ExecPolicyManager { bypass_sandbox: evaluation.matched_rules.iter().any(|rule_match| { is_policy_match(rule_match) && rule_match.decision() == Decision::Allow }), - proposed_execpolicy_amendment: try_derive_execpolicy_amendment_for_allow_rules( - &evaluation.matched_rules, - ), + proposed_execpolicy_amendment: if auto_amendment_allowed { + try_derive_execpolicy_amendment_for_allow_rules(&evaluation.matched_rules) + } else { + None + }, }, } } @@ -334,6 +344,18 @@ fn default_policy_path(codex_home: &Path) -> PathBuf { codex_home.join(RULES_DIR_NAME).join(DEFAULT_POLICY_FILE) } +fn commands_for_exec_policy(command: &[String]) -> (Vec>, bool) { + if let Some(commands) = parse_shell_lc_plain_commands(command) { + return (commands, false); + } + + if let Some(single_command) = parse_shell_lc_single_command_prefix(command) { + return (vec![single_command], true); + } + + (vec![command.to_vec()], false) +} + /// Derive a proposed execpolicy amendment when a command requires user approval /// - If any execpolicy rule prompts, return None, because an amendment would not skip that policy requirement. /// - Otherwise return the first heuristics Prompt. @@ -792,6 +814,94 @@ prefix_rule(pattern=["rm"], decision="forbidden") ); } + #[tokio::test] + async fn evaluates_heredoc_script_against_prefix_rules() { + let policy_src = r#"prefix_rule(pattern=["python3"], decision="allow")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.rules", policy_src) + .expect("parse policy"); + let policy = Arc::new(parser.build()); + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 <<'PY'\nprint('hello')\nPY".to_string(), + ]; + + let requirement = ExecPolicyManager::new(policy) + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::OnRequest, + sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::Skip { + bypass_sandbox: true, + proposed_execpolicy_amendment: None, + } + ); + } + + #[tokio::test] + async fn omits_auto_amendment_for_heredoc_fallback_prompts() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 <<'PY'\nprint('hello')\nPY".to_string(), + ]; + + let requirement = ExecPolicyManager::default() + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + ); + } + + #[tokio::test] + async fn keeps_requested_amendment_for_heredoc_fallback_prompts() { + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "python3 <<'PY'\nprint('hello')\nPY".to_string(), + ]; + let requested_prefix = vec!["python3".to_string(), "-m".to_string(), "pip".to_string()]; + + let requirement = ExecPolicyManager::default() + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::UnlessTrusted, + sandbox_policy: &SandboxPolicy::ReadOnly, + sandbox_permissions: SandboxPermissions::UseDefault, + prefix_rule: Some(requested_prefix.clone()), + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(requested_prefix)), + } + ); + } + #[tokio::test] async fn justification_is_included_in_forbidden_exec_approval_requirement() { let policy_src = r#" diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7378fe3e6f7..3713c8892e5 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -21,6 +21,7 @@ pub use codex_thread::CodexThread; pub use codex_thread::ThreadConfigSnapshot; mod agent; mod codex_delegate; +mod command_canonicalization; mod command_safety; pub mod config; pub mod config_loader; diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index f5e1ea17e4b..fd076f005e7 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -4,6 +4,7 @@ Runtime: shell Executes shell requests under the orchestrator: asks for approval when needed, builds a CommandSpec, and runs it under the current SandboxAttempt. */ +use crate::command_canonicalization::canonicalize_command_for_approval; use crate::exec::ExecToolCallOutput; use crate::features::Feature; use crate::powershell::prefix_powershell_script_with_utf8; @@ -78,7 +79,7 @@ impl Approvable for ShellRuntime { fn approval_keys(&self, req: &ShellRequest) -> Vec { vec![ApprovalKey { - command: req.command.clone(), + command: canonicalize_command_for_approval(&req.command), cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, }] diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index f75c8f75728..f3bd2e64b19 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -4,6 +4,7 @@ Runtime: unified exec Handles approval + sandbox orchestration for unified exec requests, delegating to the process manager to spawn PTYs once an ExecRequest is prepared. */ +use crate::command_canonicalization::canonicalize_command_for_approval; use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecExpiration; @@ -78,7 +79,7 @@ impl Approvable for UnifiedExecRuntime<'_> { fn approval_keys(&self, req: &UnifiedExecRequest) -> Vec { vec![UnifiedExecApprovalKey { - command: req.command.clone(), + command: canonicalize_command_for_approval(&req.command), cwd: req.cwd.clone(), tty: req.tty, sandbox_permissions: req.sandbox_permissions,