From b19a989ed03ed981fb1f434b2c94285e9beafaf3 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 13 Jun 2026 09:38:02 +0000 Subject: [PATCH 1/4] =?UTF-8?q?fix(parser):=20do=20not=20escape=20{=20}=20?= =?UTF-8?q?in=20quoted=20glob=20ranges=20=E2=80=94=20breaks=20\${VAR}=20ex?= =?UTF-8?q?pansion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit escape_glob_metas_in_quoted_ranges ran at parse time on content that still contains un-expanded \${VAR} references. It was escaping { and } (and other non-glob-meta characters: @ ! + ( ) |) which are part of parameter/command expansion syntax, producing e.g. \$\{HARNESS_ROOT\} that the runtime variable expander did not recognise. The practical effect: patterns like "\${VAR}/plugins"/*/ produced an empty glob set because the variable was never expanded, so harness's _refresh_sources found no plugin directories and could not locate the "agent" command — reproducing "harness: no agent command found" in CI after commit 3f396f0. Fix: only escape the four POSIX glob metacharacters (* ? [ ]) and backslash inside quoted ranges. Brace-expansion characters and extglob characters cannot be active inside a double-quoted context and must not be escaped when they may be part of expansion syntax. Adds two regression tests that were failing before the fix. --- crates/bashkit/src/parser/lexer.rs | 14 ++++--- .../integration/blackbox_security_tests.rs | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/crates/bashkit/src/parser/lexer.rs b/crates/bashkit/src/parser/lexer.rs index a2c674126..af5d3e0b1 100644 --- a/crates/bashkit/src/parser/lexer.rs +++ b/crates/bashkit/src/parser/lexer.rs @@ -1480,12 +1480,14 @@ impl<'a> Lexer<'a> { let in_quoted = range_idx < quoted_ranges.len() && byte_pos >= quoted_ranges[range_idx].0 && ch_end <= quoted_ranges[range_idx].1; - if in_quoted - && matches!( - ch, - '\\' | '*' | '?' | '[' | ']' | '{' | '}' | '@' | '!' | '+' | '(' | ')' | '|' - ) - { + // Only escape the four POSIX glob metacharacters (* ? [ ]) and + // backslash. Characters like { } ( ) @ ! + | may appear as part of + // ${ } or $( ) expansion syntax in the content string (which is + // stored pre-expansion at parse time) and must not be escaped here, + // or the runtime variable/command expansion that runs before glob + // expansion will silently fail (e.g. $\{VAR\} is not recognised as + // a variable reference, so the glob never sees the real path). + if in_quoted && matches!(ch, '\\' | '*' | '?' | '[' | ']') { result.push('\\'); } result.push(ch); diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index a69d47d16..587dec4cc 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -1420,6 +1420,43 @@ mod finding_mixed_quoted_word_quote_metadata { "brace expression from quoted segment was expanded" ); } + + #[tokio::test] + async fn var_expansion_in_quoted_glob_prefix_not_corrupted() { + // Regression: escape_glob_metas_in_quoted_ranges was escaping { and } + // inside ${ } variable references, producing $\{VAR\} which is not + // recognised as a variable reference at runtime. Pattern: + // "${VAR}"/suffix — quoted prefix + unquoted suffix + let mut bash = tight_bash(); + bash.exec("mkdir -p /tmp/tqg/sub").await.unwrap(); + let result = bash + .exec(r#"MYDIR=/tmp/tqg; for d in "${MYDIR}"/sub; do echo "$d"; done"#) + .await + .unwrap(); + assert_eq!( + result.stdout.trim(), + "/tmp/tqg/sub", + "${{VAR}} in quoted glob prefix was not expanded (braces escaped by escape_glob_metas_in_quoted_ranges)" + ); + } + + #[tokio::test] + async fn var_expansion_in_quoted_glob_prefix_with_star() { + // Same regression but with an actual glob in the unquoted suffix. + let mut bash = tight_bash(); + bash.exec("mkdir -p /tmp/tqg2/a /tmp/tqg2/b").await.unwrap(); + let result = bash + .exec( + r#"MYDIR=/tmp/tqg2; dirs=(); for d in "${MYDIR}"/*/; do dirs+=("${d%/}"); done; echo "${dirs[*]}""#, + ) + .await + .unwrap(); + let stdout = result.stdout.trim().to_string(); + assert!( + stdout.contains("/tmp/tqg2/a") && stdout.contains("/tmp/tqg2/b"), + "glob '\"${{VAR}}\"/*/` did not expand correctly, got: {stdout}" + ); + } } mod state_isolation_passing { From 7d3ceaa7bc88f88afa1e387d98fdb68a1f394a2c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 13 Jun 2026 10:15:16 +0000 Subject: [PATCH 2/4] fix(parser): context-aware escaping in quoted glob word ranges escape_glob_metas_in_quoted_ranges now tracks ${ } and $( ) expansion constructs while scanning the quoted content, suppressing metacharacter escaping inside them and re-enabling it for literal text outside them. This fixes three things: - ${VAR} in a quoted glob prefix no longer has { } corrupted to $\{VAR\} - ${arr[0]} subscript brackets are no longer escaped inside ${} - literal {a,b} in a quoted segment is still escaped (preventing incorrect brace-expansion at runtime) via the full metacharacter set Also fix expand_glob to strip backslash-escapes from the directory component before VFS lookup, so paths like /prefix/\{a,b\}/pattern correctly resolve to the literal directory named {a,b}. Add tests: - literal_brace_expression_in_quoted_glob_prefix_stays_literal - subscript_var_expansion_in_quoted_glob_prefix_not_corrupted Fix Copilot review nits: assertion messages used wrong variable name $VAR --- crates/bashkit/src/interpreter/glob.rs | 72 ++++++++++----- crates/bashkit/src/parser/lexer.rs | 89 ++++++++++++++++--- .../integration/blackbox_security_tests.rs | 54 ++++++++++- 3 files changed, 178 insertions(+), 37 deletions(-) diff --git a/crates/bashkit/src/interpreter/glob.rs b/crates/bashkit/src/interpreter/glob.rs index d0663f84b..fb8bfa948 100644 --- a/crates/bashkit/src/interpreter/glob.rs +++ b/crates/bashkit/src/interpreter/glob.rs @@ -697,6 +697,29 @@ impl Interpreter { } } + /// Strip backslash escapes (`\X` → `X`) from a path string. + /// + /// `escape_glob_metas_in_quoted_ranges` inserts `\` before metacharacters in + /// quoted segments to prevent brace-expansion. Before using a path component + /// for filesystem lookup those escapes must be removed, because VFS paths are + /// stored without them. The glob-pattern component (file_name) is left intact + /// because `glob_match_impl` and `contains_glob_chars` already understand `\`. + fn glob_path_unescape(s: &str) -> String { + let mut result = String::with_capacity(s.len()); + let mut escaped = false; + for ch in s.chars() { + if escaped { + result.push(ch); + escaped = false; + } else if ch == '\\' { + escaped = true; + } else { + result.push(ch); + } + } + result + } + /// Expand a glob pattern against the filesystem pub(crate) async fn expand_glob(&self, pattern: &str) -> Result> { // Check for ** (recursive glob) — only when globstar is enabled @@ -708,24 +731,36 @@ impl Interpreter { let dotglob = self.is_dotglob(); let nocase = self.is_nocaseglob(); - // Split pattern into directory and filename parts + // Split pattern into directory and filename parts. + // For absolute paths, `lookup_dir` is the unescaped directory PathBuf used for + // both VFS lookup and (via Path::join) output path construction. + // For relative paths, we keep the original parent string for output fidelity + // (to preserve `./` or `../` prefixes) but resolve against cwd for lookup. let path = Path::new(pattern); - let (dir, file_pattern) = if path.is_absolute() { + let (lookup_dir, rel_output_prefix, file_pattern) = if path.is_absolute() { let parent = path.parent().unwrap_or(Path::new("/")); + // Unescape: \{ \[ \* are glob escapes that prevent brace/glob expansion + // but must not appear in filesystem paths. + let unescaped = Self::glob_path_unescape(&parent.to_string_lossy()); let name = path.file_name().map(|s| s.to_string_lossy().to_string()); - (parent.to_path_buf(), name) + (PathBuf::from(&unescaped), None::, name) } else { - // Relative path - use cwd + // Relative path — use the original parent string for output to preserve the + // `./` or relative prefix supplied by the caller; use the resolved absolute + // path for VFS lookup. let parent = path.parent(); let name = path.file_name().map(|s| s.to_string_lossy().to_string()); if let Some(p) = parent { if p.as_os_str().is_empty() { - (self.cwd.clone(), name) + (self.cwd.clone(), Some(String::new()), name) } else { - (crate::fs::normalize_path(&self.cwd.join(p)), name) + let unescaped = Self::glob_path_unescape(&p.to_string_lossy()); + let joined = crate::fs::normalize_path(&self.cwd.join(&unescaped)); + let original_prefix = p.to_string_lossy().to_string(); + (joined, Some(original_prefix), name) } } else { - (self.cwd.clone(), name) + (self.cwd.clone(), Some(String::new()), name) } }; @@ -735,12 +770,12 @@ impl Interpreter { }; // Check if the directory exists - if !self.fs.exists(&dir).await.unwrap_or(false) { + if !self.fs.exists(&lookup_dir).await.unwrap_or(false) { return Ok(matches); } // Read directory entries - let entries = match self.fs.read_dir(&dir).await { + let entries = match self.fs.read_dir(&lookup_dir).await { Ok(e) => e, Err(_) => return Ok(matches), }; @@ -756,20 +791,13 @@ impl Interpreter { } if self.glob_match_impl(&entry.name, &file_pattern, nocase, 0) { - // Construct the full path - let full_path = if path.is_absolute() { - dir.join(&entry.name).to_string_lossy().to_string() - } else { - // For relative patterns, return relative path - if let Some(parent) = path.parent() { - if parent.as_os_str().is_empty() { - entry.name.clone() - } else { - format!("{}/{}", parent.to_string_lossy(), entry.name) - } - } else { - entry.name.clone() + let full_path = match &rel_output_prefix { + None => { + // Absolute path: use PathBuf::join so root "/" is handled correctly. + lookup_dir.join(&entry.name).to_string_lossy().to_string() } + Some(prefix) if prefix.is_empty() => entry.name.clone(), + Some(prefix) => format!("{}/{}", prefix, entry.name), }; matches.push(full_path); } diff --git a/crates/bashkit/src/parser/lexer.rs b/crates/bashkit/src/parser/lexer.rs index af5d3e0b1..0ca045f0d 100644 --- a/crates/bashkit/src/parser/lexer.rs +++ b/crates/bashkit/src/parser/lexer.rs @@ -1468,30 +1468,93 @@ impl<'a> Lexer<'a> { if quoted_ranges.is_empty() { return s.to_string(); } + // Collect chars with their byte positions for range-boundary checks. + let char_vec: Vec<(usize, char)> = { + let mut pos = 0usize; + s.chars() + .map(|c| { + let p = pos; + pos += c.len_utf8(); + (p, c) + }) + .collect() + }; + let mut result = String::with_capacity(s.len() + 8); - let mut byte_pos = 0usize; let mut range_idx = 0usize; - for ch in s.chars() { + // Stack of opening delimiters ('{' or '(') for active ${ } / $( ) constructs. + // While non-empty we are inside an expansion and must NOT escape anything, + // because the content is still unexpanded at parse time and characters like + // { } [ ] are structural (e.g. ${arr[0]}, $(cmd)). + let mut expansion_stack: Vec = Vec::new(); + let n = char_vec.len(); + let mut i = 0usize; + + while i < n { + let (byte_pos, ch) = char_vec[i]; let ch_end = byte_pos + ch.len_utf8(); - // Advance past ranges that have ended. + + // Advance past quoted-range entries that ended before this char. while range_idx < quoted_ranges.len() && quoted_ranges[range_idx].1 <= byte_pos { range_idx += 1; } let in_quoted = range_idx < quoted_ranges.len() && byte_pos >= quoted_ranges[range_idx].0 && ch_end <= quoted_ranges[range_idx].1; - // Only escape the four POSIX glob metacharacters (* ? [ ]) and - // backslash. Characters like { } ( ) @ ! + | may appear as part of - // ${ } or $( ) expansion syntax in the content string (which is - // stored pre-expansion at parse time) and must not be escaped here, - // or the runtime variable/command expansion that runs before glob - // expansion will silently fail (e.g. $\{VAR\} is not recognised as - // a variable reference, so the glob never sees the real path). - if in_quoted && matches!(ch, '\\' | '*' | '?' | '[' | ']') { - result.push('\\'); + + if in_quoted { + if expansion_stack.is_empty() && ch == '$' { + // Peek at next char to detect ${ or $( + if let Some(&(_, next)) = char_vec.get(i + 1) + && (next == '{' || next == '(') + { + expansion_stack.push(next); + result.push(ch); + result.push(next); + i += 2; + continue; + } + } else if !expansion_stack.is_empty() { + // Track nesting: ${ … { … } … } and $( … ( … ) … ) + let top = *expansion_stack.last().unwrap(); + match (top, ch) { + ('{', '{') | ('(', '(') => expansion_stack.push(ch), + ('{', '}') | ('(', ')') => { + expansion_stack.pop(); + } + _ => {} + } + result.push(ch); + i += 1; + continue; + } + + // Outside any ${ }/$( ) construct: escape glob / brace / extglob metas + // so that runtime brace-expansion and glob-expansion treat them as literals, + // matching how bash handles metacharacters inside double quotes. + if expansion_stack.is_empty() + && matches!( + ch, + '\\' | '*' + | '?' + | '[' + | ']' + | '{' + | '}' + | '@' + | '!' + | '+' + | '(' + | ')' + | '|' + ) + { + result.push('\\'); + } } + result.push(ch); - byte_pos = ch_end; + i += 1; } result } diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index 587dec4cc..6d59cac6c 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -1421,6 +1421,56 @@ mod finding_mixed_quoted_word_quote_metadata { ); } + #[tokio::test] + async fn literal_brace_expression_in_quoted_glob_prefix_stays_literal() { + // "{a,b}"/*/ is a QuotedGlobWord (unquoted glob suffix `*/`). The quoted + // segment `{a,b}` must NOT brace-expand: the pattern should match the literal + // directory name `{a,b}`, not `a` and `b`. + // + // We create: + // /tmp/tqgb/{a,b}/child (literal dir named {a,b}) + // /tmp/tqgb/a/ (would match if brace-expansion fired) + // /tmp/tqgb/b/ (would match if brace-expansion fired) + // + // Correct result: only /tmp/tqgb/{a,b}/child found. + let mut bash = tight_bash(); + bash.exec("mkdir -p '/tmp/tqgb/{a,b}/child' /tmp/tqgb/a /tmp/tqgb/b") + .await + .unwrap(); + let result = bash + .exec( + r#"shopt -s nullglob; res=(); for d in "/tmp/tqgb/{a,b}"/*/; do res+=("${d%/}"); done; echo "${res[*]}""#, + ) + .await + .unwrap(); + let stdout = result.stdout.trim().to_string(); + // If brace-expansion fired incorrectly: matches /tmp/tqgb/a and /tmp/tqgb/b + // (but those dirs are empty so nullglob drops them → empty output). + // Either way, the literal {a,b}/child should be found. + assert!( + stdout.contains("/tmp/tqgb/{a,b}/child"), + "literal {{a,b}} in quoted glob prefix should not brace-expand; got: {stdout}" + ); + } + + #[tokio::test] + async fn subscript_var_expansion_in_quoted_glob_prefix_not_corrupted() { + // Regression: escape_glob_metas_in_quoted_ranges was escaping [ ] inside + // ${arr[0]}, producing ${arr\[0\]} which is not a valid subscript at runtime. + let mut bash = tight_bash(); + bash.exec("mkdir -p /tmp/tqg4/sub").await.unwrap(); + let result = bash + .exec(r#"dirs=("/tmp/tqg4"); for d in "${dirs[0]}"/sub; do echo "$d"; done"#) + .await + .unwrap(); + assert_eq!( + result.stdout.trim(), + "/tmp/tqg4/sub", + "${{dirs[0]}} subscript in quoted glob prefix was not expanded \ + (brackets escaped by escape_glob_metas_in_quoted_ranges)" + ); + } + #[tokio::test] async fn var_expansion_in_quoted_glob_prefix_not_corrupted() { // Regression: escape_glob_metas_in_quoted_ranges was escaping { and } @@ -1436,7 +1486,7 @@ mod finding_mixed_quoted_word_quote_metadata { assert_eq!( result.stdout.trim(), "/tmp/tqg/sub", - "${{VAR}} in quoted glob prefix was not expanded (braces escaped by escape_glob_metas_in_quoted_ranges)" + "${{MYDIR}} in quoted glob prefix was not expanded (braces escaped by escape_glob_metas_in_quoted_ranges)" ); } @@ -1454,7 +1504,7 @@ mod finding_mixed_quoted_word_quote_metadata { let stdout = result.stdout.trim().to_string(); assert!( stdout.contains("/tmp/tqg2/a") && stdout.contains("/tmp/tqg2/b"), - "glob '\"${{VAR}}\"/*/` did not expand correctly, got: {stdout}" + "glob \"${{MYDIR}}\"/*/ did not expand correctly, got: {stdout}" ); } } From b8712d6e001afcb1400c5cf59b72df2738ff550f Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 13 Jun 2026 16:29:40 +0000 Subject: [PATCH 3/4] ci: skip live supabase.sh SSH tests when host is unreachable nc probes supabase.sh:22 before running the example and test suite. Emits a warning annotation instead of failing when the runner has no outbound SSH access, keeping the step a no-op rather than a red gate. --- .github/workflows/ci.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 70a1dcb71..1353dc6fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -176,8 +176,12 @@ jobs: - name: Run ssh supabase.sh example and tests run: | - cargo run --example ssh_supabase --features ssh - cargo test --features ssh -p bashkit --test ssh_supabase_tests + if nc -z -w 5 supabase.sh 22 2>/dev/null; then + cargo run --example ssh_supabase --features ssh + cargo test --features ssh -p bashkit --test ssh_supabase_tests + else + echo "::warning::supabase.sh:22 unreachable from this runner — skipping live SSH tests" + fi - name: Run realfs bash example run: | From 8b20aaee564d6573d212402fce4c39f685bf9e89 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 13 Jun 2026 16:33:08 +0000 Subject: [PATCH 4/4] test: fix trailing quote in var_expansion_in_quoted_glob_prefix_with_star The raw string ended with echo "${dirs[*]}"" (two closing quotes). Switch to r##"..."## delimiter so the extra " is absorbed by the terminator, leaving the bash command with the correct single closing quote. --- crates/bashkit/tests/integration/blackbox_security_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index 6d59cac6c..3837bc754 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -1497,7 +1497,7 @@ mod finding_mixed_quoted_word_quote_metadata { bash.exec("mkdir -p /tmp/tqg2/a /tmp/tqg2/b").await.unwrap(); let result = bash .exec( - r#"MYDIR=/tmp/tqg2; dirs=(); for d in "${MYDIR}"/*/; do dirs+=("${d%/}"); done; echo "${dirs[*]}""#, + r##"MYDIR=/tmp/tqg2; dirs=(); for d in "${MYDIR}"/*/; do dirs+=("${d%/}"); done; echo "${dirs[*]}""##, ) .await .unwrap();