Fix execpolicy parsing for multiline quoted args (#9565)

## 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
- Validate double-quoted strings by ensuring all named children are
string_content and then stripping the outer quotes from the raw node
text so embedded newlines are preserved.
- Reuse the helper for concatenated arguments.
- Ensure large SI suffix formatting uses the caller-provided locale
formatter for grouping.
- Add coverage for newline-containing quoted arguments.

Fixes #9541.

## Tests
- cargo test -p codex-core
- just fix -p codex-core
- cargo test -p codex-protocol
- just fix -p codex-protocol
- cargo test --all-features
This commit is contained in:
JUAN DAVID SALAS CAMARGO
2026-01-23 01:16:53 -05:00
committed by GitHub
Unverified
parent 0fa45fbca4
commit f815fa14ea
2 changed files with 71 additions and 39 deletions
+66 -38
View File
@@ -138,26 +138,12 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
words.push(child.utf8_text(src.as_bytes()).ok()?.to_owned());
}
"string" => {
if child.child_count() == 3
&& child.child(0)?.kind() == "\""
&& child.child(1)?.kind() == "string_content"
&& child.child(2)?.kind() == "\""
{
words.push(child.child(1)?.utf8_text(src.as_bytes()).ok()?.to_owned());
} else {
return None;
}
let parsed = parse_double_quoted_string(child, src)?;
words.push(parsed);
}
"raw_string" => {
let raw_string = child.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''));
if let Some(s) = stripped {
words.push(s.to_owned());
} else {
return None;
}
let parsed = parse_raw_string(child, src)?;
words.push(parsed);
}
"concatenation" => {
// Handle concatenated arguments like -g"*.py"
@@ -170,28 +156,12 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
.push_str(part.utf8_text(src.as_bytes()).ok()?.to_owned().as_str());
}
"string" => {
if part.child_count() == 3
&& part.child(0)?.kind() == "\""
&& part.child(1)?.kind() == "string_content"
&& part.child(2)?.kind() == "\""
{
concatenated.push_str(
part.child(1)?
.utf8_text(src.as_bytes())
.ok()?
.to_owned()
.as_str(),
);
} else {
return None;
}
let parsed = parse_double_quoted_string(part, src)?;
concatenated.push_str(&parsed);
}
"raw_string" => {
let raw_string = part.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''))?;
concatenated.push_str(stripped);
let parsed = parse_raw_string(part, src)?;
concatenated.push_str(&parsed);
}
_ => return None,
}
@@ -207,9 +177,38 @@ fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option<Ve
Some(words)
}
fn parse_double_quoted_string(node: Node, src: &str) -> Option<String> {
if node.kind() != "string" {
return None;
}
let mut cursor = node.walk();
for part in node.named_children(&mut cursor) {
if part.kind() != "string_content" {
return None;
}
}
let raw = node.utf8_text(src.as_bytes()).ok()?;
let stripped = raw
.strip_prefix('"')
.and_then(|text| text.strip_suffix('"'))?;
Some(stripped.to_string())
}
fn parse_raw_string(node: Node, src: &str) -> Option<String> {
if node.kind() != "raw_string" {
return None;
}
let raw_string = node.utf8_text(src.as_bytes()).ok()?;
let stripped = raw_string
.strip_prefix('\'')
.and_then(|s| s.strip_suffix('\''));
stripped.map(str::to_owned)
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
fn parse_seq(src: &str) -> Option<Vec<Vec<String>>> {
let tree = try_parse_shell(src)?;
@@ -250,6 +249,35 @@ mod tests {
);
}
#[test]
fn accepts_double_quoted_strings_with_newlines() {
let cmds = parse_seq("git commit -m \"line1\nline2\"").unwrap();
assert_eq!(
cmds,
vec![vec![
"git".to_string(),
"commit".to_string(),
"-m".to_string(),
"line1\nline2".to_string(),
]]
);
}
#[test]
fn accepts_mixed_quote_concatenation() {
let cmds = parse_seq("echo \"/usr\"'/'\"local\"/bin").unwrap();
assert_eq!(
cmds,
vec![vec!["echo".to_string(), "/usr/local/bin".to_string()]]
);
}
#[test]
fn rejects_double_quoted_strings_with_expansions() {
assert!(parse_seq("echo \"hi ${USER}\"").is_none());
assert!(parse_seq("echo \"$HOME\"").is_none());
}
#[test]
fn accepts_numbers_as_words() {
let cmds = parse_seq("echo 123 456").unwrap();
+5 -1
View File
@@ -28,6 +28,10 @@ pub fn format_with_separators(n: i64) -> String {
formatter().format(&Decimal::from(n)).to_string()
}
fn format_with_separators_with_formatter(n: i64, formatter: &DecimalFormatter) -> String {
formatter.format(&Decimal::from(n)).to_string()
}
fn format_si_suffix_with_formatter(n: i64, formatter: &DecimalFormatter) -> String {
let n = n.max(0);
if n < 1000 {
@@ -58,7 +62,7 @@ fn format_si_suffix_with_formatter(n: i64, formatter: &DecimalFormatter) -> Stri
// Above 1000G, keep wholeG precision.
format!(
"{}G",
format_with_separators(((n as f64) / 1e9).round() as i64)
format_with_separators_with_formatter(((n as f64) / 1e9).round() as i64, formatter)
)
}