Fix execpolicy parsing for multiline quoted args#9565
Conversation
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
codex-rs/core/src/bash.rs
Outdated
| try_parse_word_only_commands_sequence(&tree, script) | ||
| } | ||
|
|
||
| fn parse_double_quoted_string(node: Node, src: &str) -> Option<String> { |
There was a problem hiding this comment.
We generally declare utility support functions below public/primary library functions (particularly if they are well-named), so can you please move this down?
|
|
||
| #[test] | ||
| fn accepts_double_quoted_strings_with_newlines() { | ||
| let cmds = parse_seq("git commit -m \"line1\nline2\"").unwrap(); |
There was a problem hiding this comment.
parse_double_quoted_string() could use more tests. I would like to verify that things like env vars embedded in double quoted strings are rejected.
I would also like to see if this covers examples like:
"/usr"'/'"local"/bin
where mixed quote types are used.
codex-rs/protocol/src/num_format.rs
Outdated
| /// for en-US). | ||
| pub fn format_with_separators(n: i64) -> String { | ||
| formatter().format(&Decimal::from(n)).to_string() | ||
| format_with_separators_with_formatter(n, formatter()) |
There was a problem hiding this comment.
This is unrelated to the bash.rs change, no?
|
Addressed review feedback: moved the quote-parsing helpers below the public entry points, added tests for double-quoted expansions and mixed-quote concatenation, and dropped the unrelated num_format change. |
|
FYI, I made some small fixes in #9745 that I didn't think were worth another round of code review. |
3 participants
What
Fix bash command parsing to accept double-quoted strings that contain literal newlines so execpolicy can match allow rules.
Why
Allow rules like [git, commit] should still match when commit messages include a newline in a quoted argument; the parser currently rejects these strings and falls back to the outer shell invocation.
How
Fixes #9541.
Tests