From aa45cfdd9c743afb1f765fe2ce1c5bc085cd0aae Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 01:22:21 +0000 Subject: [PATCH 1/7] fix(interpreter): loop/return scoping, bad substitution and cannot-assign errors (WIP) --- crates/bashkit/src/builtins/flow.rs | 99 +++++++++++---- crates/bashkit/src/interpreter/expansion.rs | 15 +++ crates/bashkit/src/interpreter/mod.rs | 93 ++++++++++++-- crates/bashkit/src/parser/ast.rs | 5 + crates/bashkit/src/parser/mod.rs | 113 +++++++++++++++++- .../bash/bashbox-interpreter-core.test.sh | 29 ----- 6 files changed, 291 insertions(+), 63 deletions(-) diff --git a/crates/bashkit/src/builtins/flow.rs b/crates/bashkit/src/builtins/flow.rs index 7f20df493..9676595cd 100644 --- a/crates/bashkit/src/builtins/flow.rs +++ b/crates/bashkit/src/builtins/flow.rs @@ -64,19 +64,55 @@ impl Builtin for Exit { } } +/// Resolve the level count of `break`/`continue` against the enclosing loops. +/// +/// Bash semantics: outside a loop the command only warns (status 0); a count +/// below 1 is an error (status 1); a count above the loop depth leaves every +/// enclosing loop of the current function, never a caller's loop (a function +/// call starts outside any loop, so the count is clamped to its depth). +fn loop_control(name: &str, ctx: &Context<'_>, make: fn(u32) -> ControlFlow) -> ExecResult { + // Builtins run without shell state (e.g. direct unit calls) keep the + // plain unwinding behaviour. + let depth = ctx + .shell + .as_ref() + .map_or(u32::MAX, |s| u32::try_from(s.loop_depth).unwrap_or(u32::MAX)); + if depth == 0 { + return ExecResult::err( + format!("bash: {name}: only meaningful in a `for', `while', or `until' loop\n"), + 0, + ); + } + let levels = match ctx.args.first() { + None => 1, + Some(arg) => match arg.trim().parse::() { + Ok(n) if n >= 1 => n, + Ok(_) => { + // Bash still leaves every enclosing loop. + let mut result = ExecResult::with_control_flow(ControlFlow::Break(depth)); + result.exit_code = 1; + result.stderr = format!("bash: {name}: {arg}: loop count out of range\n").into(); + return result; + } + Err(_) => { + return ExecResult::err( + format!("bash: {name}: {arg}: numeric argument required\n"), + 128, + ); + } + }, + }; + let levels = u32::try_from(levels).unwrap_or(u32::MAX).min(depth); + ExecResult::with_control_flow(make(levels)) +} + /// The break builtin - break out of a loop pub struct Break; #[async_trait] impl Builtin for Break { async fn execute(&self, ctx: Context<'_>) -> Result { - let levels = ctx - .args - .first() - .and_then(|s| s.parse::().ok()) - .unwrap_or(1); - - Ok(ExecResult::with_control_flow(ControlFlow::Break(levels))) + Ok(loop_control("break", &ctx, ControlFlow::Break)) } } @@ -86,33 +122,46 @@ pub struct Continue; #[async_trait] impl Builtin for Continue { async fn execute(&self, ctx: Context<'_>) -> Result { - let levels = ctx - .args - .first() - .and_then(|s| s.parse::().ok()) - .unwrap_or(1); - - Ok(ExecResult::with_control_flow(ControlFlow::Continue(levels))) + Ok(loop_control("continue", &ctx, ControlFlow::Continue)) } } -/// The return builtin - return from a function. +/// The return builtin - return from a function or sourced file. /// Bash truncates return codes to 8-bit unsigned range (0-255) via `& 0xFF`. +/// Without an argument it returns `$?`; outside a function or sourced file +/// it fails with status 2 and the script goes on. pub struct Return; #[async_trait] impl Builtin for Return { async fn execute(&self, ctx: Context<'_>) -> Result { - let exit_code = ctx - .args - .first() - .and_then(|s| s.parse::().ok()) - .unwrap_or(0) - & 0xFF; - - Ok(ExecResult::with_control_flow(ControlFlow::Return( - exit_code, - ))) + if let Some(shell) = ctx.shell.as_ref() + && shell.return_depth == 0 + { + return Ok(ExecResult::err( + "bash: return: can only `return' from a function or sourced script\n", + 2, + )); + } + let exit_code = match ctx.args.first() { + None => ctx.shell.as_ref().map_or(0, |s| s.last_exit_code), + Some(arg) => match arg.trim().parse::() { + Ok(n) => (n & 0xFF) as i32, + Err(_) => { + let mut result = ExecResult::with_control_flow(ControlFlow::Return(2)); + result.exit_code = 2; + result.stderr = format!("bash: return: {arg}: numeric argument required\n").into(); + return Ok(result); + } + }, + }; + if ctx.args.len() > 1 { + return Ok(ExecResult::err("bash: return: too many arguments\n", 1)); + } + + let mut result = ExecResult::with_control_flow(ControlFlow::Return(exit_code)); + result.exit_code = exit_code; + Ok(result) } } diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index 479009aff..2001aae4f 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -276,6 +276,11 @@ impl Interpreter { for part in &word.parts { match part { + WordPart::BadSubstitution(text) => { + return Err(crate::error::Error::LineAbort(format!( + "bash: {text}: bad substitution\n" + ))); + } WordPart::CompoundAssignment { .. } => { // Only declaration builtins take `name=(...)`; elsewhere // the text stands for itself. @@ -431,6 +436,16 @@ impl Interpreter { let (is_set, value) = self.resolve_param_expansion_name(name); + // `${1:=x}`, `${@=x}`: only variables can be assigned. + if matches!(operator, ParameterOp::AssignDefault) + && !name.starts_with(|c: char| c.is_ascii_alphabetic() || c == '_') + && (!is_set || (*colon_variant && value.is_empty())) + { + return Err(crate::error::Error::LineAbort(format!( + "bash: ${name}: cannot assign in this way\n" + ))); + } + if self.is_nounset() && !suppress_nounset && !is_set { self.nounset_error = Some(format!("bash: {}: unbound variable\n", name)); } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index d8b0feb64..a5b15e990 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -370,6 +370,12 @@ pub(crate) struct ShellRef<'a> { pub(crate) stdout_pipe: Option>, /// Stdin of a streaming filter stage (see `Context::stdin_stream`). pub(crate) stdin_pipe: Option>, + /// Enclosing loops in the current function (`break`/`continue`). + pub(crate) loop_depth: usize, + /// Active function calls and `source`s (`return` is valid when > 0). + pub(crate) return_depth: usize, + /// `$?` when the builtin started (`return` without an argument). + pub(crate) last_exit_code: i32, } // Interpreter-dispatched "special" builtins, listed here so the public @@ -1527,6 +1533,12 @@ pub struct Interpreter { script_depth: usize, /// Nested `bash`/`sh` child shells (TM-DOS-125). child_shell_depth: usize, + /// Enclosing `for`/`while`/`until`/`select` loops in the current + /// function (bash `loop_level`). A function call starts at 0, so + /// `break`/`continue` never leave the caller's loop. + loop_depth: usize, + /// Active function calls and `source`s: `return` is valid only when > 0. + return_depth: usize, /// First arithmetic error raised by a read-only evaluation site (array /// subscripts, `declare -i` values, substring offsets). The command /// boundary turns it into a line abort, as bash does. @@ -2015,6 +2027,8 @@ impl Interpreter { concurrent_jobs: true, script_depth: 0, child_shell_depth: 0, + loop_depth: 0, + return_depth: 0, arith_error: StdMutex::new(None), pending_compound_args: Vec::new(), assign_raw: false, @@ -2179,6 +2193,8 @@ impl Interpreter { script_depth: 1, // Forks are polled on this task's stack, so they inherit its depth. child_shell_depth: self.child_shell_depth, + loop_depth: self.loop_depth, + return_depth: self.return_depth, arith_error: StdMutex::new(None), pending_compound_args: Vec::new(), assign_raw: false, @@ -2422,6 +2438,8 @@ impl Interpreter { // one cancelled script cannot suppress traps in the next script. self.in_trap = false; self.condition_sequence_depth = 0; + self.loop_depth = 0; + self.return_depth = 0; self.deferred_proc_subs.clear(); self.clear_pending_fd_redirect_state(); // Top-level timeouts drop the interpreter future at await points, so @@ -3200,6 +3218,7 @@ impl Interpreter { // line is skipped and the shell resumes at the next one. let mut aborted_line: Option = None; let mut propagate_abort = false; + let mut propagated_flow = ControlFlow::None; for command in &script.commands { if aborted_line == Some(Self::command_line(command)) { continue; @@ -3276,6 +3295,15 @@ impl Interpreter { // Stop on control flow (e.g. nounset error uses Return to abort) if result.control_flow != ControlFlow::None { if let ControlFlow::Exit(code) = result.control_flow { + if !run_exit_trap { + // `exit` in a `source`/`eval` body ends the shell: + // the caller's top level runs the exit hooks. + propagated_flow = ControlFlow::Exit(code); + exit_code = code; + self.last_exit_code = code; + stopped = true; + break; + } if fire_exit_hook { if !self.hooks.on_exit.is_empty() { self.execution_budget.consume_work(100)?; @@ -3294,6 +3322,16 @@ impl Interpreter { break; } } else { + // `return` ends a sourced file with its status; + // `break`/`continue`/`return` in a `source`/`eval` + // body act on the caller's loop or function. + if let ControlFlow::Return(code) = result.control_flow { + exit_code = code; + self.last_exit_code = code; + } + if !run_exit_trap { + propagated_flow = result.control_flow; + } stopped = true; break; } @@ -3407,7 +3445,7 @@ impl Interpreter { control_flow: if propagate_abort { ControlFlow::Abort } else { - ControlFlow::None + propagated_flow }, stdout_truncated, stderr_truncated, @@ -3619,12 +3657,25 @@ impl Interpreter { async fn execute_compound(&mut self, compound: &CompoundCommand) -> Result { match compound { CompoundCommand::If(if_cmd) => self.execute_if(if_cmd).await, - CompoundCommand::For(for_cmd) => self.execute_for(for_cmd).await, - CompoundCommand::ArithmeticFor(arith_for) => { - self.execute_arithmetic_for(arith_for).await + CompoundCommand::For(_) + | CompoundCommand::ArithmeticFor(_) + | CompoundCommand::While(_) + | CompoundCommand::Until(_) + | CompoundCommand::Select(_) => { + self.loop_depth += 1; + let result = match compound { + CompoundCommand::For(for_cmd) => self.execute_for(for_cmd).await, + CompoundCommand::ArithmeticFor(arith_for) => { + self.execute_arithmetic_for(arith_for).await + } + CompoundCommand::While(while_cmd) => self.execute_while(while_cmd).await, + CompoundCommand::Until(until_cmd) => self.execute_until(until_cmd).await, + CompoundCommand::Select(select_cmd) => self.execute_select(select_cmd).await, + _ => unreachable!("loop compound matched above"), + }; + self.loop_depth -= 1; + result } - CompoundCommand::While(while_cmd) => self.execute_while(while_cmd).await, - CompoundCommand::Until(until_cmd) => self.execute_until(until_cmd).await, CompoundCommand::Subshell(commands) => { self.counters.push_subshell(&self.limits)?; // Subshells run in fully isolated scope: variables, arrays, @@ -3704,7 +3755,6 @@ impl Interpreter { } CompoundCommand::BraceGroup(commands) => self.execute_command_sequence(commands).await, CompoundCommand::Case(case_cmd) => self.execute_case(case_cmd).await, - CompoundCommand::Select(select_cmd) => self.execute_select(select_cmd).await, CompoundCommand::Arithmetic(expr) => { self.execute_arithmetic_command(&crate::parser::arith_exec_text(expr)) .await @@ -5419,7 +5469,12 @@ impl Interpreter { self.update_bash_source(); } + // A new shell is outside any loop, function or sourced file. + let saved_loop_depth = std::mem::replace(&mut self.loop_depth, 0); + let saved_return_depth = std::mem::replace(&mut self.return_depth, 0); let result = self.execute_script_body(&script, true, false).await; + self.loop_depth = saved_loop_depth; + self.return_depth = saved_return_depth; // Restore BASH_SOURCE if script_file.is_some() { @@ -7586,6 +7641,9 @@ impl Interpreter { execution_extensions, stdout_pipe: None, stdin_pipe: None, + loop_depth: self.loop_depth, + return_depth: self.return_depth, + last_exit_code: self.last_exit_code, }; let plan_ctx = builtins::Context { args, @@ -7653,6 +7711,9 @@ impl Interpreter { execution_extensions, stdout_pipe: self.builtin_stdout_pipe.take(), stdin_pipe: self.builtin_stdin_pipe.take(), + loop_depth: self.loop_depth, + return_depth: self.return_depth, + last_exit_code: self.last_exit_code, }; let ctx = builtins::Context { args, @@ -8280,7 +8341,12 @@ impl Interpreter { let shares_caller_stdin = stdin.is_some() && stdin == prev_pipeline_stdin; self.pipeline_stdin = stdin; + // A new process is outside any loop, function or sourced file. + let saved_loop_depth = std::mem::replace(&mut self.loop_depth, 0); + let saved_return_depth = std::mem::replace(&mut self.return_depth, 0); let result = self.execute_script_body(&script, true, false).await; + self.loop_depth = saved_loop_depth; + self.return_depth = saved_return_depth; let child_stdin_left = self.pipeline_stdin.take(); // Restore full parent state — child mutations don't propagate @@ -8442,7 +8508,9 @@ impl Interpreter { // Execute the script commands in the current shell context. // Use execute_script_body (not execute) to preserve depth counters. + self.return_depth += 1; let exec_result = self.execute_script_body(&script, false, true).await; + self.return_depth -= 1; // Pop source depth and BASH_SOURCE (always, even on error) self.counters.pop_function(); @@ -8450,6 +8518,11 @@ impl Interpreter { self.update_bash_source(); let mut result = exec_result?; + // `return` ends the sourced file with its status. + if let ControlFlow::Return(code) = result.control_flow { + result.exit_code = code; + result.control_flow = ControlFlow::None; + } // Restore positional parameters if has_source_args { @@ -8685,6 +8758,7 @@ impl Interpreter { out.push_str(&word.to_string()); } WordPart::Literal(s) => Self::push_alias_reparse_literal(out, s, false), + WordPart::BadSubstitution(text) => out.push_str(text), WordPart::Variable(name) => out.push_str(&format!("${}", name)), WordPart::CommandSubstitution(cmd) => out.push_str(&format!("$({:?})", cmd)), WordPart::ArithmeticExpansion(expr) => out.push_str(&format!("$(({}))", expr)), @@ -8899,7 +8973,12 @@ impl Interpreter { }; // Execute function body. Always restore call state even on error. + // The body starts outside any loop (bash resets `loop_level`). + let saved_loop_depth = std::mem::replace(&mut self.loop_depth, 0); + self.return_depth += 1; let result = self.execute_command(&func_def.body).await; + self.return_depth -= 1; + self.loop_depth = saved_loop_depth; if let Some(cb) = saved_callback { self.output_callback = Some(cb); } diff --git a/crates/bashkit/src/parser/ast.rs b/crates/bashkit/src/parser/ast.rs index c54f71741..5ed0edd84 100644 --- a/crates/bashkit/src/parser/ast.rs +++ b/crates/bashkit/src/parser/ast.rs @@ -323,6 +323,7 @@ impl fmt::Display for Word { match part { WordPart::Literal(s) => write!(f, "{}", s)?, WordPart::Variable(name) => write!(f, "${}", name)?, + WordPart::BadSubstitution(text) => write!(f, "{}", text)?, WordPart::CommandSubstitution(cmd) => write!(f, "$({:?})", cmd)?, WordPart::ArithmeticExpansion(expr) => write!(f, "$(({}))", expr)?, WordPart::ParameterExpansion { @@ -511,6 +512,10 @@ pub enum WordPart { }, /// Parameter transformation `${var@op}` where op is Q, E, P, A, K, a, u, U, L Transformation { name: String, operator: char }, + /// A `${...}` bash cannot expand (`${x!}`, `${}`): parsing succeeds and + /// expanding it reports "bad substitution" and abandons the line. + /// Holds the source text, `${` and `}` included. + BadSubstitution(String), } /// Parameter expansion operators diff --git a/crates/bashkit/src/parser/mod.rs b/crates/bashkit/src/parser/mod.rs index 3f3c525de..d3b087db3 100644 --- a/crates/bashkit/src/parser/mod.rs +++ b/crates/bashkit/src/parser/mod.rs @@ -3504,8 +3504,14 @@ impl<'a> Parser<'a> { // ${VAR} format with possible parameter expansion chars.next(); // consume '{' - // Check for ${#var} or ${#arr[@]} - length expansion - if chars.peek() == Some(&'#') { + if let Some(inner) = bad_brace_parameter(&chars) { + // Bash accepts the word and fails when expanding it. + for _ in 0..=inner.chars().count() { + chars.next(); + } + push_part!(WordPart::BadSubstitution(format!("${{{inner}}}"))); + } else if chars.peek() == Some(&'#') { + // ${#var} or ${#arr[@]} - length expansion chars.next(); // consume '#' let mut var_name = String::new(); while let Some(&c) = chars.peek() { @@ -4279,6 +4285,109 @@ fn empty_background(span: Span) -> (ListOperator, Command) { /// Whether a raw `=~` operand holds an expansion. A `$` that cannot start /// one (`^a$`, `(x$|y$)`) is the regex end anchor; single quotes hide both. +/// Text inside `${...}` when bash would reject it with "bad substitution" +/// at expansion time (`${x!}`, `${}`, `${#x:2}`, `${ x}`), else `None`. +/// +/// `chars` is positioned just after the `{`. Only the parameter part is +/// checked: a valid name (identifier, digits or one special character, with +/// an optional subscript) must be followed by `}` or an operator. `${!...}` +/// forms and anything involving quote/escape markers are left to the +/// regular parser, so this only ever rejects words bash rejects too. +fn bad_brace_parameter(chars: &std::iter::Peekable>) -> Option { + const MARKERS: [char; 3] = ['\x00', '\u{1e}', '\u{1f}']; + let mut it = chars.clone(); + let length_prefix = match it.peek() { + Some('!') => return None, + Some('#') => { + it.next(); + match it.peek() { + // `${#}`, `${##}`, `${#-}`, ...: length of a special parameter + // or `$#` itself; leave these to the regular parser. + Some(c) if c.is_ascii_alphanumeric() || *c == '_' => true, + _ => return None, + } + } + _ => false, + }; + let bad = match it.peek().copied() { + None => return None, + Some(c) if c.is_ascii_alphabetic() || c == '_' => { + while it + .peek() + .is_some_and(|c| c.is_ascii_alphanumeric() || *c == '_') + { + it.next(); + } + if it.peek() == Some(&'[') { + // Subscript: balanced brackets; unterminated ones are bad. + let mut depth = 0usize; + let mut closed = false; + for c in it.by_ref() { + match c { + _ if MARKERS.contains(&c) => return None, + '[' => depth += 1, + ']' => { + depth -= 1; + if depth == 0 { + closed = true; + break; + } + } + _ => {} + } + } + if !closed { + return None; + } + } + next_is_bad(it.peek().copied(), length_prefix) + } + Some(c) if c.is_ascii_digit() => { + while it.peek().is_some_and(char::is_ascii_digit) { + it.next(); + } + next_is_bad(it.peek().copied(), length_prefix) + } + Some('@' | '*' | '#' | '?' | '-' | '$' | '!') if !length_prefix => { + it.next(); + next_is_bad(it.peek().copied(), false) + } + Some(c) if MARKERS.contains(&c) => return None, + Some(_) => true, + }; + if !bad { + return None; + } + // Find the closing brace to report and skip the whole expansion. + let mut inner = String::new(); + let mut depth = 0usize; + for c in chars.clone() { + if MARKERS.contains(&c) || c == '\'' || c == '"' || c == '\\' { + return None; + } + match c { + '{' => depth += 1, + '}' if depth == 0 => return Some(inner), + '}' => depth -= 1, + _ => {} + } + inner.push(c); + } + None +} + +/// After the parameter of `${...}`: whether `next` cannot start an operator. +fn next_is_bad(next: Option, length_prefix: bool) -> bool { + match next { + None => false, + Some('\x00' | '\u{1e}' | '\u{1f}') => false, + Some('}') => false, + Some(_) if length_prefix => true, + Some(':' | '-' | '=' | '+' | '?' | '#' | '%' | '/' | '^' | ',' | '~' | '@') => false, + Some(_) => true, + } +} + fn cond_regex_expands(raw: &str) -> bool { let mut chars = raw.chars().peekable(); while let Some(c) = chars.next() { diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh index ff6fa9635..90392504b 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh @@ -79,7 +79,6 @@ yes ### bashbox_interpreter_core_set_e_ignores_a_negated_pipeline # set -e ignores a negated pipeline -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -e; ! true; echo yes ### expect yes @@ -117,7 +116,6 @@ set -o pipefail; false | true; echo $?; set +o pipefail; false | true; echo $? ### bashbox_interpreter_core_pipes_stderr_too # |& pipes stderr too -### skip: TODO bashbox corpus gap, bashkit output differs from real bash { echo out; echo err >&2; } |& cat ### expect out @@ -176,7 +174,6 @@ end ### bashbox_interpreter_core_break_and_continue_outside_a_loop # break and continue outside a loop -### skip: TODO bashbox corpus gap, bashkit output differs from real bash break; continue 2; echo $? ### expect 0 @@ -184,7 +181,6 @@ break; continue 2; echo $? ### bashbox_interpreter_core_break_inside_a_function_cannot_leave_the_caller_loop # break inside a function cannot leave the caller loop -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f() { break; }; for i in 1; do f; echo no; done ### expect no @@ -260,7 +256,6 @@ set -x; x=1 printenv x; y=2 ### bashbox_interpreter_core_bad_substitution_fails_only_its_command # bad substitution fails only its command -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo ${x!} echo after $? ### expect @@ -269,7 +264,6 @@ after 1 ### bashbox_interpreter_core_expansion_error_abandons_the_rest_of_its_line # expansion error abandons the rest of its line -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f() { echo ${x!}; echo in; } f; echo same echo next @@ -279,7 +273,6 @@ next ### bashbox_interpreter_core_cannot_assign_fails_only_its_command # cannot assign fails only its command -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo ${1:=x} echo after $? ### expect @@ -295,7 +288,6 @@ f() { return -1; }; f; echo $? ### bashbox_interpreter_core_return_outside_a_function # return outside a function -### skip: TODO bashbox corpus gap, bashkit output differs from real bash return; echo $? ### expect 2 @@ -303,7 +295,6 @@ return; echo $? ### bashbox_interpreter_core_export_tracks_later_assignments # export tracks later assignments -### skip: TODO bashbox corpus gap, bashkit output differs from real bash export X=1; X=2; printenv X; export Y; Y=3; printenv Y; export -n Z=4; echo $Z ### expect 2 @@ -320,7 +311,6 @@ readonly R=1; export R=2; echo $? ### bashbox_interpreter_core_export_lists_variables # export lists variables -### skip: TODO bashbox corpus gap, bashkit output differs from real bash export B=2 A=1; export | grep "[AB]=" ### expect declare -x A="1" @@ -357,7 +347,6 @@ readonly r=1; unset r; echo $? ### bashbox_interpreter_core_local_outside_a_function # local outside a function -### skip: TODO bashbox corpus gap, bashkit output differs from real bash local x=1; echo $? ### expect 1 @@ -373,7 +362,6 @@ f() { local a=1 b; b=2; echo $a$b; }; f; echo "[$a$b]" ### bashbox_interpreter_core_local_of_a_readonly_variable # local of a readonly variable -### skip: TODO bashbox corpus gap, bashkit output differs from real bash readonly r=1; f() { local r=2; }; f; echo $? ### expect 1 @@ -475,7 +463,6 @@ z ### bashbox_interpreter_core_return_ends_a_sourced_file # return ends a sourced file -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo "return 4; echo no" > s.sh; source s.sh; echo $? ### expect 4 @@ -483,7 +470,6 @@ echo "return 4; echo no" > s.sh; source s.sh; echo $? ### bashbox_interpreter_core_return_in_a_file_sourced_by_a_function # return in a file sourced by a function -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo "x=1; return 5; x=2" > s; f() { source ./s; echo "s=$? x=$x"; }; f ### expect s=5 x=1 @@ -507,7 +493,6 @@ b ### bashbox_interpreter_core_declare_without_a_value_keeps_the_variable # declare without a value keeps the variable -### skip: TODO bashbox corpus gap, bashkit output differs from real bash x=1; declare x; echo $x; declare -p x nope; echo $? ### expect 1 @@ -525,7 +510,6 @@ declare -a a=([0]="1" [1]="b c") ### bashbox_interpreter_core_declare_in_a_function_is_local_unless_g # declare in a function is local unless -g -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f() { declare x=1; declare -g g=2; }; f; echo "[$x][$g]" ### expect [][2] @@ -533,7 +517,6 @@ f() { declare x=1; declare -g g=2; }; f; echo "[$x][$g]" ### bashbox_interpreter_core_declare_a # declare -a -### skip: TODO bashbox corpus gap, bashkit output differs from real bash declare -a arr; arr[0]=x; echo ${arr[0]}; declare -a b=v; echo ${b[0]} ### expect x @@ -556,7 +539,6 @@ declare -x E=1; printenv E ### bashbox_interpreter_core_declare_alone_lists_variables # declare alone lists variables -### skip: TODO bashbox corpus gap, bashkit output differs from real bash q=1; declare | grep "^q=" ### expect q=1 @@ -579,7 +561,6 @@ let x=0; echo $?; let y=2 z=3; echo $y$z $? ### bashbox_interpreter_core_shift # shift -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -- a b c; shift; echo $*; shift 2; echo $# $?; shift; echo $? ### expect b c @@ -589,7 +570,6 @@ b c ### bashbox_interpreter_core_getopts_walks_grouped_options_and_arguments # getopts walks grouped options and arguments -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -- -ac -bval -- -z rest1; while getopts "ab:c" o; do echo "$o ${OPTARG-unset} $OPTIND"; done; shift $((OPTIND-1)); echo "rest: $*" ### expect a unset 1 @@ -625,7 +605,6 @@ a 0 ### bashbox_interpreter_core_getopts_restarts_when_optind_is_reset # getopts restarts when OPTIND is reset -### skip: TODO bashbox corpus gap, bashkit output differs from real bash getopts ab o -ab; echo $o; OPTIND=1; getopts xy o -x; echo $o ### expect a @@ -667,7 +646,6 @@ builtin ### bashbox_interpreter_core_type_describes_each_name # type describes each name -### skip: TODO bashbox corpus gap, bashkit output differs from real bash type nope cd ls; echo $? ### expect cd is a shell builtin @@ -677,7 +655,6 @@ ls is /usr/bin/ls ### bashbox_interpreter_core_command_v # command -v -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ :; }; command -v cd ls f nope; echo $?; command -v nope; echo $? ### expect cd @@ -834,7 +811,6 @@ echo "a:b:c:d" | { IFS=: read x y; echo "[$x][$y]"; } ### bashbox_interpreter_core_adjacent_separators_make_empty_fields # adjacent separators make empty fields -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo "a::b" | { IFS=: read -a x; echo "${#x[@]} [${x[1]}]"; } ### expect 3 [] @@ -842,7 +818,6 @@ echo "a::b" | { IFS=: read -a x; echo "${#x[@]} [${x[1]}]"; } ### bashbox_interpreter_core_unterminated_line_assigns_but_fails # unterminated line assigns but fails -### skip: TODO bashbox corpus gap, bashkit output differs from real bash printf "a b" | { read x; echo "$? [$x]"; } ### expect 1 [a b] @@ -850,7 +825,6 @@ printf "a b" | { read x; echo "$? [$x]"; } ### bashbox_interpreter_core_escaped_separator_stays_in_the_field # escaped separator stays in the field -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo "a\\ b c" | { read x y; echo "[$x][$y]"; } ### expect [a b][c] @@ -928,7 +902,6 @@ x='$(echo hi >&2; echo 1)'; [[ $x -eq 1 ]]; echo $?; x='1+1'; [[ $x -eq 2 ]]; ec ### bashbox_interpreter_core_assignment_subscripts_are_expanded_and_arithmetic_for_indexe # assignment subscripts are expanded, and arithmetic for indexed arrays -### skip: TODO bashbox corpus gap, bashkit output differs from real bash i=3; a[$i]=x; a[i+1]=y; a[-1]=z; b=([i+1]=p [6]=q); read 'c[1+1]' <<< r; declare -p a b c ### expect declare -a a=([3]="x" [4]="z") @@ -1001,7 +974,6 @@ a=b ### bashbox_interpreter_core_a_subscript_may_quote_or_escape_a # a subscript may quote or escape a ] -### skip: TODO bashbox corpus gap, bashkit output differs from real bash declare -A a b c; a["x]"]=1; b[\]]=2; c['y z']=3; declare -p a b c ### expect declare -A a=(["x]"]="1" ) @@ -1043,7 +1015,6 @@ source /tmp; echo $? ### bashbox_interpreter_core_set_o_keyword_shows_as_k # set -o keyword shows as k -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -o keyword; echo $-; set -o | grep -E '^(keyword|noexec) ' ### expect hkBc From 2b54e53430d74b5945c7bb74b3aedbf3cde2b0f2 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 01:25:54 +0000 Subject: [PATCH 2/7] fix(builtins): trap keeps $?, getopts OPTIND, builtin/hash/dirs/read -d '' parity (WIP) --- crates/bashkit/src/builtins/dirstack.rs | 73 +++++++++++-------- crates/bashkit/src/builtins/flow.rs | 10 +-- crates/bashkit/src/builtins/introspect.rs | 49 +++++++++++-- crates/bashkit/src/builtins/read.rs | 3 +- crates/bashkit/src/interpreter/mod.rs | 48 ++++++++++-- .../bash/bashbox-interpreter-core.test.sh | 8 -- 6 files changed, 133 insertions(+), 58 deletions(-) diff --git a/crates/bashkit/src/builtins/dirstack.rs b/crates/bashkit/src/builtins/dirstack.rs index bca7e08bd..d719fbf7a 100644 --- a/crates/bashkit/src/builtins/dirstack.rs +++ b/crates/bashkit/src/builtins/dirstack.rs @@ -24,15 +24,37 @@ fn normalize_path(base: &std::path::Path, target: &str) -> PathBuf { super::resolve_path(&PathBuf::from("/"), &path.to_string_lossy()) } -/// Format the stack as bash's `dirs` default: current dir followed by the -/// stack from top (most recent) to bottom. -fn format_stack(ctx: &Context<'_>) -> String { - let cwd = ctx.cwd.to_string_lossy().to_string(); - let mut parts = vec![cwd]; +/// bash `polite_directory_format`: a directory under `$HOME` is shown as +/// `~` plus the rest (`$HOME` of `/` or unset disables it). +fn polite(dir: &str, home: Option<&str>) -> String { + if let Some(home) = home.filter(|h| h.len() > 1) + && let Some(rest) = dir.strip_prefix(home) + && (rest.is_empty() || rest.starts_with('/')) + { + return format!("~{rest}"); + } + dir.to_string() +} + +/// Current dir followed by the stack from top (most recent) to bottom, +/// `~`-abbreviated unless `long`. +fn stack_entries(ctx: &Context<'_>, long: bool) -> Vec { + let home = if long { + None + } else { + ctx.variables.get("HOME").map(String::as_str) + }; + let cwd = ctx.cwd.to_string_lossy(); + let mut parts = vec![polite(&cwd, home)]; if let Some(shell) = ctx.shell.as_ref() { - parts.extend(shell.dir_stack.iter().rev().cloned()); + parts.extend(shell.dir_stack.iter().rev().map(|d| polite(d, home))); } - parts.join(" ") + parts +} + +/// Format the stack as bash's `dirs` default: one line, `~`-abbreviated. +fn format_stack(ctx: &Context<'_>) -> String { + stack_entries(ctx, false).join(" ") } /// The pushd builtin - push directory onto stack and cd. @@ -145,6 +167,7 @@ impl Builtin for Dirs { let mut clear = false; let mut per_line = false; let mut verbose = false; + let mut long = false; for arg in ctx.args.iter() { match arg.as_str() { @@ -154,7 +177,7 @@ impl Builtin for Dirs { verbose = true; per_line = true; } - "-l" => {} // long listing (we don't do ~ substitution anyway) + "-l" => long = true, _ => {} } } @@ -166,28 +189,18 @@ impl Builtin for Dirs { return Ok(ExecResult::ok(String::new())); } - if !verbose && !per_line { - // Default output doesn't need to walk the stack separately. - return Ok(ExecResult::ok(format!("{}\n", format_stack(&ctx)))); - } - - let cwd = ctx.cwd.to_string_lossy().to_string(); - // Stack from top (most recent) to bottom, borrowed (no clone). - let empty: Vec = Vec::new(); - let stack = ctx.shell.as_ref().map(|s| &*s.dir_stack).unwrap_or(&empty); - - if verbose { - let mut output = format!(" 0 {}\n", cwd); - for (n, dir) in stack.iter().rev().enumerate() { - output.push_str(&format!(" {} {}\n", n + 1, dir)); - } - Ok(ExecResult::ok(output)) + let entries = stack_entries(&ctx, long); + let output = if verbose { + entries + .iter() + .enumerate() + .map(|(n, dir)| format!(" {n} {dir}\n")) + .collect() + } else if per_line { + entries.iter().map(|dir| format!("{dir}\n")).collect() } else { - let mut output = format!("{}\n", cwd); - for dir in stack.iter().rev() { - output.push_str(&format!("{}\n", dir)); - } - Ok(ExecResult::ok(output)) - } + format!("{}\n", entries.join(" ")) + }; + Ok(ExecResult::ok(output)) } } diff --git a/crates/bashkit/src/builtins/flow.rs b/crates/bashkit/src/builtins/flow.rs index 9676595cd..69324e45a 100644 --- a/crates/bashkit/src/builtins/flow.rs +++ b/crates/bashkit/src/builtins/flow.rs @@ -73,10 +73,9 @@ impl Builtin for Exit { fn loop_control(name: &str, ctx: &Context<'_>, make: fn(u32) -> ControlFlow) -> ExecResult { // Builtins run without shell state (e.g. direct unit calls) keep the // plain unwinding behaviour. - let depth = ctx - .shell - .as_ref() - .map_or(u32::MAX, |s| u32::try_from(s.loop_depth).unwrap_or(u32::MAX)); + let depth = ctx.shell.as_ref().map_or(u32::MAX, |s| { + u32::try_from(s.loop_depth).unwrap_or(u32::MAX) + }); if depth == 0 { return ExecResult::err( format!("bash: {name}: only meaningful in a `for', `while', or `until' loop\n"), @@ -150,7 +149,8 @@ impl Builtin for Return { Err(_) => { let mut result = ExecResult::with_control_flow(ControlFlow::Return(2)); result.exit_code = 2; - result.stderr = format!("bash: return: {arg}: numeric argument required\n").into(); + result.stderr = + format!("bash: return: {arg}: numeric argument required\n").into(); return Ok(result); } }, diff --git a/crates/bashkit/src/builtins/introspect.rs b/crates/bashkit/src/builtins/introspect.rs index 541cda26a..0973a58ef 100644 --- a/crates/bashkit/src/builtins/introspect.rs +++ b/crates/bashkit/src/builtins/introspect.rs @@ -189,16 +189,53 @@ impl Builtin for Which { } } -/// `hash` builtin — no-op in sandboxed environment. +/// `hash` builtin. /// -/// In real bash, `hash` manages the command hash table for PATH lookups. -/// In bashkit's sandboxed environment there is no real PATH search cache, -/// so this is a no-op that always succeeds. +/// Bashkit keeps no command hash table (every lookup walks the virtual +/// PATH), so the table always reads as empty: a bare `hash` reports that, +/// `hash -r` succeeds, and `hash NAME` only checks that NAME resolves. pub struct Hash; #[async_trait] impl Builtin for Hash { - async fn execute(&self, _ctx: Context<'_>) -> Result { - Ok(ExecResult::ok(String::new())) + async fn execute(&self, ctx: Context<'_>) -> Result { + let mut names = Vec::new(); + let mut list_only = true; + let mut options_done = false; + for arg in ctx.args.iter() { + if !options_done && arg == "--" { + options_done = true; + } else if !options_done && arg.starts_with('-') && arg.len() > 1 { + // -r clears the (empty) table; -l/-t/-d/-p act on entries. + if arg.contains(['r', 'd', 'p', 't']) { + list_only = false; + } + } else { + names.push(arg.as_str()); + } + } + if names.is_empty() { + return Ok(if list_only { + ExecResult::ok("hash: hash table empty\n".to_string()) + } else { + ExecResult::ok(String::new()) + }); + } + let mut errors = String::new(); + for name in names { + let builtin = ctx + .shell + .as_ref() + .is_some_and(|s| s.has_builtin(name) || s.has_function(name)); + if !builtin && search_path(&ctx, name).await.is_none() { + errors.push_str(&format!("bash: hash: {name}: not found\n")); + } + } + let code = i32::from(!errors.is_empty()); + Ok(ExecResult { + stderr: errors.into(), + exit_code: code, + ..Default::default() + }) } } diff --git a/crates/bashkit/src/builtins/read.rs b/crates/bashkit/src/builtins/read.rs index 9f2d4766a..1ed7dd551 100644 --- a/crates/bashkit/src/builtins/read.rs +++ b/crates/bashkit/src/builtins/read.rs @@ -56,7 +56,8 @@ impl Builtin for Read { } else { &rest }; - delimiter = delim_str.chars().next(); + // `-d ''` reads up to a NUL byte. + delimiter = Some(delim_str.chars().next().unwrap_or('\0')); break; } 'n' | 'N' => { diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index a5b15e990..5efa9fa1e 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1930,6 +1930,8 @@ impl Interpreter { variables.insert("OSTYPE".to_string(), "linux-gnu".to_string()); variables.insert("HOSTTYPE".to_string(), "x86_64".to_string()); variables.insert("MACHTYPE".to_string(), "x86_64-pc-linux-gnu".to_string()); + // bash starts every shell with OPTIND=1 (getopts state). + variables.insert("OPTIND".to_string(), "1".to_string()); // BASH_VERSINFO array: (major minor patch build status machine) let mut arrays = HashMap::new(); @@ -5546,6 +5548,8 @@ impl Interpreter { next_vars.insert(name.to_string(), val.clone()); } } + // A new shell starts getopts afresh. + next_vars.insert("OPTIND".to_string(), "1".to_string()); *self.vars_mut() = next_vars; self.arrays_mut().clear(); self.assoc_arrays_mut().clear(); @@ -8401,7 +8405,10 @@ impl Interpreter { let filename = match args.first() { Some(f) => f, None => { - return Ok(ExecResult::err("source: filename argument required", 1)); + return Ok(ExecResult::err( + "bash: source: filename argument required\nsource: usage: source filename [arguments]\n", + 2, + )); } }; @@ -9250,9 +9257,14 @@ impl Interpreter { // function); an empty or unset OPTIND starts over at 1. let optind: usize = self.expand_variable("OPTIND").parse().unwrap_or(1); - // Check if we're past the end + // Check if we're past the end (bash leaves OPTIND just past the + // last argument). if optind < 1 || optind > parse_args.len() { self.set_variable(varname.clone(), "?".to_string()); + self.set_variable( + "OPTIND".to_string(), + optind.clamp(1, parse_args.len() + 1).to_string(), + ); return Ok(ExecResult { stdout: crate::StreamData::new(), stderr: crate::StreamData::new(), @@ -9267,9 +9279,14 @@ impl Interpreter { // Check if this is an option (starts with -) if !current_arg.starts_with('-') || current_arg == "-" || current_arg == "--" { self.set_variable(varname.clone(), "?".to_string()); - if current_arg == "--" { - self.set_variable("OPTIND".to_string(), (optind + 1).to_string()); - } + // OPTIND stays on the first operand (past a `--`), and getopts + // writes it back even when it was unset. + let next = if current_arg == "--" { + optind + 1 + } else { + optind + }; + self.set_variable("OPTIND".to_string(), next.to_string()); return Ok(ExecResult { stdout: crate::StreamData::new(), stderr: crate::StreamData::new(), @@ -9397,10 +9414,17 @@ impl Interpreter { let Some(name) = args.first() else { return Ok(ExecResult::ok(String::new())); }; - if !(Self::is_special_builtin_name(name) + let registered = Self::is_special_builtin_name(name) || self.builtins.contains_key(name.as_str()) - || self.has_host_builtin(name)) - { + || self.has_host_builtin(name); + // Same rule as `type`/`command -v`: a registered command that real + // bash runs from PATH (`cat`, `ls`) is a file, not a shell builtin, + // when the root filesystem provides it. + let is_shell_builtin = registered + && (Self::is_special_builtin_name(name) + || builtins::BASH_BUILTIN_NAMES.contains(&name.as_str()) + || self.resolve_command_path(name).await.is_none()); + if !is_shell_builtin { return Ok(ExecResult::err( format!("bash: builtin: {name}: not a shell builtin\n"), 1, @@ -11298,10 +11322,14 @@ impl Interpreter { .with_execution_budget(self.execution_budget.clone()) .parse() { + // The handler runs with `$?` of the trapping command and the + // shell keeps that status afterwards (bash restores it). + let saved_exit = self.last_exit_code; self.in_trap = true; let emit_before = self.output_emit_count; let result = self.execute_command_sequence(&trap_script.commands).await; self.in_trap = false; + self.last_exit_code = saved_exit; if let Ok(trap_result) = result { self.maybe_emit_output(&trap_result.stdout, &trap_result.stderr, emit_before); return (trap_result.stdout, trap_result.stderr); @@ -11331,10 +11359,14 @@ impl Interpreter { .with_execution_budget(self.execution_budget.clone()) .parse() { + // The handler runs with `$?` of the trapping command and the + // shell keeps that status afterwards (bash restores it). + let saved_exit = self.last_exit_code; self.in_trap = true; let emit_before = self.output_emit_count; let result = self.execute_command_sequence(&trap_script.commands).await; self.in_trap = false; + self.last_exit_code = saved_exit; if let Ok(trap_result) = result { self.maybe_emit_output(&trap_result.stdout, &trap_result.stderr, emit_before); stdout.append(&trap_result.stdout); diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh index 90392504b..9fec88f51 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh @@ -56,7 +56,6 @@ true || echo no; echo $? ### bashbox_interpreter_core_err_trap_fires_for_a_failing_command # ERR trap fires for a failing command -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap "echo caught \$?" ERR; false; echo next $? ### expect caught 1 @@ -432,7 +431,6 @@ shopt -s nullglob; echo $? ### bashbox_interpreter_core_source_without_a_file # source without a file -### skip: TODO bashbox corpus gap, bashkit output differs from real bash source; echo $? ### expect 2 @@ -613,7 +611,6 @@ x ### bashbox_interpreter_core_getopts_stops_at_a_non_option # getopts stops at a non-option -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -- a -b; getopts b o; echo "$? $o $OPTIND" ### expect 1 ? 1 @@ -682,7 +679,6 @@ command cd /tmp; pwd ### bashbox_interpreter_core_builtin # builtin -### skip: TODO bashbox corpus gap, bashkit output differs from real bash builtin cd /tmp && pwd; builtin echo hi; builtin cat; echo $?; builtin; echo $? ### expect /tmp @@ -746,7 +742,6 @@ done ### bashbox_interpreter_core_err_trap_keeps # ERR trap keeps $? -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap "true" ERR; false; echo $? ### expect 1 @@ -780,7 +775,6 @@ pushd /nope; echo $? ### bashbox_interpreter_core_pushd_abbreviates_home # pushd abbreviates HOME -### skip: TODO bashbox corpus gap, bashkit output differs from real bash cd /tmp; pushd ~ ### expect ~ /tmp @@ -788,7 +782,6 @@ cd /tmp; pushd ~ ### bashbox_interpreter_core_hash # hash -### skip: TODO bashbox corpus gap, bashkit output differs from real bash hash; hash -r; : a b; echo $? ### expect hash: hash table empty @@ -839,7 +832,6 @@ printf "x y" | { IFS= read -r v; echo "[$v]"; } ### bashbox_interpreter_core_nul_delimited_records # NUL delimited records -### skip: TODO bashbox corpus gap, bashkit output differs from real bash printf "a\0b\0" | while read -r -d "" v; do echo "[$v]"; done ### expect [a] From 7d67f522d09683d45a745ce68474217e5c5cf984 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 01:32:09 +0000 Subject: [PATCH 3/7] fix(interpreter): set/alias/trap listings, [[ ]] arithmetic operands and regex errors, unset options (WIP) --- crates/bashkit/src/builtins/alias.rs | 5 +- crates/bashkit/src/builtins/helpers.rs | 6 + crates/bashkit/src/builtins/trap.rs | 201 ++++++++++++--- crates/bashkit/src/interpreter/mod.rs | 231 ++++++++++++++---- .../bash/bashbox-interpreter-core.test.sh | 8 - 5 files changed, 361 insertions(+), 90 deletions(-) diff --git a/crates/bashkit/src/builtins/alias.rs b/crates/bashkit/src/builtins/alias.rs index c39147b76..6333413cf 100644 --- a/crates/bashkit/src/builtins/alias.rs +++ b/crates/bashkit/src/builtins/alias.rs @@ -4,6 +4,7 @@ use async_trait::async_trait; +use super::helpers::single_quote; use super::{Builtin, Context}; use crate::error::Result; use crate::interpreter::ExecResult; @@ -29,7 +30,7 @@ impl Builtin for Alias { sorted.sort_by_key(|(k, _)| (*k).clone()); let mut output = String::new(); for (name, value) in sorted { - output.push_str(&format!("alias {}='{}'\n", name, value)); + output.push_str(&format!("alias {name}={}\n", single_quote(value))); } return Ok(ExecResult::ok(output)); } @@ -47,7 +48,7 @@ impl Builtin for Alias { } else { // alias name — show the alias if let Some(value) = shell.aliases.get(arg.as_str()) { - output.push_str(&format!("alias {}='{}'\n", arg, value)); + output.push_str(&format!("alias {arg}={}\n", single_quote(value))); } else { stderr.push_str(&format!("bash: alias: {}: not found\n", arg)); exit_code = 1; diff --git a/crates/bashkit/src/builtins/helpers.rs b/crates/bashkit/src/builtins/helpers.rs index a2b9c60df..6306ea16d 100644 --- a/crates/bashkit/src/builtins/helpers.rs +++ b/crates/bashkit/src/builtins/helpers.rs @@ -11,6 +11,12 @@ use super::check_help_version; use crate::interpreter::ExecResult; +/// bash `sh_single_quote`: wrap in single quotes, each `'` as `'\''`. +/// Used by listings meant to be re-read as input (`alias`, `trap`). +pub(crate) fn single_quote(s: &str) -> String { + format!("'{}'", s.replace('\'', "'\\''")) +} + /// Build the error real GNU coreutils/util-linux print for an unrecognized /// command-line option, so builtins reject typos the way the real tools do /// instead of silently ignoring the bad flag. diff --git a/crates/bashkit/src/builtins/trap.rs b/crates/bashkit/src/builtins/trap.rs index 96bb394b9..51e19aeba 100644 --- a/crates/bashkit/src/builtins/trap.rs +++ b/crates/bashkit/src/builtins/trap.rs @@ -1,21 +1,80 @@ //! Trap builtin — register signal/event handlers. //! //! Directly mutates traps via [`ShellRef`](super::ShellRef). +//! +//! Handlers are keyed by canonical name: `EXIT`, `DEBUG`, `ERR`, `RETURN`, +//! or a signal name without its `SIG` prefix (`INT`). Specs are accepted the +//! way bash takes them (`0`, `2`, `int`, `SIGINT`) and listed in bash's +//! order (EXIT, signals by number, DEBUG, ERR, RETURN) with re-readable +//! single quoting. use async_trait::async_trait; +use super::helpers::single_quote; use super::{Builtin, Context}; use crate::error::Result; use crate::interpreter::ExecResult; +/// Linux signal names, numbered from 1. +const SIGNALS: [&str; 31] = [ + "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", "USR2", + "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", "URG", + "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "IO", "PWR", "SYS", +]; + +/// Pseudo-signals after the real ones, in bash's listing order. +const PSEUDO: [&str; 3] = ["DEBUG", "ERR", "RETURN"]; + +/// Canonical key and listing rank for a trap spec, `None` if invalid. +fn canonical(spec: &str) -> Option<(usize, String)> { + if !spec.is_empty() && spec.bytes().all(|b| b.is_ascii_digit()) { + let n: usize = spec.parse().ok()?; + return match n { + 0 => Some((0, "EXIT".to_string())), + n if n <= SIGNALS.len() => Some((n, SIGNALS[n - 1].to_string())), + _ => None, + }; + } + let upper = spec.to_ascii_uppercase(); + let bare = upper.strip_prefix("SIG").unwrap_or(&upper); + if bare == "EXIT" { + return Some((0, "EXIT".to_string())); + } + if let Some(i) = SIGNALS.iter().position(|s| *s == bare) { + return Some((i + 1, bare.to_string())); + } + // DEBUG/ERR/RETURN take no `SIG` prefix. + PSEUDO + .iter() + .position(|s| *s == upper) + .map(|i| (SIGNALS.len() + 1 + i, upper)) +} + +/// Listing name of a canonical key (`INT` → `SIGINT`). +fn display_name(key: &str) -> String { + if key == "EXIT" || PSEUDO.contains(&key) { + key.to_string() + } else { + format!("SIG{key}") + } +} + +fn format_trap(key: &str, cmd: &str) -> String { + format!("trap -- {} {}\n", single_quote(cmd), display_name(key)) +} + +fn invalid_spec(spec: &str) -> String { + format!("bash: trap: {spec}: invalid signal specification\n") +} + /// `trap` builtin — register signal/event handlers. /// /// Usage: -/// - `trap` — list all traps -/// - `trap -p [SIGNAL...]` — print trap commands -/// - `trap COMMAND SIGNAL...` — set trap handler -/// - `trap - SIGNAL...` — remove trap handler -/// - `trap SIGNAL` (single arg) — remove trap for SIGNAL +/// - `trap` / `trap -p` — list all traps +/// - `trap -p SIGNAL...` — print the given traps +/// - `trap -l` — list signal names +/// - `trap COMMAND SIGNAL...` — set trap handler (`''` ignores the signal) +/// - `trap - SIGNAL...` / `trap SIGNAL` — reset to the default pub struct Trap; #[async_trait] @@ -25,52 +84,120 @@ impl Builtin for Trap { return Ok(ExecResult::ok(String::new())); }; - if ctx.args.is_empty() { - // List all traps - let mut sorted: Vec<_> = shell.traps.iter().collect(); - sorted.sort_by_key(|(sig, _)| (*sig).clone()); - let mut output = String::new(); - for (sig, cmd) in sorted { - output.push_str(&format!("trap -- '{}' {}\n", cmd, sig)); + let mut args: &[String] = ctx.args; + let mut print = false; + while let Some(first) = args.first() { + match first.as_str() { + "--" => { + args = &args[1..]; + break; + } + "-p" => { + print = true; + args = &args[1..]; + } + "-l" => { + let mut output = String::new(); + for (i, name) in SIGNALS.iter().enumerate() { + output.push_str(&format!("{:2}) SIG{name}\n", i + 1)); + } + return Ok(ExecResult::ok(output)); + } + _ => break, } - return Ok(ExecResult::ok(output)); } - // Handle -p flag (print traps) - if ctx.args[0] == "-p" { + if args.is_empty() || print { let mut output = String::new(); - if ctx.args.len() == 1 { - let mut sorted: Vec<_> = shell.traps.iter().collect(); - sorted.sort_by_key(|(sig, _)| (*sig).clone()); - for (sig, cmd) in sorted { - output.push_str(&format!("trap -- '{}' {}\n", cmd, sig)); + let mut stderr = String::new(); + if args.is_empty() { + let mut sorted: Vec<_> = shell + .traps + .iter() + .map(|(key, cmd)| { + let rank = canonical(key).map_or(usize::MAX, |(r, _)| r); + (rank, key, cmd) + }) + .collect(); + sorted.sort(); + for (_, key, cmd) in sorted { + output.push_str(&format_trap(key, cmd)); } } else { - for sig in &ctx.args[1..] { - let sig_upper = sig.to_uppercase(); - if let Some(cmd) = shell.traps.get(&sig_upper) { - output.push_str(&format!("trap -- '{}' {}\n", cmd, sig_upper)); + for spec in args { + match canonical(spec) { + Some((_, key)) => { + if let Some(cmd) = shell.traps.get(&key) { + output.push_str(&format_trap(&key, cmd)); + } + } + None => stderr.push_str(&invalid_spec(spec)), } } } - return Ok(ExecResult::ok(output)); + let code = i32::from(!stderr.is_empty()); + return Ok(ExecResult { + stdout: output.into(), + stderr: stderr.into(), + exit_code: code, + ..Default::default() + }); } - if ctx.args.len() == 1 { - let sig = ctx.args[0].to_uppercase(); - shell.traps.remove(&sig); + // `trap SIGNAL` and `trap N ...` (first operand a number) reset + // every operand; otherwise the first operand is the handler. + let reset_all = + args.len() == 1 || (!args[0].is_empty() && args[0].bytes().all(|b| b.is_ascii_digit())); + let (handler, specs) = if reset_all { + (None, args) } else { - let cmd = ctx.args[0].clone(); - for sig in &ctx.args[1..] { - let sig_upper = sig.to_uppercase(); - if cmd == "-" { - shell.traps.remove(&sig_upper); - } else { - shell.traps.insert(sig_upper, cmd.clone()); - } + let cmd = &args[0]; + (if cmd == "-" { None } else { Some(cmd) }, &args[1..]) + }; + let mut stderr = String::new(); + for spec in specs { + match canonical(spec) { + Some((_, key)) => match handler { + Some(cmd) => { + shell.traps.insert(key, cmd.clone()); + } + None => { + shell.traps.remove(&key); + } + }, + None => stderr.push_str(&invalid_spec(spec)), } } + let code = i32::from(!stderr.is_empty()); + Ok(ExecResult { + stderr: stderr.into(), + exit_code: code, + ..Default::default() + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn canonical_accepts_bash_spellings() { + assert_eq!(canonical("0"), Some((0, "EXIT".to_string()))); + assert_eq!(canonical("exit"), Some((0, "EXIT".to_string()))); + assert_eq!(canonical("2"), Some((2, "INT".to_string()))); + assert_eq!(canonical("sigint"), Some((2, "INT".to_string()))); + assert_eq!(canonical("TERM"), Some((15, "TERM".to_string()))); + assert_eq!(canonical("ERR").map(|(_, k)| k), Some("ERR".to_string())); + assert_eq!(canonical("SIGERR"), None); + assert_eq!(canonical("FOO"), None); + assert_eq!(canonical("99"), None); + assert_eq!(canonical("-1"), None); + } - Ok(ExecResult::ok(String::new())) + #[test] + fn listing_quotes_and_prefixes() { + assert_eq!(format_trap("INT", ""), "trap -- '' SIGINT\n"); + assert_eq!(format_trap("EXIT", "it's"), "trap -- 'it'\\''s' EXIT\n"); } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 5efa9fa1e..68c4c1f01 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1539,6 +1539,11 @@ pub struct Interpreter { loop_depth: usize, /// Active function calls and `source`s: `return` is valid only when > 0. return_depth: usize, + /// The last `[[ ]]` leaf was an invalid `=~` regex (status 2 if it + /// decides the result). + cond_regex_error: bool, + /// Diagnostics from `[[ ]]` leaves (arithmetic operand errors). + cond_stderr: String, /// First arithmetic error raised by a read-only evaluation site (array /// subscripts, `declare -i` values, substring offsets). The command /// boundary turns it into a line abort, as bash does. @@ -2031,6 +2036,8 @@ impl Interpreter { child_shell_depth: 0, loop_depth: 0, return_depth: 0, + cond_regex_error: false, + cond_stderr: String::new(), arith_error: StdMutex::new(None), pending_compound_args: Vec::new(), assign_raw: false, @@ -2197,6 +2204,8 @@ impl Interpreter { child_shell_depth: self.child_shell_depth, loop_depth: self.loop_depth, return_depth: self.return_depth, + cond_regex_error: false, + cond_stderr: String::new(), arith_error: StdMutex::new(None), pending_compound_args: Vec::new(), assign_raw: false, @@ -4241,7 +4250,12 @@ impl Interpreter { // Evaluate with lazy expansion to support short-circuit semantics. // In `[[ -n "${X:-}" && "$X" != "off" ]]`, if the left side is false, // the right side must NOT be expanded (to avoid set -u errors). - let result = self.evaluate_conditional_words(words).await?; + self.cond_regex_error = false; + self.cond_stderr.clear(); + let result = self.evaluate_conditional_words(words).await; + let regex_error = std::mem::take(&mut self.cond_regex_error); + let cond_stderr = std::mem::take(&mut self.cond_stderr); + let result = result?; // If a nounset error occurred during evaluation, propagate it. if let Some(err_msg) = self.nounset_error.take() { self.last_exit_code = 1; @@ -4252,12 +4266,17 @@ impl Interpreter { ..Default::default() }); } - let exit_code = if result { 0 } else { 1 }; + // An invalid regex that decides the result gives status 2. + let exit_code = match (result, regex_error) { + (true, _) => 0, + (false, true) => 2, + (false, false) => 1, + }; self.last_exit_code = exit_code; Ok(ExecResult { stdout: crate::StreamData::new(), - stderr: crate::StreamData::new(), + stderr: cond_stderr.into(), exit_code, control_flow: ControlFlow::None, ..Default::default() @@ -4367,7 +4386,9 @@ impl Interpreter { // Handle negation if Self::conditional_word_literal(&words[0]) == Some("!") { - return Ok(!self.evaluate_conditional_words(&words[1..]).await?); + let negated = !self.evaluate_conditional_words(&words[1..]).await?; + self.cond_regex_error = false; + return Ok(negated); } // Handle parentheses only when they wrap the whole expression. @@ -4410,6 +4431,7 @@ impl Interpreter { self.expand_word(word).await? }); } + self.cond_regex_error = false; Ok(self.evaluate_conditional(&expanded).await) }) } @@ -4502,30 +4524,12 @@ impl Interpreter { "!=" => !self.pattern_matches(&args[0], &args[2]), "<" => args[0] < args[2], ">" => args[0] > args[2], - "-eq" => { - args[0].parse::().unwrap_or(0) - == args[2].parse::().unwrap_or(0) - } - "-ne" => { - args[0].parse::().unwrap_or(0) - != args[2].parse::().unwrap_or(0) - } - "-lt" => { - args[0].parse::().unwrap_or(0) - < args[2].parse::().unwrap_or(0) - } - "-le" => { - args[0].parse::().unwrap_or(0) - <= args[2].parse::().unwrap_or(0) - } - "-gt" => { - args[0].parse::().unwrap_or(0) - > args[2].parse::().unwrap_or(0) - } - "-ge" => { - args[0].parse::().unwrap_or(0) - >= args[2].parse::().unwrap_or(0) - } + "-eq" => self.cond_int_cmp(&args[0], &args[2], |a, b| a == b), + "-ne" => self.cond_int_cmp(&args[0], &args[2], |a, b| a != b), + "-lt" => self.cond_int_cmp(&args[0], &args[2], |a, b| a < b), + "-le" => self.cond_int_cmp(&args[0], &args[2], |a, b| a <= b), + "-gt" => self.cond_int_cmp(&args[0], &args[2], |a, b| a > b), + "-ge" => self.cond_int_cmp(&args[0], &args[2], |a, b| a >= b), "=~" => self.regex_match(&args[0], &args[2]), "-nt" => { let lm = self.fs.stat(std::path::Path::new(&args[0])).await; @@ -4581,9 +4585,39 @@ impl Interpreter { self.is_variable_set(name) } + /// `[[ a -eq b ]]` and friends: both operands are arithmetic + /// expressions, evaluated as they stand (no second `$` expansion). An + /// invalid operand is reported and makes the test false. + fn cond_int_cmp(&mut self, left: &str, right: &str, cmp: fn(i64, i64) -> bool) -> bool { + let mut values = [0i64; 2]; + for (slot, operand) in values.iter_mut().zip([left, right]) { + let mut ev = arithmetic::ArithEval::new(self); + let r = ev.eval_str(operand); + let writes = std::mem::take(&mut ev.writes); + self.apply_arith_writes(writes); + match r { + Ok(v) => *slot = v, + Err(msg) => { + let msg = format!("{}: {msg}", operand.trim()); + let diag = self.arith_diag("[[: ", &msg); + self.cond_stderr.push_str(&diag); + return false; + } + } + } + cmp(values[0], values[1]) + } + /// Perform regex match and set BASH_REMATCH array. fn regex_match(&mut self, string: &str, pattern: &str) -> bool { - match self.regex_cache.get_or_compile(pattern) { + // POSIX ERE rejects unknown `[:class:]` names; the Rust engine would + // read them as plain bracket members. + let compiled = if ere_has_invalid_char_class(pattern) { + None + } else { + self.regex_cache.get_or_compile(pattern) + }; + match compiled { Some(re) => { if let Some(captures) = re.captures(string) { // Set BASH_REMATCH array @@ -4600,6 +4634,8 @@ impl Interpreter { } } None => { + // Invalid regex: status 2 when it decides `[[ ]]`. + self.cond_regex_error = true; self.arrays_mut().remove("BASH_REMATCH"); false } @@ -7914,6 +7950,11 @@ impl Interpreter { "let" => Some(self.execute_let_builtin(args, redirects).await), "unset" => Some(self.execute_unset_builtin(args, redirects).await), "getopts" => Some(self.execute_getopts(args, redirects).await), + // Bare `set`: the same sorted, quoted listing as `declare`. + "set" if args.is_empty() => { + let result = ExecResult::ok(self.format_set_listing()); + Some(self.apply_redirections(result, redirects).await) + } _ => None, } } @@ -7962,8 +8003,9 @@ impl Interpreter { stdin: Option, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { - // Interpreter-level special builtins - if Self::is_special_builtin_name(name) { + // Interpreter-level special builtins. A bare `set` lists every + // variable (arrays included), which needs interpreter state. + if Self::is_special_builtin_name(name) || (name == "set" && args.is_empty()) { return self .execute_special_builtin_with_hooks( name, @@ -9076,20 +9118,46 @@ impl Interpreter { args: &[String], redirects: &[Redirect], ) -> Result { + // Options come first (clusters like `-fn`, `--` ends them); the + // first operand ends option parsing (`unset x -n` unsets `-n`). let mut unset_nameref = false; let mut unset_function = false; - let mut var_args: Vec<&String> = Vec::new(); - for arg in args { - if arg == "-n" { - unset_nameref = true; - } else if arg == "-f" { - unset_function = true; - } else if arg == "-v" { - // -v (variable, default) - explicit variable mode - } else { - var_args.push(arg); + let mut unset_var = false; + let mut operands_start = args.len(); + for (i, arg) in args.iter().enumerate() { + if arg == "--" { + operands_start = i + 1; + break; + } + let Some(flags) = arg.strip_prefix('-').filter(|f| !f.is_empty()) else { + operands_start = i; + break; + }; + for flag in flags.chars() { + match flag { + 'n' => unset_nameref = true, + 'f' => unset_function = true, + 'v' => unset_var = true, + _ => { + let result = ExecResult::err( + format!( + "bash: unset: -{flag}: invalid option\nunset: usage: unset [-f] [-v] [-n] [name ...]\n" + ), + 2, + ); + return self.apply_redirections(result, redirects).await; + } + } } } + if unset_function && unset_var { + let result = ExecResult::err( + "bash: unset: cannot simultaneously unset a function and a variable\n", + 1, + ); + return self.apply_redirections(result, redirects).await; + } + let var_args: Vec<&String> = args[operands_start..].iter().collect(); let mut stderr = String::new(); let mut exit_code: i32 = 0; @@ -9099,6 +9167,12 @@ impl Interpreter { self.functions_mut().remove(arg.as_str()); continue; } + // Only an explicit `-v` checks the name (bash). + if unset_var && !is_valid_var_name(arg) && !Self::is_array_reference(arg) { + stderr.push_str(&format!("bash: unset: `{arg}': not a valid identifier\n")); + exit_code = 1; + continue; + } if let Some(bracket) = arg.find('[') && arg.ends_with(']') { @@ -9106,6 +9180,13 @@ impl Interpreter { let key = &arg[bracket + 1..arg.len() - 1]; let expanded_key = self.expand_variable_or_literal(key); let resolved_name = self.resolve_nameref(arr_name).to_string(); + if self.is_var_readonly(&resolved_name) { + stderr.push_str(&format!( + "bash: unset: {resolved_name}: cannot unset: readonly variable\n" + )); + exit_code = 1; + continue; + } // THREAT[TM-DOS-114]: releasing one element must give back both // its entry slot and its bytes, or repeated set/unset cycles // drift the budget until a healthy script is refused. @@ -9125,10 +9206,16 @@ impl Interpreter { } continue; } - if unset_nameref { + if unset_nameref && self.scoped.namerefs.contains_key(arg.as_str()) { self.remove_nameref(arg); } else { - let resolved = self.resolve_nameref(arg).to_string(); + // `-n` on a plain variable unsets it without following + // namerefs. + let resolved = if unset_nameref { + arg.to_string() + } else { + self.resolve_nameref(arg).to_string() + }; // THREAT[TM-INJ-009]: Block unset of internal marker variables if is_internal_variable(&resolved) { stderr.push_str(&format!( @@ -9160,6 +9247,12 @@ impl Interpreter { self.apply_redirections(result, redirects).await } + /// `name[subscript]` with a valid name (`unset -v 'a[1]'`). + fn is_array_reference(arg: &str) -> bool { + arg.find('[') + .is_some_and(|b| b > 0 && arg.ends_with(']') && is_valid_var_name(&arg[..b])) + } + /// Remove every live binding of `name`: scalar value, arrays, /// attributes, nameref and env entry. fn clear_live_binding(&mut self, name: &str) { @@ -11377,6 +11470,58 @@ impl Interpreter { } } +/// Whether an ERE uses a `[:name:]` character class POSIX does not define +/// (`[[:foo:]]`), which regcomp rejects. +fn ere_has_invalid_char_class(pattern: &str) -> bool { + const CLASSES: [&str; 12] = [ + "alnum", "alpha", "blank", "cntrl", "digit", "graph", "lower", "print", "punct", "space", + "upper", "xdigit", + ]; + let chars: Vec = pattern.chars().collect(); + let mut i = 0; + while i < chars.len() { + match chars[i] { + '\\' => i += 2, + '[' => { + // Bracket expression: a leading `^` and a first `]` are members. + i += 1; + if chars.get(i) == Some(&'^') { + i += 1; + } + if chars.get(i) == Some(&']') { + i += 1; + } + while i < chars.len() && chars[i] != ']' { + if chars[i] == '[' + && let Some(&kind @ (':' | '.' | '=')) = chars.get(i + 1) + { + let start = i + 2; + let mut end = start; + while end + 1 < chars.len() + && !(chars[end] == kind && chars[end + 1] == ']') + { + end += 1; + } + if end + 1 >= chars.len() { + return false; + } + let name: String = chars[start..end].iter().collect(); + if kind == ':' && !CLASSES.contains(&name.as_str()) { + return true; + } + i = end + 2; + } else { + i += 1; + } + } + i += 1; + } + _ => i += 1, + } + } + false +} + /// Turn a builtin's usage error into a failed command (exit 2) instead of /// aborting the script. THREAT[TM-INF-022]: the message is capped at 1 KB. fn builtin_usage_error(msg: &str) -> ExecResult { diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh index 9fec88f51..6e228ce45 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh @@ -368,7 +368,6 @@ readonly r=1; f() { local r=2; }; f; echo $? ### bashbox_interpreter_core_set_lists_variables_quoting_when_needed # set lists variables, quoting when needed -### skip: TODO bashbox corpus gap, bashkit output differs from real bash a=1; b="x y"; c=""; d="it's"; set | grep "^[abcd]=" ### expect a=1 @@ -703,7 +702,6 @@ still ### bashbox_interpreter_core_alias_define_list_and_query # alias define, list and query -### skip: TODO bashbox corpus gap, bashkit output differs from real bash alias a=b c="it's"; alias; alias a; alias zz; echo $? ### expect alias a='b' @@ -721,7 +719,6 @@ alias a=b; unalias a zz; echo $?; alias x=y; unalias -a; alias ### bashbox_interpreter_core_trap_listing_order_and_quoting # trap listing order and quoting -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap "echo b" ERR; trap "echo a" EXIT; trap "" INT; trap "it's" SIGTERM; trap ### expect trap -- 'echo a' EXIT @@ -733,7 +730,6 @@ a ### bashbox_interpreter_core_trap_with_a_lone_signal_resets_it # trap with a lone signal resets it -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap "echo x" 0; trap; trap EXIT; trap; echo done ### expect trap -- 'echo x' EXIT @@ -874,7 +870,6 @@ time -p true ### bashbox_interpreter_core_an_invalid_regex_fails_with_status_2 # an invalid regex fails [[ with status 2 -### skip: TODO bashbox corpus gap, bashkit output differs from real bash re='('; [[ a =~ $re ]]; echo $?; re='[[:foo:]]'; [[ a =~ $re ]]; echo $?; [[ abc =~ b(c) ]]; echo $? ${BASH_REMATCH[@]} ### expect 2 @@ -884,7 +879,6 @@ re='('; [[ a =~ $re ]]; echo $?; re='[[:foo:]]'; [[ a =~ $re ]]; echo $?; [[ abc ### bashbox_interpreter_core_evaluates_integer_operands_without_expanding_them_again # [[ ]] evaluates integer operands without expanding them again -### skip: TODO bashbox corpus gap, bashkit output differs from real bash x='$(echo hi >&2; echo 1)'; [[ $x -eq 1 ]]; echo $?; x='1+1'; [[ $x -eq 2 ]]; echo $?; [[ 08 -eq 1 ]]; echo $? ### expect 1 @@ -920,7 +914,6 @@ next 1 ### bashbox_interpreter_core_unset_takes_options_first # unset takes options first -### skip: TODO bashbox corpus gap, bashkit output differs from real bash unset -v x -n y; echo $?; unset -z; echo $?; unset -fn; echo $?; unset -n; echo $?; unset 'a b'; echo $? ### expect 1 @@ -932,7 +925,6 @@ unset -v x -n y; echo $?; unset -z; echo $?; unset -fn; echo $?; unset -n; echo ### bashbox_interpreter_core_unset_of_an_element_and_of_a_readonly_variable # unset of an element and of a readonly variable -### skip: TODO bashbox corpus gap, bashkit output differs from real bash a=(1 2 3); unset 'a[1]'; declare -p a; readonly r; unset -n r; echo $?; readonly -a ra=(1); unset 'ra[0]'; echo $? ### expect declare -a a=([0]="1" [2]="3") From 3f7e78ab86abf308bca6d9bcf358195a9067d804 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 01:35:31 +0000 Subject: [PATCH 4/7] fix(interpreter): set -k keyword assignments and out-of-range negative subscripts (WIP) --- crates/bashkit/src/interpreter/declare.rs | 4 +- crates/bashkit/src/interpreter/mod.rs | 85 +++++++++++++++++++ .../bash/bashbox-interpreter-core.test.sh | 4 +- 3 files changed, 89 insertions(+), 4 deletions(-) diff --git a/crates/bashkit/src/interpreter/declare.rs b/crates/bashkit/src/interpreter/declare.rs index 39b62e4b7..f7cdecbd9 100644 --- a/crates/bashkit/src/interpreter/declare.rs +++ b/crates/bashkit/src/interpreter/declare.rs @@ -1223,7 +1223,9 @@ impl Interpreter { self.set_assoc_element_checked(name.to_string(), key, v); } else { self.promote_scalar_to_indexed(name); - let idx = self.resolve_indexed_array_subscript(name, sub); + let idx = self + .indexed_write_subscript(name, sub) + .map_err(crate::error::Error::LineAbort)?; let old = if append { Some( self.scoped diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 68c4c1f01..b98c61824 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1227,6 +1227,7 @@ bitflags::bitflags! { const NOCLOBBER = 0b0000_0000_1000_0000; // set -C / SHOPT_C const PIPEFAIL = 0b0000_0001_0000_0000; // set -o pipefail / SHOPT_pipefail const EXPAND_ALIAS = 0b0000_0010_0000_0000; // shopt expand_aliases + const KEYWORD = 0b0000_0100_0000_0000; // set -k / SHOPT_k } } @@ -1244,6 +1245,7 @@ impl BashFlags { "SHOPT_C" => Some(Self::NOCLOBBER), "SHOPT_pipefail" => Some(Self::PIPEFAIL), "SHOPT_expand_aliases" => Some(Self::EXPAND_ALIAS), + "SHOPT_k" => Some(Self::KEYWORD), _ => None, } } @@ -6813,6 +6815,17 @@ impl Interpreter { stdin: Option, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + // `set -k`: assignment words anywhere in the command go to its + // environment, not its arguments. + let hoisted; + let command = if self.flags.contains(BashFlags::KEYWORD) + && command.args.iter().any(|w| keyword_assignment(w).is_some()) + { + hoisted = hoist_keyword_assignments(command); + &hoisted + } else { + command + }; let deferred_proc_sub_start = self.deferred_proc_subs.len(); let (_debug_stdout, _debug_stderr) = self.run_debug_trap().await; @@ -10619,6 +10632,31 @@ impl Interpreter { } } + /// Subscript for a write to an indexed array: a negative index counts + /// back from the end, and one before the start is an error + /// (`a[-5]=x` on three elements: "bad array subscript"). + fn indexed_write_subscript( + &self, + arr_name: &str, + key: &str, + ) -> std::result::Result { + let raw_idx = self.evaluate_arithmetic(key); + if raw_idx >= 0 { + return Ok(raw_idx as usize); + } + let len = self + .scoped + .arrays + .get(arr_name) + .and_then(|a| a.keys().max().map(|m| m.saturating_add(1) as i128)) + .unwrap_or(0); + let idx = len + raw_idx as i128; + if idx < 0 { + return Err(format!("bash: {arr_name}[{key}]: bad array subscript\n")); + } + Ok(idx as usize) + } + /// Resolve an indexed-array subscript the same way for read-before-write and write paths. fn resolve_indexed_array_subscript(&self, arr_name: &str, key: &str) -> usize { let raw_idx = self.evaluate_arithmetic(key); @@ -11470,6 +11508,53 @@ impl Interpreter { } } +/// `name=value` / `name+=value` argument with an unquoted, literal name: +/// an assignment under `set -k`. Returns the assignment. +fn keyword_assignment(word: &Word) -> Option { + let Some(WordPart::Literal(first)) = word.parts.first() else { + return None; + }; + // A quoted word without per-part flags was quoted as a whole. + let first_quoted = if word.part_quoted.iter().any(|q| *q) { + word.part_quoted[0] + } else { + word.quoted + }; + if first_quoted { + return None; + } + let eq = first.find('=')?; + let (name, append) = match first[..eq].strip_suffix('+') { + Some(n) => (n, true), + None => (&first[..eq], false), + }; + if !is_valid_var_name(name) { + return None; + } + let mut value = word.clone(); + value.raw = None; + value.parts[0] = WordPart::Literal(first[eq + 1..].to_string()); + Some(Assignment { + name: name.to_string(), + index: None, + value: AssignmentValue::Scalar(value), + append, + }) +} + +/// `set -k`: move assignment arguments into the command's assignments. +fn hoist_keyword_assignments(command: &SimpleCommand) -> SimpleCommand { + let mut hoisted = command.clone(); + hoisted.args.clear(); + for word in &command.args { + match keyword_assignment(word) { + Some(assignment) => hoisted.assignments.push(assignment), + None => hoisted.args.push(word.clone()), + } + } + hoisted +} + /// Whether an ERE uses a `[:name:]` character class POSIX does not define /// (`[[:foo:]]`), which regcomp rejects. fn ere_has_invalid_char_class(pattern: &str) -> bool { diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh index 6e228ce45..b2e26da04 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh @@ -904,7 +904,6 @@ declare -A m; k='a b'; m[$k]=1; m[1+1]=2; echo "${m[a b]} ${m[1+1]}" ### bashbox_interpreter_core_a_negative_subscript_counts_back_from_the_end # a negative subscript counts back from the end -### skip: TODO bashbox corpus gap, bashkit output differs from real bash a=(1 2 3); a[-1]+=x; declare -p a; a[-5]=q; echo same echo next $? ### expect @@ -947,7 +946,6 @@ set -o noexec; echo hi ### bashbox_interpreter_core_set_k_takes_assignments_from_anywhere_in_a_command # set -k takes assignments from anywhere in a command -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f() { echo "$x"; }; set -k; echo a=b c; f x=5; echo $-; set +k; echo a=b ### expect c @@ -974,7 +972,7 @@ x=1; unset -- x; echo $? ${x-unset} ### bashbox_interpreter_core_redirections_report_why_a_file_can_t_be_opened # redirections report why a file can't be opened -### skip: TODO bashbox corpus gap, bashkit output differs from real bash +### skip: TODO `<>` (read-write open) redirect is not parsed yet, and InMemoryFs lets `f/x` be created under a regular file `f` instead of failing with ENOTDIR true <> /tmp; echo $?; touch f; echo hi > f/x; echo $?; cat < f/x; echo $?; ln -s loop loop; echo hi > loop; echo $? ### expect 1 From c23cf26e44f4674066e34b36125790e65a232dbb Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 01:38:53 +0000 Subject: [PATCH 5/7] chore(knowledge): record interpreter-core builtin parity decisions --- knowledge/foundations/builtins.md | 30 ++++++++++++++++++++++++++++++ knowledge/log.md | 1 + 2 files changed, 31 insertions(+) diff --git a/knowledge/foundations/builtins.md b/knowledge/foundations/builtins.md index 0bd7d8ab4..8e4abcaf6 100644 --- a/knowledge/foundations/builtins.md +++ b/knowledge/foundations/builtins.md @@ -234,6 +234,36 @@ Internal builtins that need interpreter state receive it via `Context.shell`: - `unset`, functions, arrays, namerefs, call stack locals - `let`, arithmetic evaluation with assignment - `getopts`, complex variable + call stack interaction +- bare `set` (no arguments), the same sorted, quoted listing as `declare`, + which needs arrays; `set` with arguments stays a registered builtin + +### Control flow and shell-state builtins + +- `break`/`continue` act on the loops of the current function only: the + interpreter keeps `loop_depth` (reset to 0 on a function call, `bash -c` + and pipeline children; `source`/`eval` keep it), passed through `ShellRef`. + Outside a loop they warn and return 0; the count is clamped to the depth. +- `return` is valid only inside a function or sourced file (`return_depth`); + elsewhere it fails with status 2 and the script goes on. Without an + argument it returns `$?`. `return` ends a sourced file with its status, and + `break`/`continue`/`return`/`exit` in a `source`/`eval` body propagate to + the caller's loop, function or shell. +- Trap handlers (`ERR`, `DEBUG`) run with the trapping command's `$?` and + leave it unchanged. `trap` keys handlers canonically (`EXIT`, `INT`, + `DEBUG`, `ERR`, `RETURN`), accepts `0`/`2`/`int`/`SIGINT`, rejects unknown + specs (status 1) and lists in bash order (EXIT, signals by number, DEBUG, + ERR, RETURN) with `sh_single_quote` quoting, as `alias` does. +- `hash` keeps no table (every lookup walks the virtual PATH): a bare `hash` + reports `hash table empty`, `hash NAME` only checks that NAME resolves. +- `builtin NAME`, like `type`/`command -v`, treats a registered command that + real bash runs from PATH (`cat`) as a file when the root filesystem + provides it, so it is "not a shell builtin". +- `set -k` hoists unquoted `name=value` arguments into the command's + temporary environment at execution time. +- `[[ a -eq b ]]` evaluates each operand as an arithmetic expression without + a second `$` expansion; an invalid operand prints the arithmetic error and + makes the test false. An invalid `=~` regex (including an unknown + `[:class:]`) makes the deciding test return 2. ### Declaration builtins and variable scope diff --git a/knowledge/log.md b/knowledge/log.md index 822c93397..142d0abf1 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -79,6 +79,7 @@ * **Fix**: Arrays, scalars and `read` names follow bash. A scalar is element 0 of an array (`x=5; echo ${x[@]} ${#x[@]} ${!x[@]} ${x[0]}` prints `5 1 0 5`, `((x[1]=7))` promotes it); `${a[@]}`/`${!a[@]}` of an associative array use bash's hash order, like `declare -p`. `[[ -v name ]]` was missing (always true); it now tests set-ness, element 0 for an array, `name[sub]` and `name[@]`. `${!ref}` gives the end of a nameref chain, a nameref `for` variable is re-pointed at each word, and `read` through a circular nameref fails with status 1. `read` takes options only before the first name, stops with status 1 at an invalid name (earlier names keep their fields), validates `-a` names, and supports `-N` (exact count, no splitting). * **Fix**: Declaration builtins and local scope match bash. `declare`/`typeset`/`local`/`export`/`readonly` share one engine with bash's option sets, errors and `declare -p` format (quoting, assoc hash order); locals use shallow binding saved in the call frame, so nested functions, `unset` of a caller's local, `declare -g` past a local, and `local -n` behave as in bash. Compound operands stay one `CompoundAssignment` word part (keyed `[k]="v w"` and `` [k]=`cmd` `` elements lex as one word). Readonly assignment and circular namerefs abandon the line. See [Builtins](foundations/builtins.md#declaration-builtins-and-variable-scope). * **Fix**: Arithmetic follows bash's grammar and error model. New tokenizer + precedence-climbing evaluator (`interpreter/arithmetic.rs`): comma lists, chained/compound assignment, `**` right-assoc, `base#n` up to base 64, `noeval` short-circuit. `$((1/0))`, syntax errors and bad subscripts now print bash's message and abort the rest of the line (status 1) instead of yielding 0; `((...))` and `let` return 1. `((z <<= 65))` is read as raw text, not a heredoc. Bounds unchanged (`MAX_ARITHMETIC_DEPTH`, fuel; TM-DOS-026). See [Parser](foundations/parser.md#arithmetic-expressions). +* **Fix**: Interpreter-core parity from the BashBox corpus (bash-verified cases unskipped). `break`/`continue` never leave a caller's loop and warn outside one; `return` outside a function fails with 2, `return` with no argument returns `$?`, and `return` ends a sourced file; `${x!}`/`${}` report "bad substitution" and `${1:=x}` "cannot assign in this way", abandoning the line; ERR/DEBUG traps keep `$?`; `trap` canonicalizes specs and lists in bash order with re-readable quoting (as do `alias` and a bare `set`); `getopts` always writes `OPTIND` (initialized to 1); `unset` parses options only before operands and refuses readonly elements; `a[-5]=x` past the start is a bad subscript; `[[ -eq ]]` operands are arithmetic; an invalid `=~` regex returns 2; `set -k`; `read -d ''`; `dirs`/`pushd` abbreviate `$HOME` as `~`; `hash`, `builtin`, `source` error statuses. `<>` and ENOTDIR under a regular file stay open. See [Builtins](foundations/builtins.md#control-flow-and-shell-state-builtins). ## 2026-10-06 From 06b454e9338a714690f59510481f98aeff60a407 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 02:06:32 +0000 Subject: [PATCH 6/7] fix(builtins): break with a non-numeric count exits the shell; hash/source spec cases follow bash --- crates/bashkit/src/builtins/flow.rs | 14 ++++++++------ .../bashkit/tests/spec_cases/bash/source.test.sh | 5 ++--- crates/bashkit/tests/spec_cases/bash/type.test.sh | 4 ++-- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/crates/bashkit/src/builtins/flow.rs b/crates/bashkit/src/builtins/flow.rs index 69324e45a..b6369d308 100644 --- a/crates/bashkit/src/builtins/flow.rs +++ b/crates/bashkit/src/builtins/flow.rs @@ -94,10 +94,11 @@ fn loop_control(name: &str, ctx: &Context<'_>, make: fn(u32) -> ControlFlow) -> return result; } Err(_) => { - return ExecResult::err( - format!("bash: {name}: {arg}: numeric argument required\n"), - 128, - ); + // A special-builtin usage error ends a non-interactive shell. + let mut result = ExecResult::with_control_flow(ControlFlow::Exit(128)); + result.exit_code = 128; + result.stderr = format!("bash: {name}: {arg}: numeric argument required\n").into(); + return result; } }, }; @@ -318,13 +319,14 @@ mod tests { } #[tokio::test] - async fn break_non_numeric_defaults_one() { + async fn break_non_numeric_exits_the_shell() { let (fs, mut cwd, mut variables) = setup().await; let env = HashMap::new(); let args = vec!["abc".to_string()]; let ctx = Context::new_for_test(&args, &env, &mut variables, &mut cwd, fs.clone(), None); let result = Break.execute(ctx).await.unwrap(); - assert!(matches!(result.control_flow, ControlFlow::Break(1))); + assert!(matches!(result.control_flow, ControlFlow::Exit(128))); + assert!(result.stderr.contains("numeric argument required")); } // ==================== continue ==================== diff --git a/crates/bashkit/tests/spec_cases/bash/source.test.sh b/crates/bashkit/tests/spec_cases/bash/source.test.sh index 277b7561d..daa5098f7 100644 --- a/crates/bashkit/tests/spec_cases/bash/source.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/source.test.sh @@ -122,12 +122,11 @@ count=3 ### end ### source_missing_filename -# Source without filename argument returns non-zero -### bash_diff: error message wording differs +# Source without filename argument is a usage error (status 2) source echo $? ### expect -1 +2 ### end ### source_nonexistent_file diff --git a/crates/bashkit/tests/spec_cases/bash/type.test.sh b/crates/bashkit/tests/spec_cases/bash/type.test.sh index f4da6ac14..847cc7a59 100644 --- a/crates/bashkit/tests/spec_cases/bash/type.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/type.test.sh @@ -123,11 +123,11 @@ cat is /usr/bin/cat ### end ### hash_noop -### bash_diff: real bash prints hash table contents -# hash is a no-op in sandboxed env +# bashkit keeps no hash table, so it always reads as empty hash echo "ok" ### expect +hash: hash table empty ok ### end From 4bf34e630e311cb0f178e59bb52100fcc2ccf447 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 02:33:23 +0000 Subject: [PATCH 7/7] test(spec): anchor export listing grep so host env vars don't match --- .../tests/spec_cases/bash/bashbox-interpreter-core.test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh index b2e26da04..51a65b8a5 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-core.test.sh @@ -310,7 +310,7 @@ readonly R=1; export R=2; echo $? ### bashbox_interpreter_core_export_lists_variables # export lists variables -export B=2 A=1; export | grep "[AB]=" +export B=2 A=1; export | grep -E "^declare -x [AB]=" ### expect declare -x A="1" declare -x B="2"