fix(core): canonicalize wrapper approvals and support heredoc prefix … (#10941)

## Summary
- Reduced repeated approvals for equivalent wrapper commands and fixed
execpolicy matching for heredoc-style shell invocations, with minimal
behavior change and fail-closed defaults.

## Fixes
1. Canonicalized approval matching for wrappers so equivalent commands
map to the same approval intent.
2. Added heredoc-aware prefix extraction for execpolicy so commands like
`python3 <<'PY' ... PY` match rules such as `prefix_rule(["python3"],
...)`.
3. Kept fallback behavior conservative: if parsing is ambiguous,
existing prompt behavior is preserved.

## Edge Cases Covered
- Wrapper path/name differences: `/bin/bash` vs `bash`, `/bin/zsh` vs
`zsh`.
- Shell modes: `-c` and `-lc`.
- Heredoc forms: quoted delimiter (`<<'PY'`) and unquoted delimiter (`<<
PY`).
- Multi-command heredoc scripts are rejected by the fallback
- Non-heredoc redirections (`>`, etc.) are not treated as heredoc prefix
matches.
- Complex scripts still fall back to prior behavior rather than
expanding permissions.

---------

Co-authored-by: Dylan Hurd <dylan.hurd@openai.com>
This commit is contained in:
viyatb-oai
2026-02-10 11:46:40 -08:00
committed by GitHub
parent e4b5384539
commit 62d0f302fd
6 changed files with 440 additions and 10 deletions

View File

@@ -119,6 +119,23 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option<Vec<Vec<Strin
try_parse_word_only_commands_sequence(&tree, script)
}
/// Returns the parsed argv for a single shell command in a here-doc style
/// script (`<<`), as long as the script contains exactly one command node.
pub fn parse_shell_lc_single_command_prefix(command: &[String]) -> Option<Vec<String>> {
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<Vec<String>> {
if cmd.kind() != "command" {
return None;
@@ -177,6 +194,94 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
Some(words)
}
fn parse_heredoc_command_words(cmd: Node<'_>, src: &str) -> Option<Vec<String>> {
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<Node<'_>> {
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<String> {
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);
}
}

View File

@@ -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<String> {
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);
}
}

View File

@@ -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<Vec<String>>, 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#"

View File

@@ -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;

View File

@@ -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<ShellRequest> for ShellRuntime {
fn approval_keys(&self, req: &ShellRequest) -> Vec<Self::ApprovalKey> {
vec![ApprovalKey {
command: req.command.clone(),
command: canonicalize_command_for_approval(&req.command),
cwd: req.cwd.clone(),
sandbox_permissions: req.sandbox_permissions,
}]

View File

@@ -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<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
fn approval_keys(&self, req: &UnifiedExecRequest) -> Vec<Self::ApprovalKey> {
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,