clippy

codex-rs/core/src/bash.rs

@@ -1,5 +1,6 @@
 use std::path::PathBuf;
 
+use shlex::split as shlex_split;
 use tree_sitter::Node;
 use tree_sitter::Parser;
 use tree_sitter::Tree;
@@ -119,6 +120,83 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option<Vec<Vec<Strin
     try_parse_word_only_commands_sequence(&tree, script)
 }
 
+/// Returns a best-effort sequence of command prefixes for execpolicy evaluation.
+///
+/// This intentionally falls back to a looser split than `parse_shell_lc_plain_commands`,
+/// but refuses to parse when the fallback line contains shell substitutions.
+pub fn parse_shell_lc_execpolicy_prefix(command: &[String]) -> Option<Vec<Vec<String>>> {
+    if let Some(commands) = parse_shell_lc_plain_commands(command)
+        && !commands.is_empty()
+    {
+        return Some(commands);
+    }
+
+    let (_, script) = extract_bash_command(command)?;
+    let tokens = if let Some(tokens) = shlex_split(script) {
+        tokens
+    } else {
+        let first_line = script.lines().next().unwrap_or_default();
+        if first_line.is_empty() || contains_shell_substitution(first_line) {
+            return None;
+        }
+        shlex_split(first_line)?
+    };
+    let mut commands = Vec::new();
+    let mut current = Vec::new();
+
+    for token in tokens {
+        if is_shell_separator(&token) {
+            if let Some(command_tokens) = finalize_execpolicy_tokens(&mut current) {
+                commands.push(command_tokens);
+            }
+            continue;
+        }
+        current.push(token);
+    }
+
+    if let Some(command_tokens) = finalize_execpolicy_tokens(&mut current) {
+        commands.push(command_tokens);
+    }
+
+    if commands.is_empty() {
+        None
+    } else {
+        Some(commands)
+    }
+}
+
+fn finalize_execpolicy_tokens(tokens: &mut Vec<String>) -> Option<Vec<String>> {
+    if tokens.is_empty() {
+        return None;
+    }
+
+    let mut command_tokens = Vec::new();
+    for token in tokens.iter() {
+        if is_redirection_token(token) {
+            break;
+        }
+        command_tokens.push(token.clone());
+    }
+    tokens.clear();
+    if command_tokens.is_empty() {
+        None
+    } else {
+        Some(command_tokens)
+    }
+}
+
+fn is_shell_separator(token: &str) -> bool {
+    matches!(token, "&&" | "||" | ";" | "|")
+}
+
+fn is_redirection_token(token: &str) -> bool {
+    token.contains('<') || token.contains('>')
+}
+
+fn contains_shell_substitution(line: &str) -> bool {
+    line.chars().any(|c| matches!(c, '$' | '`' | '(' | ')'))
+}
+
 fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Vec<String>> {
     if cmd.kind() != "command" {
         return None;
@@ -217,6 +295,17 @@ mod tests {
         try_parse_word_only_commands_sequence(&tree, src)
     }
 
+    #[test]
+    fn parse_shell_lc_execpolicy_prefix_rejects_substitution_chars() {
+        let command = vec![
+            "bash".to_string(),
+            "-lc".to_string(),
+            "python3 $(echo hi) <<'PY'\nprint('hi)\nPY".to_string(),
+        ];
+
+        assert!(parse_shell_lc_execpolicy_prefix(&command).is_none());
+    }
+
     #[test]
     fn accepts_single_simple_command() {
         let cmds = parse_seq("ls -1").unwrap();

codex-rs/core/src/exec_policy.rs

@@ -24,7 +24,7 @@ use thiserror::Error;
 use tokio::fs;
 use tokio::task::spawn_blocking;
 
-use crate::bash::parse_shell_lc_plain_commands;
+use crate::bash::parse_shell_lc_execpolicy_prefix;
 use crate::features::Feature;
 use crate::features::Features;
 use crate::sandboxing::SandboxPermissions;
@@ -133,7 +133,7 @@ impl ExecPolicyManager {
         } = req;
         let exec_policy = self.current();
         let commands =
-            parse_shell_lc_plain_commands(command).unwrap_or_else(|| vec![command.to_vec()]);
+            parse_shell_lc_execpolicy_prefix(command).unwrap_or_else(|| vec![command.to_vec()]);
         let exec_policy_fallback = |cmd: &[String]| {
             render_decision_for_unmatched_command(
                 approval_policy,

codex-rs/core/tests/suite/approvals.rs

@@ -1849,3 +1849,131 @@ async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts
 
     Ok(())
 }
+
+#[tokio::test(flavor = "current_thread")]
+#[cfg(unix)]
+async fn heredoc_execpolicy_amendment_skips_future_approvals() -> Result<()> {
+    let server = start_mock_server().await;
+    let approval_policy = AskForApproval::UnlessTrusted;
+    let sandbox_policy = SandboxPolicy::ReadOnly;
+    let sandbox_policy_for_config = sandbox_policy.clone();
+    let mut builder = test_codex().with_config(move |config| {
+        config.approval_policy = Constrained::allow_any(approval_policy);
+        config.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
+    });
+    let test = builder.build(&server).await?;
+
+    let command = r#"python3 <<'PY'
+print("hello")
+PY"#;
+
+    let call_id_first = "heredoc-allow-first";
+    let args_first = json!({
+        "command": command,
+        "timeout_ms": 1_000,
+        "prefix_rule": ["python3"],
+    });
+    let first_event = ev_function_call(
+        call_id_first,
+        "shell_command",
+        &serde_json::to_string(&args_first)?,
+    );
+
+    let _ = mount_sse_once(
+        &server,
+        sse(vec![
+            ev_response_created("resp-heredoc-1"),
+            first_event,
+            ev_completed("resp-heredoc-1"),
+        ]),
+    )
+    .await;
+    let _first_results = mount_sse_once(
+        &server,
+        sse(vec![
+            ev_assistant_message("msg-heredoc-1", "done"),
+            ev_completed("resp-heredoc-2"),
+        ]),
+    )
+    .await;
+
+    submit_turn(
+        &test,
+        "heredoc allow first",
+        approval_policy,
+        sandbox_policy.clone(),
+    )
+    .await?;
+
+    let approval = expect_exec_approval(&test, command).await;
+    let expected_amendment = ExecPolicyAmendment::new(vec!["python3".to_string()]);
+    assert_eq!(
+        approval.proposed_execpolicy_amendment,
+        Some(expected_amendment.clone())
+    );
+
+    test.codex
+        .submit(Op::ExecApproval {
+            id: "0".into(),
+            decision: ReviewDecision::ApprovedExecpolicyAmendment {
+                proposed_execpolicy_amendment: expected_amendment,
+            },
+        })
+        .await?;
+    wait_for_completion(&test).await;
+
+    let policy_path = test.home.path().join("rules").join("default.rules");
+    let policy_contents = fs::read_to_string(&policy_path)?;
+    assert!(
+        policy_contents.contains(r#"prefix_rule(pattern=["python3"], decision="allow")"#),
+        "unexpected policy contents: {policy_contents}"
+    );
+
+    let call_id_second = "heredoc-allow-second";
+    let args_second = json!({
+        "command": command,
+        "timeout_ms": 1_000,
+    });
+    let second_event = ev_function_call(
+        call_id_second,
+        "shell_command",
+        &serde_json::to_string(&args_second)?,
+    );
+
+    let _ = mount_sse_once(
+        &server,
+        sse(vec![
+            ev_response_created("resp-heredoc-3"),
+            second_event,
+            ev_completed("resp-heredoc-3"),
+        ]),
+    )
+    .await;
+    let second_results = mount_sse_once(
+        &server,
+        sse(vec![
+            ev_assistant_message("msg-heredoc-2", "done"),
+            ev_completed("resp-heredoc-4"),
+        ]),
+    )
+    .await;
+
+    submit_turn(
+        &test,
+        "heredoc allow second",
+        approval_policy,
+        sandbox_policy.clone(),
+    )
+    .await?;
+
+    wait_for_completion_without_approval(&test).await;
+
+    let second_output = parse_result(
+        &second_results
+            .single_request()
+            .function_call_output(call_id_second),
+    );
+    assert_eq!(second_output.exit_code.unwrap_or(0), 0);
+
+    Ok(())
+}