diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 279ee296a..7779d71b7 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -10326,14 +10326,30 @@ impl Interpreter { } } let commands: &[Command] = if file_read.is_some() { &[] } else { commands }; + // Captured output must not reach the streaming callback: compound + // commands (`case`, `{ }`, `if`) emit through it as they run. + let saved_callback = self.output_callback.take(); + let mut run: Result<()> = Ok(()); for cmd in commands { - let cmd_result = self.execute_command(cmd).await?; - stdout.try_push_str(&cmd_result.stdout.command_substitution_text())?; + let cmd_result = match self.execute_command(cmd).await { + Ok(r) => r, + Err(e) => { + run = Err(e); + break; + } + }; + if let Err(e) = stdout.try_push_str(&cmd_result.stdout.command_substitution_text()) + { + run = Err(e.into()); + break; + } self.last_exit_code = cmd_result.exit_code; if matches!(cmd_result.control_flow, ControlFlow::Exit(_)) { break; } } + self.output_callback = saved_callback; + run?; // Fire EXIT trap set inside the command substitution if let Some(trap_cmd) = self.scoped.traps.get("EXIT").cloned() && snapshot.scoped.traps.get("EXIT") != Some(&trap_cmd) diff --git a/crates/bashkit/src/parser/lexer.rs b/crates/bashkit/src/parser/lexer.rs index 696a19055..a81a4c58c 100644 --- a/crates/bashkit/src/parser/lexer.rs +++ b/crates/bashkit/src/parser/lexer.rs @@ -244,7 +244,15 @@ impl<'a> Lexer<'a> { } '[' => { self.advance(); - if self.peek_char() == Some('[') { + // `[[` is the keyword only as a whole word; `[[:digit:]]*` is + // a glob bracket expression. + let mut lookahead = self.chars.clone(); + let keyword = lookahead.next() == Some('[') + && matches!( + lookahead.next(), + None | Some(' ' | '\t' | '\n' | ';' | '&' | '|' | '(' | ')') + ); + if keyword { self.advance(); Some(Token::DoubleLeftBracket) } else { diff --git a/crates/bashkit/src/parser/subst_scan.rs b/crates/bashkit/src/parser/subst_scan.rs index 966385ae5..06ced9be4 100644 --- a/crates/bashkit/src/parser/subst_scan.rs +++ b/crates/bashkit/src/parser/subst_scan.rs @@ -6,8 +6,10 @@ //! and `$(echo ")")` at the quoted `)`. Bash finds the end by parsing; this //! tracks the contexts that can hide a `)`: single/double quotes, backslash //! escapes, backticks, nested `$(...)`, comments and heredoc bodies. -//! Known gap: a `case` pattern's bare `)` inside `$(...)` still closes early, -//! as before this scanner (`$(case x in x) ...esac)`); bash accepts it. +//! `case` is tracked too: the bare `)` that ends a case pattern +//! (`$(case x in x) ...;; esac)`) does not close the substitution. Words are +//! split at blanks and operators, and `case`/`in`/`esac` count only as +//! unquoted words in the right place (command position for `case`/`esac`). #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Frame { @@ -18,6 +20,41 @@ enum Frame { Backtick, } +/// Where an open `case` (inside one code frame) is. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CasePhase { + /// After `case`, reading the subject word. + Subject, + /// Expecting `in`. + In, + /// Reading a pattern list; `started` once any pattern text was seen. + Pattern { started: bool }, + /// In a branch body, until `;;`, `;&`, `;;&` or `esac`. + Body, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct CaseCtx { + phase: CasePhase, + /// Paren depth of the frame when `case` started. + depth: usize, +} + +/// Word and `case` tracking for one `Code` frame. +#[derive(Debug, Clone, Default)] +struct CodeState { + /// Unquoted text of the current word, capped (only keywords matter). + word: String, + /// The current word has quoted or escaped parts, so it is no keyword. + word_quoted: bool, + /// The next word starts a command. + cmd_pos: bool, + cases: Vec, +} + +/// Longest keyword we need to recognise (`case`, `esac`). +const MAX_KEYWORD: usize = 8; + #[derive(Debug, Clone, PartialEq, Eq)] struct Heredoc { delimiter: String, @@ -60,6 +97,8 @@ pub(crate) struct SubstScanner { stack: Vec, /// Paren depth per `Code` frame (parallel to Code entries in `stack`). parens: Vec, + /// Word/`case` state per `Code` frame (parallel to `parens`). + code: Vec, mode: Mode, prev: Option, /// `<` chars seen in a row in code context. @@ -81,6 +120,10 @@ impl SubstScanner { Self { stack: vec![Frame::Code], parens: vec![0], + code: vec![CodeState { + cmd_pos: true, + ..CodeState::default() + }], mode: Mode::Normal, prev: None, lt_run: 0, @@ -250,6 +293,109 @@ impl SubstScanner { fn push_code(&mut self) { self.stack.push(Frame::Code); self.parens.push(0); + self.code.push(CodeState { + cmd_pos: true, + ..CodeState::default() + }); + } + + /// Mark the current word as quoted (a quote or escape starts in code). + fn quote_word(&mut self) { + if let Some(st) = self.code.last_mut() { + st.word_quoted = true; + st.word.push('"'); + } + } + + /// End the current word in the top code frame and update `case` state. + fn end_word(&mut self) { + let Some(st) = self.code.last_mut() else { + return; + }; + if st.word.is_empty() { + return; + } + let word = std::mem::take(&mut st.word); + let keyword = !std::mem::take(&mut st.word_quoted); + let cmd_pos = st.cmd_pos; + let depth = self.parens.last().copied().unwrap_or(0); + let kw = |k: &str| keyword && word == k; + let mut next_cmd_pos = false; + match st.cases.last_mut().map(|c| (c.phase, c.depth)) { + Some((CasePhase::Subject, _)) => { + if let Some(c) = st.cases.last_mut() { + c.phase = CasePhase::In; + } + } + Some((CasePhase::In, _)) => { + if let Some(c) = st.cases.last_mut() { + c.phase = if kw("in") { + CasePhase::Pattern { started: false } + } else { + // Not `in`: not a case we understand; stop tracking. + st.cases.pop(); + return; + }; + } + } + Some((CasePhase::Pattern { started }, d)) if d == depth => { + if !started && kw("esac") { + st.cases.pop(); + } else if let Some(c) = st.cases.last_mut() { + c.phase = CasePhase::Pattern { started: true }; + } + } + Some((CasePhase::Body, d)) if d == depth && cmd_pos && kw("esac") => { + st.cases.pop(); + } + _ => { + if cmd_pos && kw("case") { + st.cases.push(CaseCtx { + phase: CasePhase::Subject, + depth, + }); + } else if cmd_pos + && keyword + && matches!( + word.as_str(), + "if" | "then" + | "else" + | "elif" + | "do" + | "while" + | "until" + | "{" + | "!" + | "time" + ) + { + next_cmd_pos = true; + } + } + } + st.cmd_pos = next_cmd_pos; + } + + /// The open case in the top frame, if it is at the current paren depth. + fn case_here(&self) -> Option { + let depth = self.parens.last().copied().unwrap_or(0); + self.code + .last() + .and_then(|st| st.cases.last()) + .filter(|c| c.depth == depth) + .map(|c| c.phase) + } + + fn set_case_phase(&mut self, phase: CasePhase) { + if let Some(c) = self.code.last_mut().and_then(|st| st.cases.last_mut()) { + c.phase = phase; + } + } + + fn set_cmd_pos(&mut self, on: bool) { + if let Some(st) = self.code.last_mut() { + st.cmd_pos = on; + } } fn feed_code(&mut self, c: char) -> Step { @@ -266,43 +412,107 @@ impl SubstScanner { }; return self.feed_inner(c); } - match c { - '\\' => self.mode = Mode::Escape, - '\'' => self.stack.push(Frame::Single), - '"' => self.stack.push(Frame::Double), - '`' => self.stack.push(Frame::Backtick), - '#' if self - .prev - .is_none_or(|p| p.is_whitespace() || matches!(p, ';' | '|' | '&' | '(' | ')')) => + let word_char = !c.is_whitespace() + && !matches!( + c, + ';' | '|' | '&' | '(' | ')' | '<' | '>' | '\\' | '\'' | '"' | '`' + ); + if word_char { + if c == '#' + && self + .prev + .is_none_or(|p| p.is_whitespace() || matches!(p, ';' | '|' | '&' | '(' | ')')) { + self.end_word(); self.mode = Mode::Comment; + return Step::Body; + } + if let Some(st) = self.code.last_mut() + && st.word.len() < MAX_KEYWORD + { + st.word.push(c); + } else if let Some(st) = self.code.last_mut() { + st.word_quoted = true; // too long to be a keyword + } + return Step::Body; + } + match c { + '\\' => { + self.quote_word(); + self.mode = Mode::Escape; + } + '\'' => { + self.quote_word(); + self.stack.push(Frame::Single); + } + '"' => { + self.quote_word(); + self.stack.push(Frame::Double); + } + '`' => { + self.quote_word(); + self.stack.push(Frame::Backtick); + } + '\n' => { + self.end_word(); + self.set_cmd_pos(true); + self.on_newline(); } - '\n' => self.on_newline(), '(' => { + self.end_word(); + // The optional `(` before a case pattern is not nesting. + if let Some(CasePhase::Pattern { started: false }) = self.case_here() { + self.set_case_phase(CasePhase::Pattern { started: true }); + return Step::Body; + } if let Some(depth) = self.parens.last_mut() { *depth += 1; if self.prev == Some('(') && self.arith_at.is_none() { self.arith_at = Some(depth.saturating_sub(2)); } } + self.set_cmd_pos(true); } ')' => { + self.end_word(); + // `)` ending a case pattern list starts the branch body. + if let Some(CasePhase::Pattern { .. }) = self.case_here() { + self.set_case_phase(CasePhase::Body); + self.set_cmd_pos(true); + return Step::Body; + } let depth = self.parens.last_mut().expect("code frame"); if *depth > 0 { *depth -= 1; if self.arith_at.is_some_and(|at| *depth <= at) { self.arith_at = None; } + self.set_cmd_pos(false); } else { // Closes this `$(` frame. self.parens.pop(); + self.code.pop(); self.stack.pop(); if self.stack.is_empty() { return Step::Close; } } } - _ => {} + ';' | '&' => { + self.end_word(); + // `;;`, `;&` and `;;&` end a case branch. + if self.prev == Some(';') && self.case_here() == Some(CasePhase::Body) { + self.set_case_phase(CasePhase::Pattern { started: false }); + } + self.set_cmd_pos(true); + } + '|' => { + self.end_word(); + if !matches!(self.case_here(), Some(CasePhase::Pattern { .. })) { + self.set_cmd_pos(true); + } + } + _ => self.end_word(), } Step::Body } @@ -374,6 +584,35 @@ mod tests { assert_eq!(body("echo $((1<<2))) y"), Some("echo $((1<<2))")); } + #[test] + fn case_patterns_do_not_close() { + assert_eq!( + body("case $x in a) echo A;; *) echo B;; esac) y"), + Some("case $x in a) echo A;; *) echo B;; esac") + ); + assert_eq!( + body("case x in (a|b) echo;; esac) y"), + Some("case x in (a|b) echo;; esac") + ); + assert_eq!( + body("case x in\n a)\n echo;;\nesac\n) y"), + Some("case x in\n a)\n echo;;\nesac\n") + ); + // nested case, fallthrough terminators + assert_eq!( + body("case a in a) case b in b) :;& *) :;;& esac;; esac) y"), + Some("case a in a) case b in b) :;& *) :;;& esac;; esac") + ); + // `case`/`esac` only count in command position + assert_eq!(body("echo case) y"), Some("echo case")); + assert_eq!(body("echo esac) y"), Some("echo esac")); + // a `case` word inside a pattern body subshell + assert_eq!( + body("case x in x) (echo case);; esac) y"), + Some("case x in x) (echo case);; esac") + ); + } + #[test] fn unterminated() { assert_eq!(body("echo (hi"), None); diff --git a/crates/bashkit/tests/integration/pipeline_tests.rs b/crates/bashkit/tests/integration/pipeline_tests.rs index e42c8b7b4..066a3a08d 100644 --- a/crates/bashkit/tests/integration/pipeline_tests.rs +++ b/crates/bashkit/tests/integration/pipeline_tests.rs @@ -121,3 +121,23 @@ fn threat_recursive_pipeline_is_bounded() { }; assert!(failed, "{result:?}"); } + +/// Output of compound commands inside `$(...)` is captured, never streamed +/// to the caller (a `case`, group or `if` inside a substitution leaked). +#[tokio::test] +async fn command_substitution_output_is_not_streamed() { + let streamed = Arc::new(Mutex::new(String::new())); + let sink = streamed.clone(); + let mut bash = Bash::new(); + let result = bash + .exec_streaming( + "x=$(case a in a) echo c;; esac); y=$({ echo g; }); z=$(if true; then echo i; fi)\necho \"$x$y$z\"", + Box::new(move |stdout, _stderr| { + sink.lock().unwrap().push_str(&stdout.to_string()); + }), + ) + .await + .unwrap(); + assert_eq!(result.stdout, "cgi\n"); + assert_eq!(*streamed.lock().unwrap(), "cgi\n"); +} diff --git a/crates/bashkit/tests/spec_cases/bash/case-in-cmdsub.test.sh b/crates/bashkit/tests/spec_cases/bash/case-in-cmdsub.test.sh new file mode 100644 index 000000000..40d43a418 --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/case-in-cmdsub.test.sh @@ -0,0 +1,57 @@ +### case_in_cmdsub_one_line +x=b +echo "$(case $x in a) echo A;; b) echo B;; esac)" +### expect +B +### end + +### case_in_cmdsub_multiline_nested +r=$(case 1 in + 1) case 2 in + 2) echo inner;; + esac;; + *) echo other;; +esac) +echo "$r" +### expect +inner +### end + +### case_in_cmdsub_paren_pattern_and_comment +v=$(case z in (y|z) echo yz ;; # a) comment +esac) +echo "$v" +### expect +yz +### end + +### case_in_backticks +echo `case q in q) echo Q;; esac` +### expect +Q +### end + +### case_word_as_argument +echo $(echo case esac in) +### expect +case esac in +### end + +### dbracket_posix_class_pattern +z=5abc +[[ $z == [[:digit:]]* ]] && echo digit +[[ a1 == [[:alpha:]][[:digit:]] ]] && echo alnum +[[ x == [[:digit:]] ]] || echo nodigit +### expect +digit +alnum +nodigit +### end + +### case_posix_class_pattern +case 5 in [[:digit:]]) echo d;; *) echo n;; esac +case k in [[:digit:]]) echo d;; *) echo n;; esac +### expect +d +n +### end diff --git a/knowledge/foundations/parser.md b/knowledge/foundations/parser.md index 8c9927df2..c053643c0 100644 --- a/knowledge/foundations/parser.md +++ b/knowledge/foundations/parser.md @@ -112,8 +112,15 @@ prefix. **End of `$(...)`.** `parser/subst_scan.rs` is the one scanner the lexer and `parse_word` share to find the closing `)`: it tracks quotes, escapes, backticks, nested `$(`, comments and heredoc bodies (`<<`, `<<-`, quoted -delimiters; `<<<` and arithmetic `<<` excluded). Known gap: a bare `case` -pattern `)` inside `$(...)` still closes early. +delimiters; `<<<` and arithmetic `<<` excluded). It also follows `case` +in command position (subject, `in`, pattern, body; `;;`/`;&`/`;;&` back to +pattern), so a pattern's `)` and an optional leading `(` do not change the +paren depth. Keywords are recognised only as whole unquoted words in command +position, so `echo case)` still closes. + +**`[[` lexing.** `[[` is the keyword only when followed by a word break +(space, tab, newline, `;`, `&`, `|`, `(`, `)`, or end); `[[:digit:]]*` is a +bracket-expression word. ## Alternatives Considered diff --git a/knowledge/log.md b/knowledge/log.md index 196d2b0cc..b24d5ca25 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -19,6 +19,7 @@ * **Fix**: Quotes and patterns inside `${...}` operands. Unquoted `${x:-'d'}` keeps no quotes and `'$y'` stays literal; `"${x:-'d'}"` keeps them, as in bash; a `}` or `/` inside quotes no longer ends the operand. `${x/pattern/rep}` now expands `$var`, quotes and `\c` in the pattern (it used them verbatim) and matches globs leftmost-longest with `?`, `[...]` and any number of `*`; extglob patterns follow bash's empty-match scan. Globs compile to a size-capped regex, extglob search is budgeted (TM-DOS-127). Known gap: a quoted default with spaces (`${x:-'a b'}`) is still word-split. Spec: `param-operand-quotes.test.sh`. * **Feature**: Gap telemetry (`bashkit-replay`, `just gaps`). Replays the 5,206 recorded eval tool calls (1,493 sessions) on the current bashkit and ranks missing commands, unknown options, unsupported features and parse errors; `/benches` shows the top 20 and the history. First report: calls hitting a gap fell from 319 recorded to 125, non-zero exits from 723 to 231. Top remaining: heredoc `expected 'done'` and `unterminated command substitution` parse errors, `tar --strip-components`, `grep -I`, `join -o`, `find -ls`, plus probes for host tools (`busybox`, `gawk`, `sudo`, `python3`). * **Fix**: `#` mid-word is literal (`echo a#b` printed `a`; it starts a comment only at word start), and the `[[ =~ ]]` operand is read from source as one regex word as bash does: `#`, `|` and spaces inside groups are pattern text (`^(#+)[[:space:]](.+)$` was a parse error), quoted text and `\c` match literally (`"a.b"` no longer matches `axb`), and a bare trailing `$` is an anchor. Found by gap telemetry: `expected 'done'` / `unexpected end of input in [[ ]]` were the top parse errors in replayed agent calls. +* **Fix**: `case` works inside `$( )`, backticks and `"$( )"`. The `$( )` end scanner tracks case patterns, so `x=$(case $v in a) echo A;; esac)` no longer closes at the pattern `)`; nested and multi-line forms work. `[[ $z == [[:digit:]]* ]]` lexes the pattern as a word instead of a second `[[`. Compound commands inside a command substitution no longer leak to the streaming output callback (the CLI printed `case`/`if`/`{ }` output twice). Found in the pseudo-linus oracle list. * **Fix**: Option gaps from the replayed agent calls. `tar` takes GNU long options (`--strip-components`, `--exclude`, `--file`, `-C/--directory`, `--to-stdout`, ...), extracts named members, and rejects unknown long options with exit 2. `gzip`/`gunzip` take `-c`/`--stdout` and `-1..-9`, binary-safe through pipes. `grep -I`/`--binary-files=without-match` skip binary files, and `grep -r` names matches the way GNU does (`d/b`, `./b`, bare `b` with no operand, no prefix for a single file operand) instead of absolute VFS paths. `join` was rewritten to GNU semantics (`-o`/`auto`, `-e`, `-v`, `-a`, `-j`, `-i`, `--header`, cartesian products). `xargs` runs the command once on empty input, like GNU, and `-r`/`--no-run-if-empty` skips it. * **Feature**: `make` builtin, a GNU make 4.3 subset run as an execution-plan driver: assignments, conditionals, `define`, `include`, explicit/pattern/static/suffix rules, target-specific variables, special targets and the common functions incl. `$(shell)`/`$(wildcard)`. Recipes run as `sh -c` in the sandbox shell; output and make's messages stream in order (new `PlanStep::Capture`/`Emit`). 19 spec cases match real GNU make 4.3. Gaps L-MAKE-001..003 (no `$(eval)`/`$(file)`, no built-in or chained implicit rules or `vpath`, sequential `-j`, `SHELL` ignored, MAKELEVEL cap 4); caps TM-DOS-126. `make` left the "build tools are not available" hint list. * **Security**: A script that re-runs itself through `sh`/`bash` crashed the whole process with a stack overflow (TM-DOS-125); nothing bounded nested child shells. They now count against `max_function_depth` plus their own cap of 8, measured as what fits a 2 MiB debug-build stack (about 15 bare `sh` levels fit; a level costs several function calls). Found while bounding `make` recursion.