fix(core): canonicalize wrapper approvals and support heredoc prefix …#10941
fix(core): canonicalize wrapper approvals and support heredoc prefix …#10941viyatb-oai wants to merge 8 commits intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3964f175be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "word" | "number" => { | ||
| 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. |
There was a problem hiding this comment.
<< scripts don’t bypass approvals
This parser accepts any word nodes verbatim, and parse_shell_lc_single_command_prefix is gated only by script.contains("<<"). That combination means scripts that merely contain << (e.g., arithmetic shift echo $((1<<2)) or a here‑string with command substitution like python3 <<< "$(rm -rf /)") can be reduced to a simple argv prefix and matched by prefix_rule, skipping approvals that previously triggered when word‑only parsing failed. This is a regression in safety: << in non‑heredoc contexts can now be used to bypass the intended “complex script” fallback. Consider verifying that the AST actually contains a heredoc redirect, and/or rejecting words that include expansions before returning a prefix.
Useful? React with 👍 / 👎.
…cy-heredoc-canonicalization # Conflicts: # codex-rs/core/src/exec_policy.rs
| 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); | ||
| let auto_amendment_allowed = !used_heredoc_fallback; |
There was a problem hiding this comment.
cc @bolinfest for a bit of nuance here, but this feels reasonable to me
There was a problem hiding this comment.
I'll add a comment here so that its explicit whats happening.
2 participants
Summary
Fixes
python3 <<'PY' ... PYmatch rules such asprefix_rule(["python3"], ...).Edge Cases Covered
/bin/bashvsbash,/bin/zshvszsh.-cand-lc.<<'PY') and unquoted delimiter (<< PY).>, etc.) are not treated as heredoc prefix matches.