From 7cf5efbf19990d7cd5dc41f81d1aea4a63b81f9c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 7 Oct 2026 22:01:15 +0000 Subject: [PATCH 1/2] fix(interpreter): arithmetic errors report and fail instead of giving 0 Division by zero and a literal outside its base now print bash's message and exit 1; (( )) and let report it themselves and keep the script running. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit/src/interpreter/arithmetic.rs | 64 ++++++++++++++-- crates/bashkit/src/interpreter/mod.rs | 73 ++++++++++++++++++- .../spec_cases/bash/arith-errors.test.sh | 39 ++++++++++ knowledge/log.md | 1 + 4 files changed, 167 insertions(+), 10 deletions(-) create mode 100644 crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh diff --git a/crates/bashkit/src/interpreter/arithmetic.rs b/crates/bashkit/src/interpreter/arithmetic.rs index 1be9433c2..d031f09f2 100644 --- a/crates/bashkit/src/interpreter/arithmetic.rs +++ b/crates/bashkit/src/interpreter/arithmetic.rs @@ -12,6 +12,7 @@ impl Interpreter { /// variable name is preserved. pub(super) fn evaluate_arithmetic_with_assign(&mut self, expr: &str) -> i64 { let expr = expr.trim(); + self.note_arith_expr(expr); // Handle comma operator (lowest precedence): evaluate all, return last // But not inside parentheses @@ -227,6 +228,7 @@ impl Interpreter { /// Evaluate arithmetic while carrying recursion depth from caller contexts. /// THREAT[TM-DOS-026]: Preserves the recursion guard across nested array index eval. pub(super) fn evaluate_arithmetic_depth(&self, expr: &str, depth: usize) -> i64 { + self.note_arith_expr(expr.trim()); let mut state = ArithmeticExpansionState::new(Self::MAX_ARITHMETIC_EXPANSION_FUEL); self.evaluate_arithmetic_depth_state(expr, depth, &mut state) } @@ -659,7 +661,7 @@ impl Interpreter { let value_str = &expr[hash_pos + 1..]; if let Ok(base) = base_str.parse::() { if (2..=36).contains(&base) { - return i64::from_str_radix(value_str, base).unwrap_or(0); + return self.radix_or_error(expr, value_str, base); } else if (37..=64).contains(&base) { return Self::parse_base_n(value_str, base); } @@ -668,10 +670,10 @@ impl Interpreter { // Hex (0x...), octal (0...) literals if expr.starts_with("0x") || expr.starts_with("0X") { - return i64::from_str_radix(&expr[2..], 16).unwrap_or(0); + return self.radix_or_error(expr, &expr[2..], 16); } if expr.starts_with('0') && expr.len() > 1 && expr.chars().all(|c| c.is_ascii_digit()) { - return i64::from_str_radix(&expr[1..], 8).unwrap_or(0); + return self.radix_or_error(expr, &expr[1..], 8); } // Parse as number or variable @@ -771,11 +773,20 @@ impl Interpreter { } '/' | '%' if depth == 0 => { let left = self.parse_arithmetic_impl(&expr[..bo[i]], arith_depth + 1); - let right = self.parse_arithmetic_impl(&expr[bo[i] + 1..], arith_depth + 1); + let rhs_text = &expr[bo[i] + 1..]; + let right = self.parse_arithmetic_impl(rhs_text, arith_depth + 1); + if right == 0 { + // An empty right side means this split was not the real + // operator (the scan also splits inside signed + // operands); only a written zero is an error. + if !rhs_text.trim().is_empty() { + self.set_arith_error("division by 0", rhs_text.trim()); + } + return Some(0); + } return Some(match chars[i] { - '/' if right != 0 => left.wrapping_div(right), - '%' if right != 0 => left.wrapping_rem(right), - _ => 0, + '/' => left.wrapping_div(right), + _ => left.wrapping_rem(right), }); } _ => {} @@ -1081,6 +1092,45 @@ impl Interpreter { self.parse_arith_atom(expr, arith_depth) } + /// Parse `digits` in `base`, or record bash's "value too great for base" + /// error (the error token is the whole literal, as bash prints it). + fn radix_or_error(&self, literal: &str, digits: &str, base: u32) -> i64 { + match i64::from_str_radix(digits, base) { + Ok(v) => v, + Err(_) => { + self.set_arith_error("value too great for base", literal.trim()); + 0 + } + } + } + + /// Remember the expression text for a later error message, while no error + /// is pending (the expression of the first error is the one bash prints). + fn note_arith_expr(&self, expr: &str) { + if let Ok(mut e) = self.arith_error.lock() + && e.failure.is_none() + { + e.expr = expr.to_string(); + e.line = self.current_line; + } + } + + /// Record an arithmetic error in bash's wording. The first error wins, as + /// bash stops evaluating at the first one. + pub(super) fn set_arith_error(&self, what: &str, token: &str) { + let Ok(mut e) = self.arith_error.lock() else { + return; + }; + if e.failure.is_some() { + return; + } + if e.expr.is_empty() { + e.expr = token.to_string(); + e.line = self.current_line; + } + e.failure = Some((what.to_string(), token.to_string())); + } + /// Parse a number in base 37-64 using bash's extended charset: 0-9, a-z, A-Z, @, _ pub(super) fn parse_base_n(value_str: &str, base: u32) -> i64 { let mut result: i64 = 0; diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index d36c9b3a6..76f827df7 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1346,6 +1346,31 @@ struct SubshellSnapshot { bash_subshell: u32, } +/// Pending arithmetic evaluation error: bash reports the first one, naming the +/// whole expression, and the command exits 1 instead of using 0. +#[derive(Default)] +pub(crate) struct ArithError { + /// What failed and the offending token, once an error happened. + failure: Option<(String, String)>, + /// Expression text of the evaluation in progress. + expr: String, + /// Line the expression was on. + line: usize, +} + +impl ArithError { + /// bash's wording. `prefix` names the builtin when one reports the error + /// itself (`((: `, `let: `); expansion errors carry no prefix. + fn message(&self, prefix: &str, expr: Option<&str>) -> Option { + let (what, token) = self.failure.as_ref()?; + let expr = expr.unwrap_or(&self.expr); + Some(format!( + "bash: line {}: {prefix}{expr}: {what} (error token is \"{token}\")\n", + self.line + )) + } +} + /// Interpreter state. pub struct Interpreter { fs: Arc, @@ -1470,6 +1495,12 @@ pub struct Interpreter { /// Outputs of the `$(...)` parts of a `${x:-...}`-family operand, run /// ahead (async) and consumed in order by the sync `expand_operand`. operand_substs: std::collections::VecDeque, + /// Pending arithmetic error message, and the expression text it came from. + /// A `Mutex` because the evaluator runs behind `&self` and `Interpreter` + /// must stay `Sync`; the lock is only taken on the arithmetic path. + /// Reported and cleared where `nounset_error` is, so the command aborts + /// with status 1 instead of silently using 0, as bash does. + arith_error: std::sync::Mutex, /// PIPESTATUS: exit codes of the last pipeline's commands pipestatus: Vec, /// Aliases currently being expanded (prevents infinite recursion). @@ -2039,6 +2070,7 @@ impl Interpreter { output_stream_stderr_bytes: 0, nounset_error: None, operand_substs: std::collections::VecDeque::new(), + arith_error: std::sync::Mutex::new(ArithError::default()), pipestatus: Vec::new(), expanding_aliases: HashSet::new(), regex_cache: RuntimeRegexCache::default(), @@ -2199,6 +2231,7 @@ impl Interpreter { output_stream_stderr_bytes: 0, nounset_error: None, operand_substs: std::collections::VecDeque::new(), + arith_error: std::sync::Mutex::new(ArithError::default()), pipestatus: Vec::new(), expanding_aliases: HashSet::new(), regex_cache: RuntimeRegexCache::default(), @@ -4141,7 +4174,7 @@ impl Interpreter { // the right side must NOT be expanded (to avoid set -u errors). let result = self.evaluate_conditional_words(words).await?; // If a nounset error occurred during evaluation, propagate it. - if let Some(err_msg) = self.nounset_error.take() { + if let Some(err_msg) = self.take_expansion_error() { self.last_exit_code = 1; return Ok(ExecResult { stderr: err_msg.into(), @@ -4488,6 +4521,11 @@ impl Interpreter { async fn execute_arithmetic_command(&mut self, expr: &str) -> Result { let result = self.execute_arithmetic_with_side_effects(expr); + // `(( ))` reports the error itself and fails; it does not abort the + // script the way a failed `$(( ))` expansion does. + if let Some(msg) = self.take_arith_error("((: ") { + return Ok(ExecResult::err(msg, 1)); + } let exit_code = if result != 0 { 0 } else { 1 }; Ok(ExecResult { @@ -6737,6 +6775,28 @@ impl Interpreter { } } + /// The pending expansion error, if any: an arithmetic failure first, then a + /// `set -u` failure. Clears both. + fn take_expansion_error(&mut self) -> Option { + let arith = self.take_arith_error(""); + let nounset = self.nounset_error.take(); + arith.or(nounset) + } + + /// Take the pending arithmetic error, formatted with `prefix`. + fn take_arith_error(&self, prefix: &str) -> Option { + self.take_arith_error_for(prefix, None) + } + + /// As [`Self::take_arith_error`], with the expression text the caller saw + /// (`let` reports its whole argument, assignment included). + fn take_arith_error_for(&self, prefix: &str, expr: Option<&str>) -> Option { + let mut e = self.arith_error.lock().ok()?; + let msg = e.message(prefix, expr); + *e = ArithError::default(); + msg + } + /// Try alias expansion. Returns `Some(result)` if alias was expanded, `None` otherwise. async fn try_alias_expansion( &mut self, @@ -6963,7 +7023,7 @@ impl Interpreter { } }; - if let Some(err_msg) = self.nounset_error.take() { + if let Some(err_msg) = self.take_expansion_error() { self.last_exit_code = 1; self.discard_deferred_proc_subs_from(deferred_proc_sub_start); return Ok(ExecResult { @@ -7216,7 +7276,7 @@ impl Interpreter { } // Check for nounset error from argument expansion - if let Some(err_msg) = self.nounset_error.take() { + if let Some(err_msg) = self.take_expansion_error() { self.last_exit_code = 1; return Ok(ExecResult { stdout: crate::StreamData::new(), @@ -8390,6 +8450,9 @@ impl Interpreter { self.coproc_buffers.clear(); self.last_exit_code = 0; self.nounset_error = None; + if let Ok(mut e) = self.arith_error.lock() { + *e = ArithError::default(); + } // Push call frame: $0 = script name, $1..N = args self.call_stack = vec![CallFrame { @@ -9295,6 +9358,10 @@ impl Interpreter { let mut last_val = 0i64; for arg in args { last_val = self.evaluate_arithmetic_with_assign(arg); + if let Some(msg) = self.take_arith_error_for("let: ", Some(arg.trim())) { + let result = ExecResult::err(msg, 1); + return self.apply_redirections(result, redirects).await; + } } let exit_code = if last_val == 0 { 1 } else { 0 }; let result = ExecResult { diff --git a/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh b/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh new file mode 100644 index 000000000..62fa1dbb4 --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh @@ -0,0 +1,39 @@ +### arith_division_by_zero_aborts_and_fails +( echo before; echo $((1/0)); echo never ) 2>/dev/null +echo "rc=$?" +### expect +before +rc=1 +### end + +### arith_error_messages +x=$( { echo $((1 % 0)); } 2>&1 ); echo "${x#bash: line *: }" +y=$( { echo $((08)); } 2>&1 ); echo "${y#bash: line *: }" +z=$( { echo $((2#12)); } 2>&1 ); echo "${z#bash: line *: }" +### expect +1 % 0: division by 0 (error token is "0") +08: value too great for base (error token is "08") +2#12: value too great for base (error token is "2#12") +### end + +### arith_command_and_let_report_and_continue +m=$( { ((1/0)); } 2>&1 ); echo "${m#bash: line *: }" +((1/0)); echo "cmd rc=$?" +n=$( { let "y=2#12"; } 2>&1 ); echo "${n#bash: line *: }" +let "y=1/0"; echo "let rc=$?" +echo done +### expect +((: 1/0: division by 0 (error token is "0") +cmd rc=1 +let: y=2#12: value too great for base (error token is "2#12") +let rc=1 +done +### end + +### arith_valid_bases_still_work +echo $((0x1f)) $((010)) $((16#ff)) $((10#08)) $((2#101)) $((64#@)) $(( -1 / 1 )) +a=5; ((a+=2)); let b=a*2; echo $a $b +### expect +31 8 255 8 5 62 -1 +7 14 +### end diff --git a/knowledge/log.md b/knowledge/log.md index f945965e6..02a934c37 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -19,6 +19,7 @@ * **Fix**: `eval` with a syntax error exits 2 like bash and reports `bash: eval: line N: syntax error: ...` plus the offending line, each newline-terminated, with the eval's own redirects applied (`$(eval bad 2>&1)` captures it). `$LINENO` inside `$( )` now counts from the outer line instead of restarting at 1. The parser's message text still differs from bash's `near unexpected token` wording. * **Fix**: parameter expansion and `mapfile` gaps from the Debian oracle. `${v~}`/`${v~~}` toggle case (also per array element). `\/` in a `${v/pat/rep}` replacement is a plain `/`. Unquoted `${!prefix@}`, `${!ref}`, `${v:off:len}`, `${a[@]:o:n}` and `${v@op}` now undergo field splitting like other expansions, so `for v in ${!cfg_@}` loops per name. `mapfile` gained `-d DELIM` (`''` is NUL), `-n`, `-s`, `-O` (keeps other elements) and grouped options; `-u` other than 0 and `-C`/`-c` exit 2 (L-MAPFILE-001). * **Fix**: `[[ a < b ]]` and `[[ a > b ]]` compare strings instead of being swallowed as redirections (the conditional grammar has no redirects), so `[[ a < B ]]` is false in byte order like bash. `shopt` listings pad the name to 15 columns and separate the state with a tab, matching bash. +* **Fix**: arithmetic evaluation reports errors instead of yielding 0. `$((1/0))` and `$((08))` now print bash's `line N: EXPR: division by 0 (error token is "0")` / `value too great for base` and abort the command with status 1; `(( ))` and `let` print the same with their `((: ` / `let: ` prefix and fail without aborting the script. A new `arith_error` channel on the interpreter carries the failure out of the `&self` evaluator, reported where `set -u` errors are. Error lines say `bash: line N:` because the interpreter does not track the running script's path (see architecture). * **Decision**: CPython's workload priority is agents running `python3` for scripted logic (processing a file, transforming data). Low-use stdlib modules are no longer shipped: `unittest`, `doctest`, profilers, `compileall`/`zipapp`, `dbm`/`shelve`, `plistlib`, `wave`, `netrc`, `cmd`, `tty`/`pty`, and `bz2`/`lzma`/`compression.zstd` (no C codec in the guest anyway). Stdlib zip 3.58 to 3.16 MB. L-CPY-009 updated. See [CPython WebAssembly Runtime](runtimes/cpython-wasm.md#workload-priority). * **Feature**: `yes` and `seq` stream into pipelines. As a producer stage they write chunks into the pipe as they go (`Context::stdout_stream`), so `yes | head -1` and `seq 1000000 | head -1` stop after the first chunk with `PIPESTATUS` `141 0` instead of running to the output cap and printing a truncation notice. Step 2 of the roadmap (streaming builtin I/O), starting with the generators. * **Feature**: `sudo` and `busybox`, the top missing commands in the gap replay (`sudo tee /etc/x < Date: Wed, 7 Oct 2026 22:45:05 +0000 Subject: [PATCH 2/2] test(bindings): division by zero reports and fails The Python and JS security suites asserted bashkit printed 0 for division and modulo by zero. It now reports like bash and fails, so the expectations move with it. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit-js/__test__/security.spec.ts | 13 ++++++++----- crates/bashkit-python/tests/_security_core.py | 10 +++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/bashkit-js/__test__/security.spec.ts b/crates/bashkit-js/__test__/security.spec.ts index b12f8a940..afe0e53b7 100644 --- a/crates/bashkit-js/__test__/security.spec.ts +++ b/crates/bashkit-js/__test__/security.spec.ts @@ -959,18 +959,21 @@ test("BB: arithmetic overflow (TM-DOS-029)", (t) => { test("BB: division by zero does not crash (TM-DOS-029)", (t) => { const bash = new Bash(); const r = bash.executeSync("echo $((1 / 0)) 2>&1"); - // Bashkit returns 0 for div-by-zero (differs from bash which errors). - // Key security property: interpreter must not crash or panic. + // Reports like bash. Key security property: no crash or panic. t.is(typeof r.exitCode, "number", "division by zero must not crash"); - t.is(r.stdout.trim(), "0", "bashkit returns 0 for div-by-zero"); + t.not(r.exitCode, 0, "division by zero fails"); + t.is(r.stdout, "", "no value is printed"); + t.true(r.stderr.includes("division by 0"), "the error is reported"); }); test("BB: modulo by zero does not crash (TM-DOS-029)", (t) => { const bash = new Bash(); const r = bash.executeSync("echo $((1 % 0)) 2>&1"); - // Same as div-by-zero: bashkit returns 0 instead of erroring. + // Same as div-by-zero: reports and fails. t.is(typeof r.exitCode, "number", "modulo by zero must not crash"); - t.is(r.stdout.trim(), "0", "bashkit returns 0 for mod-by-zero"); + t.not(r.exitCode, 0, "modulo by zero fails"); + t.is(r.stdout, "", "no value is printed"); + t.true(r.stderr.includes("division by 0"), "the error is reported"); }); test("BB: negative exponent", (t) => { diff --git a/crates/bashkit-python/tests/_security_core.py b/crates/bashkit-python/tests/_security_core.py index 8ffc9b549..9a7fc0a58 100644 --- a/crates/bashkit-python/tests/_security_core.py +++ b/crates/bashkit-python/tests/_security_core.py @@ -253,10 +253,12 @@ def test_tm_dos_029_arithmetic_overflow_does_not_crash(): def test_tm_dos_029_division_by_zero_does_not_crash(): - """TM-DOS-029: division by zero must return an error value, not panic.""" + """TM-DOS-029: division by zero must report like bash, not panic.""" r = Bash().execute_sync("echo $((1 / 0)) 2>&1") assert isinstance(r.exit_code, int) - assert r.stdout.strip() == "0" + assert r.exit_code != 0 + assert r.stdout == "" + assert "division by 0" in r.stderr _assert_sanitized_error(r.stderr) @@ -264,7 +266,9 @@ def test_tm_dos_029_modulo_by_zero_does_not_crash(): """TM-DOS-029: modulo by zero must not unwind through the binding.""" r = Bash().execute_sync("echo $((1 % 0)) 2>&1") assert isinstance(r.exit_code, int) - assert r.stdout.strip() == "0" + assert r.exit_code != 0 + assert r.stdout == "" + assert "division by 0" in r.stderr _assert_sanitized_error(r.stderr)