From a963dd6a7bd1185aea72de7abd2434e734f93089 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 13:12:33 +0000 Subject: [PATCH 1/5] fix: use brush_parser::word::parse for context-aware shell unquoting The previous `unquote_str` function naively stripped all quote characters regardless of context, causing single quotes inside double-quoted strings to be removed. This corrupted arguments like: node -e "require('child_process').execSync('vp config')" where 'child_process' would lose its quotes, breaking the JavaScript. Replace with `brush_parser::word::parse` which properly parses word pieces with quoting context, then flatten the pieces to extract literal text. This correctly preserves single quotes inside double quotes and vice versa. Fixes running `prepare` script from packages like node-modules/urllib. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --- crates/vite_shell/src/lib.rs | 103 +++++++++++++++++++++++++++++------ 1 file changed, 87 insertions(+), 16 deletions(-) diff --git a/crates/vite_shell/src/lib.rs b/crates/vite_shell/src/lib.rs index 8baca49c..2433e7cd 100644 --- a/crates/vite_shell/src/lib.rs +++ b/crates/vite_shell/src/lib.rs @@ -8,7 +8,7 @@ use brush_parser::{ CommandPrefixOrSuffixItem, CommandSuffix, CompoundListItem, Pipeline, Program, SeparatorOperator, SimpleCommand, SourceLocation, Word, }, - unquote_str, + word::{WordPiece, WordPieceWithSource}, }; use diff::Diff; use serde::{Deserialize, Serialize}; @@ -42,10 +42,54 @@ impl Display for TaskParsedCommand { } } -#[expect(clippy::disallowed_types, reason = "brush_parser::unquote_str returns String")] -fn unquote(word: &Word) -> String { +/// Parser options matching those used in [`try_parse_as_and_list`]. +const PARSER_OPTIONS: ParserOptions = ParserOptions { + enable_extended_globbing: false, + posix_mode: true, + sh_mode: true, + tilde_expansion: false, +}; + +/// Remove shell quoting from a word value, respecting quoting context. +/// +/// Uses `brush_parser::word::parse` to properly handle nested quoting +/// (e.g. single quotes inside double quotes are preserved as literal characters). +/// Returns `None` if the word contains expansions that cannot be statically resolved +/// (parameter expansion, command substitution, arithmetic). +#[expect(clippy::disallowed_types, reason = "brush_parser word API uses String")] +fn unquote(word: &Word) -> Option { let Word { value, loc: _ } = word; - unquote_str(value.as_str()) + let pieces = brush_parser::word::parse(value.as_str(), &PARSER_OPTIONS).ok()?; + let mut result = String::with_capacity(value.len()); + flatten_pieces(&pieces, &mut result)?; + Some(result) +} + +/// Recursively extract literal text from parsed word pieces. +/// +/// Returns `None` if any piece requires runtime expansion. +#[expect(clippy::disallowed_types, reason = "brush_parser word API uses String")] +fn flatten_pieces(pieces: &[WordPieceWithSource], result: &mut String) -> Option<()> { + for piece in pieces { + match &piece.piece { + WordPiece::Text(s) | WordPiece::SingleQuotedText(s) | WordPiece::AnsiCQuotedText(s) => { + result.push_str(s) + } + // EscapeSequence contains the raw sequence (e.g. `\"` as two chars); + // the escaped character is everything after the leading backslash. + WordPiece::EscapeSequence(s) => { + result.push_str(s.strip_prefix('\\').unwrap_or(s)); + } + WordPiece::DoubleQuotedSequence(inner) + | WordPiece::GettextDoubleQuotedSequence(inner) => { + flatten_pieces(inner, result)?; + } + // Tilde prefix, parameter expansion, command substitution, arithmetic + // cannot be statically resolved — bail out. + _ => return None, + } + } + Some(()) } fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range)> { @@ -78,7 +122,7 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range< let AssignmentValue::Scalar(value) = value else { return None; }; - envs.insert(name.as_str().into(), unquote(value).into()); + envs.insert(name.as_str().into(), unquote(value)?.into()); } } let mut args = Vec::::new(); @@ -87,23 +131,15 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range< let CommandPrefixOrSuffixItem::Word(word) = suffix_item else { return None; }; - args.push(unquote(word).into()); + args.push(unquote(word)?.into()); } } - Some((TaskParsedCommand { envs, program: unquote(program).into(), args }, range)) + Some((TaskParsedCommand { envs, program: unquote(program)?.into(), args }, range)) } #[must_use] pub fn try_parse_as_and_list(cmd: &str) -> Option)>> { - let mut parser = Parser::new( - cmd.as_bytes(), - &ParserOptions { - enable_extended_globbing: false, - posix_mode: true, - sh_mode: true, - tilde_expansion: false, - }, - ); + let mut parser = Parser::new(cmd.as_bytes(), &PARSER_OPTIONS); let Program { complete_commands } = parser.parse_program().ok()?; let [compound_list] = complete_commands.as_slice() else { return None; @@ -202,6 +238,41 @@ mod tests { assert!(str1.starts_with("ALPHA=first MIDDLE=middle ZEBRA=last")); } + #[test] + fn test_unquote_preserves_nested_quotes() { + // Single quotes inside double quotes are preserved + let cmd = r#"echo "hello 'world'""#; + let list = try_parse_as_and_list(cmd).unwrap(); + assert_eq!(list[0].0.args[0].as_str(), "hello 'world'"); + + // Double quotes inside single quotes are preserved + let cmd = r#"echo 'hello "world"'"#; + let list = try_parse_as_and_list(cmd).unwrap(); + assert_eq!(list[0].0.args[0].as_str(), "hello \"world\""); + + // Backslash escaping in double quotes + let cmd = r#"echo "hello\"world""#; + let list = try_parse_as_and_list(cmd).unwrap(); + assert_eq!(list[0].0.args[0].as_str(), "hello\"world"); + + // Backslash escaping outside quotes + let cmd = r"echo hello\ world"; + let list = try_parse_as_and_list(cmd).unwrap(); + assert_eq!(list[0].0.args[0].as_str(), "hello world"); + } + + #[test] + fn test_parse_urllib_prepare() { + let cmd = r#"node -e "const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});""#; + let result = try_parse_as_and_list(cmd); + let (parsed, _) = &result.as_ref().unwrap()[0]; + // Single quotes inside double quotes must be preserved as literal characters + assert_eq!( + parsed.args[1].as_str(), + "const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});" + ); + } + #[test] fn test_task_parsed_command_serialization_stability() { use bincode::{decode_from_slice, encode_to_vec}; From 5dab544814075a9194fe29f18e11594964af6fe8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 13:30:48 +0000 Subject: [PATCH 2/5] test: add unit test for recursive flatten_pieces through word::parse Tests DoubleQuotedSequence recursion with escape sequences, literal single quotes, and bail-out on parameter expansion / command substitution. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --- crates/vite_shell/src/lib.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crates/vite_shell/src/lib.rs b/crates/vite_shell/src/lib.rs index 2433e7cd..14ce1e40 100644 --- a/crates/vite_shell/src/lib.rs +++ b/crates/vite_shell/src/lib.rs @@ -261,6 +261,31 @@ mod tests { assert_eq!(list[0].0.args[0].as_str(), "hello world"); } + #[test] + fn test_flatten_pieces_recursion() { + fn parse_and_flatten(input: &str) -> Option { + let pieces = brush_parser::word::parse(input, &PARSER_OPTIONS).ok()?; + let mut result = String::new(); + flatten_pieces(&pieces, &mut result)?; + Some(result) + } + + // DoubleQuotedSequence containing Text + EscapeSequence + Text + assert_eq!(parse_and_flatten(r#""hello\"world""#).unwrap(), "hello\"world"); + + // DoubleQuotedSequence with single quotes preserved as literal text + assert_eq!(parse_and_flatten(r#""it's a 'test'""#).unwrap(), "it's a 'test'"); + + // Nested escape sequences inside double quotes + assert_eq!(parse_and_flatten(r#""a\\b""#).unwrap(), "a\\b"); + + // DoubleQuotedSequence bails on parameter expansion inside + assert!(parse_and_flatten(r#""hello $VAR""#).is_none()); + + // DoubleQuotedSequence bails on command substitution inside + assert!(parse_and_flatten(r#""hello $(cmd)""#).is_none()); + } + #[test] fn test_parse_urllib_prepare() { let cmd = r#"node -e "const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});""#; From 75c1b9731a52f08ab8c1ab85bcec6ee25529320b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 13:36:24 +0000 Subject: [PATCH 3/5] fix: add missing semicolon to satisfy clippy pedantic lint https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --- crates/vite_shell/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vite_shell/src/lib.rs b/crates/vite_shell/src/lib.rs index 14ce1e40..fea41147 100644 --- a/crates/vite_shell/src/lib.rs +++ b/crates/vite_shell/src/lib.rs @@ -73,7 +73,7 @@ fn flatten_pieces(pieces: &[WordPieceWithSource], result: &mut String) -> Option for piece in pieces { match &piece.piece { WordPiece::Text(s) | WordPiece::SingleQuotedText(s) | WordPiece::AnsiCQuotedText(s) => { - result.push_str(s) + result.push_str(s); } // EscapeSequence contains the raw sequence (e.g. `\"` as two chars); // the escaped character is everything after the leading backslash. From 9efe2c12d8682a67b866488464de248372d898c5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 14:34:55 +0000 Subject: [PATCH 4/5] fix: suppress disallowed_types clippy lint in test helper using String The test helper `parse_and_flatten` necessarily uses `String` because the underlying `flatten_pieces` function takes `&mut String`. Added `#[expect]` attributes matching the pattern already used in production code. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --- crates/vite_shell/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/vite_shell/src/lib.rs b/crates/vite_shell/src/lib.rs index fea41147..6e12b26e 100644 --- a/crates/vite_shell/src/lib.rs +++ b/crates/vite_shell/src/lib.rs @@ -263,8 +263,10 @@ mod tests { #[test] fn test_flatten_pieces_recursion() { + #[expect(clippy::disallowed_types, reason = "flatten_pieces uses String")] fn parse_and_flatten(input: &str) -> Option { let pieces = brush_parser::word::parse(input, &PARSER_OPTIONS).ok()?; + #[expect(clippy::disallowed_types, reason = "flatten_pieces uses String")] let mut result = String::new(); flatten_pieces(&pieces, &mut result)?; Some(result) From 75152872d90a11f7dd660588cf2a0bbec638381e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 10 Mar 2026 14:38:46 +0000 Subject: [PATCH 5/5] fix: use Str instead of String in unquote/flatten_pieces Replace disallowed String type with vite_str::Str in unquote() and flatten_pieces(), removing the need for #[expect(clippy::disallowed_types)] attributes and redundant .into() conversions at call sites. https://claude.ai/code/session_01SoJXo78ET9sowKTSdAuFAg --- crates/vite_shell/src/lib.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/crates/vite_shell/src/lib.rs b/crates/vite_shell/src/lib.rs index 6e12b26e..2d768997 100644 --- a/crates/vite_shell/src/lib.rs +++ b/crates/vite_shell/src/lib.rs @@ -56,11 +56,10 @@ const PARSER_OPTIONS: ParserOptions = ParserOptions { /// (e.g. single quotes inside double quotes are preserved as literal characters). /// Returns `None` if the word contains expansions that cannot be statically resolved /// (parameter expansion, command substitution, arithmetic). -#[expect(clippy::disallowed_types, reason = "brush_parser word API uses String")] -fn unquote(word: &Word) -> Option { +fn unquote(word: &Word) -> Option { let Word { value, loc: _ } = word; let pieces = brush_parser::word::parse(value.as_str(), &PARSER_OPTIONS).ok()?; - let mut result = String::with_capacity(value.len()); + let mut result = Str::with_capacity(value.len()); flatten_pieces(&pieces, &mut result)?; Some(result) } @@ -68,8 +67,7 @@ fn unquote(word: &Word) -> Option { /// Recursively extract literal text from parsed word pieces. /// /// Returns `None` if any piece requires runtime expansion. -#[expect(clippy::disallowed_types, reason = "brush_parser word API uses String")] -fn flatten_pieces(pieces: &[WordPieceWithSource], result: &mut String) -> Option<()> { +fn flatten_pieces(pieces: &[WordPieceWithSource], result: &mut Str) -> Option<()> { for piece in pieces { match &piece.piece { WordPiece::Text(s) | WordPiece::SingleQuotedText(s) | WordPiece::AnsiCQuotedText(s) => { @@ -122,7 +120,7 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range< let AssignmentValue::Scalar(value) = value else { return None; }; - envs.insert(name.as_str().into(), unquote(value)?.into()); + envs.insert(name.as_str().into(), unquote(value)?); } } let mut args = Vec::::new(); @@ -131,10 +129,10 @@ fn pipeline_to_command(pipeline: &Pipeline) -> Option<(TaskParsedCommand, Range< let CommandPrefixOrSuffixItem::Word(word) = suffix_item else { return None; }; - args.push(unquote(word)?.into()); + args.push(unquote(word)?); } } - Some((TaskParsedCommand { envs, program: unquote(program)?.into(), args }, range)) + Some((TaskParsedCommand { envs, program: unquote(program)?, args }, range)) } #[must_use] @@ -263,11 +261,9 @@ mod tests { #[test] fn test_flatten_pieces_recursion() { - #[expect(clippy::disallowed_types, reason = "flatten_pieces uses String")] - fn parse_and_flatten(input: &str) -> Option { + fn parse_and_flatten(input: &str) -> Option { let pieces = brush_parser::word::parse(input, &PARSER_OPTIONS).ok()?; - #[expect(clippy::disallowed_types, reason = "flatten_pieces uses String")] - let mut result = String::new(); + let mut result = Str::default(); flatten_pieces(&pieces, &mut result)?; Some(result) }