From 8639b9f41c16a0aac52ff484870aefb149232280 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:43:59 +0000 Subject: [PATCH 01/13] fix(interpreter): bound comma-list brace expansion (TM-DOS-042) The combinatorial count cap was only charged after each recursive call returned, so the first DFS path descended one frame per brace group with the count still zero. An input like {a,b}{a,b}... repeated tens of thousands of times (far under max_input_bytes) stack-overflowed the worker thread, and could accumulate O(depth*suffix) memory, before any cap or the execution timeout could fire. Add a recursion-depth cap and a cumulative output-byte budget (MAX_EXPANSION_RESULT_BYTES) so a runaway expansion degrades to a bounded literal result instead of crashing the host. --- crates/bashkit/src/interpreter/mod.rs | 60 ++++++++++++++++--- .../tests/integration/threat_model_tests.rs | 39 ++++++++++++ 2 files changed, 90 insertions(+), 9 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 13fcaa39b..3c9d2c876 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -11892,11 +11892,32 @@ impl Interpreter { fn expand_braces(&self, s: &str) -> Vec { const MAX_BRACE_EXPANSION_TOTAL: usize = 100_000; let mut count = 0; - self.expand_braces_capped(s, &mut count, MAX_BRACE_EXPANSION_TOTAL) - } - - fn expand_braces_capped(&self, s: &str, count: &mut usize, max: usize) -> Vec { - if *count >= max { + let mut bytes = 0; + self.expand_braces_capped(s, &mut count, &mut bytes, MAX_BRACE_EXPANSION_TOTAL, 0) + } + + /// THREAT[TM-DOS-042]: The combinatorial `count` cap alone is insufficient + /// because it is only incremented *after* each recursive call returns, so + /// the first DFS path descends to the full nesting/sequence depth (one + /// level per brace group) with `count` still zero. An input like + /// `{a,b}{a,b}...` repeated tens of thousands of times (well under + /// `max_input_bytes`) therefore stack-overflows the worker thread, and + /// allocates O(depth * suffix) memory, before the count cap or execution + /// timeout can fire. The `depth` cap bounds the descent up front; + /// legitimate scripts never approach this many nested/sequential groups. + fn expand_braces_capped( + &self, + s: &str, + count: &mut usize, + bytes: &mut usize, + max: usize, + recursion_depth: usize, + ) -> Vec { + const MAX_BRACE_EXPANSION_DEPTH: usize = 100; + if *count >= max + || *bytes >= Self::MAX_EXPANSION_RESULT_BYTES + || recursion_depth >= MAX_BRACE_EXPANSION_DEPTH + { return vec![s.to_string()]; } @@ -11955,12 +11976,14 @@ impl Interpreter { if let Some(range_result) = self.try_expand_range(&brace_content) { let mut results = Vec::new(); for item in range_result { - if *count >= max { + if *count >= max || *bytes >= Self::MAX_EXPANSION_RESULT_BYTES { break; } let expanded = format!("{}{}{}", prefix, item, suffix); - let sub = self.expand_braces_capped(&expanded, count, max); + let sub = + self.expand_braces_capped(&expanded, count, bytes, max, recursion_depth + 1); *count += sub.len(); + *bytes += sub.iter().map(String::len).sum::(); results.extend(sub); } return results; @@ -11976,12 +11999,13 @@ impl Interpreter { let mut results = Vec::new(); for item in items { - if *count >= max { + if *count >= max || *bytes >= Self::MAX_EXPANSION_RESULT_BYTES { break; } let expanded = format!("{}{}{}", prefix, item, suffix); - let sub = self.expand_braces_capped(&expanded, count, max); + let sub = self.expand_braces_capped(&expanded, count, bytes, max, recursion_depth + 1); *count += sub.len(); + *bytes += sub.iter().map(String::len).sum::(); results.extend(sub); } @@ -12136,6 +12160,24 @@ mod tests { use crate::fs::InMemoryFs; use crate::parser::Parser; + /// TM-DOS-042: comma-list brace expansion must not recurse one frame per + /// brace group (stack overflow) nor accumulate unbounded memory. A long + /// `{a,b}{a,b}...` sequence — far under the input cap — used to descend to + /// full depth before any cap engaged. + #[test] + fn brace_expansion_comma_sequence_is_bounded() { + let fs: Arc = Arc::new(InMemoryFs::new()); + let interp = Interpreter::new(Arc::clone(&fs)); + let s = "{a,b}".repeat(50_000); + let out = interp.expand_braces(&s); + // Must terminate without panic/overflow and stay bounded. + let total: usize = out.iter().map(String::len).sum(); + assert!( + total <= Interpreter::MAX_EXPANSION_RESULT_BYTES + 1024, + "brace expansion produced {total} bytes — should be byte-capped" + ); + } + #[test] fn test_empty_anchored_replacement_respects_expansion_limit() { let fs: Arc = Arc::new(InMemoryFs::new()); diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 6ebfca24a..3490fedc9 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -4650,6 +4650,45 @@ echo ${#arr[@]} } } + /// TM-DOS-042: Comma-list brace expansion recursion bomb. + /// `{a,b}{a,b}...` repeated tens of thousands of times is far under + /// `max_input_bytes`, but the combinatorial count cap is only charged on + /// recursion *return*, so the first DFS path used to descend one frame per + /// group and stack-overflow the worker thread before any cap fired. The + /// depth cap must bound the descent: this completes without panic/OOM and + /// with bounded output. + #[tokio::test] + async fn brace_expansion_comma_recursion_bomb() { + let limits = ExecutionLimits::new() + .max_commands(1_000) + .max_stdout_bytes(1_000_000); + let mut bash = Bash::builder().limits(limits).build(); + + // 50k repetitions of `{a,b}` (~250 KB source, well under the input cap). + let script = format!("echo {}", "{a,b}".repeat(50_000)); + let result = bash.exec(&script).await; + match result { + Ok(r) => { + assert!( + r.stdout.len() <= 1_000_000, + "comma-list brace bomb produced {} bytes — should be capped", + r.stdout.len() + ); + } + Err(e) => { + let msg = e.to_string(); + assert!( + msg.contains("brace") + || msg.contains("exceeded") + || msg.contains("budget") + || msg.contains("too large"), + "Expected a limit error, got: {}", + msg + ); + } + } + } + /// TM-DOS-059: Parameter expansion replacement bomb. /// `${x//a/$(echo bbbbbbbb)}` replaces each 'a' with 'bbbbbbbb'. /// At scale (10K 'a's × 1K 'b's = 10MB), this must be caught by From 7d48fbd24418aa3fac13d6bbc1432cbf802a8729 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:44:05 +0000 Subject: [PATCH 02/13] fix(jq): cap output bytes and honor execution deadline (TM-DOS-095) jaq evaluation is a synchronous iterator that the async execution timeout cannot preempt, and the result loop appended every produced value to an in-memory String with no byte cap, value-count cap, or deadline check. 'jq -n repeat(1)' (or range(0;1e18)) therefore grew output without limit (OOM) or spun forever producing no output (hang), reachable by default since jq is a core builtin. Cap accumulated output against the caller's max_stdout_bytes and poll a new ExecutionDeadline::is_expired() periodically so a runaway filter aborts with a clean error instead of wedging the host. --- crates/bashkit/src/builtins/jq/mod.rs | 29 ++++++++++++++++++++++++- crates/bashkit/src/builtins/jq/tests.rs | 23 ++++++++++++++++++++ crates/bashkit/src/builtins/mod.rs | 7 ++++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/bashkit/src/builtins/jq/mod.rs b/crates/bashkit/src/builtins/jq/mod.rs index 92da84ee2..800a48f9a 100644 --- a/crates/bashkit/src/builtins/jq/mod.rs +++ b/crates/bashkit/src/builtins/jq/mod.rs @@ -24,9 +24,10 @@ use jaq_core::{Compiler, Ctx, Vars, data}; use jaq_json::Val; use jaq_std::input::{HasInputs, Inputs, RcIter}; -use super::{Builtin, Context, read_text_file, resolve_path}; +use super::{Builtin, Context, ExecutionDeadline, read_text_file, resolve_path}; use crate::error::Result; use crate::interpreter::ExecResult; +use crate::limits::ExecutionLimits; mod args; mod compat; @@ -338,6 +339,19 @@ async fn run_jq(ctx: Context<'_>, parsed: JqArgs<'_>) -> Result { let mut has_output = false; let mut all_null_or_false = true; + // THREAT[TM-DOS-095]: jaq evaluation is a synchronous iterator that the + // async execution timeout cannot preempt. Unbounded generators + // (`jq -n 'repeat(1)'`, `range(0;1e18)`) would grow `output` without limit + // (OOM) or spin forever producing no output (hang). Cap accumulated output + // bytes against the caller's stdout limit and check the wall-clock deadline + // periodically so a runaway filter aborts instead of wedging the host. + let max_output_bytes = ctx + .execution_extension::() + .map(|l| l.max_stdout_bytes) + .unwrap_or_else(|| ExecutionLimits::default().max_stdout_bytes); + let deadline = ctx.execution_extension::(); + let mut values_emitted: usize = 0; + // Drive the outer loop from the shared input iterator so `input`/`inputs` // inside the filter consume from the same source (matching real jq: // `[inputs]` on a 3-value stream returns the *remaining* values, not all). @@ -405,6 +419,19 @@ async fn run_jq(ctx: Context<'_>, parsed: JqArgs<'_>) -> Result { if !parsed.join_output { output.push('\n'); } + + if output.len() > max_output_bytes { + return Ok(ExecResult::err( + format!("jq: output limit exceeded ({max_output_bytes} bytes)\n"), + 5, + )); + } + values_emitted += 1; + if values_emitted.is_multiple_of(4096) + && deadline.is_some_and(ExecutionDeadline::is_expired) + { + return Ok(ExecResult::err("jq: execution timed out\n".to_string(), 5)); + } } Err(e) => { return Ok(ExecResult::err(format_runtime_error(&e), 5)); diff --git a/crates/bashkit/src/builtins/jq/tests.rs b/crates/bashkit/src/builtins/jq/tests.rs index 2ea89c6aa..2d280c312 100644 --- a/crates/bashkit/src/builtins/jq/tests.rs +++ b/crates/bashkit/src/builtins/jq/tests.rs @@ -126,6 +126,29 @@ async fn keys_pretty_prints_array() { assert_eq!(result.trim(), "[\n \"a\",\n \"b\"\n]"); } +/// TM-DOS-095: an unbounded generator must not grow output without limit or +/// hang the host. jaq's iterator is synchronous so the execution timeout +/// cannot preempt it; the output-byte cap (here the 1 MB default, since the +/// test harness supplies no shell/limits) must abort with a clean error. +#[tokio::test] +async fn unbounded_generator_is_capped() { + let result = run_jq_result_with_args(&["-n", "repeat(1)"], "") + .await + .unwrap(); + assert_ne!(result.exit_code, 0, "runaway generator should fail"); + assert!( + result.stderr.contains("output limit exceeded"), + "expected output-limit error, got stderr={:?} stdout_len={}", // debug-ok: test assertion + result.stderr, + result.stdout.len() + ); + assert!( + result.stdout.len() <= 1_048_576 + 64, + "output must be bounded by the cap, got {} bytes", + result.stdout.len() + ); +} + #[tokio::test] async fn length_returns_number() { let result = run_jq("length", r#"[1,2,3,4,5]"#).await.unwrap(); diff --git a/crates/bashkit/src/builtins/mod.rs b/crates/bashkit/src/builtins/mod.rs index 7069a5626..18f7b64a6 100644 --- a/crates/bashkit/src/builtins/mod.rs +++ b/crates/bashkit/src/builtins/mod.rs @@ -417,6 +417,13 @@ impl ExecutionDeadline { remaining } } + + /// Whether the wall-clock budget is exhausted. Builtins with synchronous + /// loops the async timeout cannot preempt poll this to abort. + #[allow(dead_code)] + pub(crate) fn is_expired(&self) -> bool { + self.started_at.elapsed() >= self.timeout + } } impl ExecutionExtensions { From 884a74acd5ca30b79dd76dfb2743fd8a4008b97e Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:48:00 +0000 Subject: [PATCH 03/13] fix(expr): guard i64::MIN / -1 division overflow expr's / and % used raw a / b while + - * already used checked_*, so 'expr -9223372036854775808 / -1' (and % -1) overflowed: a debug-build panic, a wrong wrapping result in release. Use checked_div/checked_rem and return the existing 'integer overflow' error. --- crates/bashkit/src/builtins/expr.rs | 30 +++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/bashkit/src/builtins/expr.rs b/crates/bashkit/src/builtins/expr.rs index 86d81f196..852a3718b 100644 --- a/crates/bashkit/src/builtins/expr.rs +++ b/crates/bashkit/src/builtins/expr.rs @@ -118,13 +118,14 @@ fn evaluate(args: &[&str]) -> std::result::Result { if b == 0 { return Err("division by zero".to_string()); } - a / b + // checked_div guards i64::MIN / -1 (overflow panic). + a.checked_div(b).ok_or("integer overflow")? } "%" => { if b == 0 { return Err("division by zero".to_string()); } - a % b + a.checked_rem(b).ok_or("integer overflow")? } _ => unreachable!(), }; @@ -545,6 +546,31 @@ mod tests { assert_eq!(result.stdout.trim(), "0"); } + // ==================== arithmetic overflow ==================== + + /// i64::MIN / -1 (and % -1) overflow; must return a clean error, not panic. + #[tokio::test] + async fn expr_div_overflow_no_panic() { + let (fs, mut cwd, mut variables) = setup().await; + let env = HashMap::new(); + for op in ["/", "%"] { + let args = vec![ + "-9223372036854775808".to_string(), + op.to_string(), + "-1".to_string(), + ]; + let ctx = + Context::new_for_test(&args, &env, &mut variables, &mut cwd, fs.clone(), None); + let result = Expr.execute(ctx).await.unwrap(); + assert_eq!(result.exit_code, 2, "op {op} should error"); + assert!( + result.stderr.contains("integer overflow"), + "op {op}: expected overflow error, got {:?}", // debug-ok: test assertion + result.stderr + ); + } + } + // ==================== pattern matching ==================== #[tokio::test] From 4f46d1d8bdebf96d2e9bd03f8e172063bdb9e228 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:48:00 +0000 Subject: [PATCH 04/13] fix(date): guard relative-offset arithmetic overflow unit_duration multiplied n * 30 / n * 365 and built chrono Durations unchecked, then the caller added the offset to a base datetime unchecked. Inputs like 'date -d "30000000000000000 years"' overflowed the multiply or the chrono range and panicked. Use checked_mul + Duration::try_* and checked_add_signed, returning a 'date out of range' error instead. --- crates/bashkit/src/builtins/date.rs | 58 +++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/crates/bashkit/src/builtins/date.rs b/crates/bashkit/src/builtins/date.rs index c9c348516..448b28c51 100644 --- a/crates/bashkit/src/builtins/date.rs +++ b/crates/bashkit/src/builtins/date.rs @@ -151,7 +151,9 @@ fn parse_base_date(s: &str, now: DateTime) -> std::result::Result) -> std::result::Result Option { regex::Regex::new(r"^(\d+)\s+(second|minute|hour|day|week|month|year)s?\s+ago$").ok()?; if let Some(caps) = re_ago.captures(s) { let n: i64 = caps[1].parse().ok()?; - return Some(unit_duration(&caps[2], -n)); + return unit_duration(&caps[2], n.checked_neg()?); } // "+N unit(s)" or "-N unit(s)" or "N unit(s)" @@ -267,32 +276,35 @@ fn parse_relative_date(s: &str) -> Option { if let Some(caps) = re_rel.captures(s) { let sign = if &caps[1] == "-" { -1i64 } else { 1i64 }; let n: i64 = caps[2].parse().ok()?; - return Some(unit_duration(&caps[3], sign * n)); + return unit_duration(&caps[3], sign.checked_mul(n)?); } // "next unit" / "last unit" if let Some(unit) = s.strip_prefix("next ") { let unit = unit.trim().trim_end_matches('s'); - return Some(unit_duration(unit, 1)); + return unit_duration(unit, 1); } if let Some(unit) = s.strip_prefix("last ") { let unit = unit.trim().trim_end_matches('s'); - return Some(unit_duration(unit, -1)); + return unit_duration(unit, -1); } None } -fn unit_duration(unit: &str, n: i64) -> Duration { +// Returns None when the requested offset overflows i64 (the multiply) or the +// chrono Duration range, so callers can report "date out of range" instead of +// panicking on inputs like `date -d "30000000000000000 years"`. +fn unit_duration(unit: &str, n: i64) -> Option { match unit { - "second" => Duration::seconds(n), - "minute" => Duration::minutes(n), - "hour" => Duration::hours(n), - "day" => Duration::days(n), - "week" => Duration::weeks(n), - "month" => Duration::days(n * 30), // Approximate - "year" => Duration::days(n * 365), // Approximate - _ => Duration::zero(), + "second" => Duration::try_seconds(n), + "minute" => Duration::try_minutes(n), + "hour" => Duration::try_hours(n), + "day" => Duration::try_days(n), + "week" => Duration::try_weeks(n), + "month" => n.checked_mul(30).and_then(Duration::try_days), // Approximate + "year" => n.checked_mul(365).and_then(Duration::try_days), // Approximate + _ => Some(Duration::zero()), } } @@ -777,6 +789,20 @@ mod tests { assert_eq!(date.len(), 10); } + /// Relative offsets that overflow i64 or the chrono range must report an + /// error, not panic (e.g. `n * 365` in unit_duration, then base + offset). + #[tokio::test] + async fn test_date_d_relative_overflow_no_panic() { + for spec in [ + "30000000000000000 years", + "+9000000000000000000 days", + "300000000000000000 months ago", + ] { + let result = run_date(&["-d", spec, "+%Y"]).await; + assert_ne!(result.exit_code, 0, "spec {spec:?} should fail cleanly"); + } + } + #[tokio::test] async fn test_date_d_tomorrow() { let result = run_date(&["-d", "tomorrow", "+%Y-%m-%d"]).await; From 8d33952d7c15647322dc319f82346fe3fedb64c6 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:51:13 +0000 Subject: [PATCH 05/13] fix(interpreter): run eval and alias bodies in the current shell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit eval and alias expansion called Interpreter::execute(), which runs the EXIT trap, instead of execute_script_body() like source does. This is wrong bash semantics — the EXIT trap fired at the end of every eval'd or aliased command rather than at shell exit — and because the top-level EXIT-trap block has no re-entrancy guard, 'trap "eval :" EXIT' recursed one command per level until the budget aborted, risking a stack overflow first. Route both through execute_script_body(.., run_exit_trap=false). --- crates/bashkit/src/interpreter/mod.rs | 12 +++++-- .../integration/blackbox_security_tests.rs | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 3c9d2c876..456cf74a4 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -4926,7 +4926,10 @@ impl Interpreter { 1, )) } else { - self.execute(&s).await + // Alias expansion runs in the current shell: use + // execute_script_body (like source/eval), NOT execute(), + // so an aliased command does not fire the EXIT trap. + self.execute_script_body(&s, false, true).await } } Err(e) => Ok(ExecResult::err( @@ -6292,7 +6295,12 @@ impl Interpreter { self.pipeline_stdin = stdin; } - let mut result = self.execute(&script).await?; + // eval runs in the current shell: use execute_script_body (like source), + // NOT execute(), so it does not fire the EXIT trap. execute() runs the + // EXIT trap, which is wrong for eval and — because the top-level EXIT + // trap has no re-entrancy guard — lets `trap 'eval :' EXIT` recurse one + // command per level until the budget aborts, risking stack overflow. + let mut result = self.execute_script_body(&script, false, true).await?; self.pipeline_stdin = prev_pipeline_stdin; diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index a000ea95d..595e8085d 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -1224,6 +1224,37 @@ mod command_injection_passing { assert!(result.stdout.contains("normal")); } + /// eval runs in the current shell and must NOT fire the EXIT trap when the + /// eval'd code finishes — the trap fires once, at actual shell exit. + #[tokio::test] + async fn eval_does_not_fire_exit_trap_early() { + let mut bash = tight_bash(); + let result = bash + .exec("trap 'echo BYE' EXIT\neval 'echo hi'\necho done") + .await + .unwrap(); + assert_eq!( + result.stdout.matches("BYE").count(), + 1, + "EXIT trap should fire exactly once, got: {:?}", + result.stdout + ); + // BYE must come after `done` (end of script), not after `hi`. + let bye = result.stdout.find("BYE").unwrap(); + let done = result.stdout.find("done").unwrap(); + assert!(done < bye, "EXIT trap fired early: {:?}", result.stdout); + } + + /// An EXIT trap that runs `eval` must not recurse unboundedly. Before the + /// fix, eval re-ran the (unguarded) EXIT trap, recursing one command per + /// level until the budget aborted — risking a stack overflow first. + #[tokio::test] + async fn exit_trap_eval_recursion_terminates() { + let mut bash = tight_bash(); + // Must return (Ok or budget Err), never hang or stack-overflow. + let _ = bash.exec("trap 'eval :' EXIT\necho go").await; + } + /// Array subscript command substitution stays sandboxed #[tokio::test] async fn array_subscript_cmd_subst_sandboxed() { From 3ec3297bf39bff6dda05feb94086944eeb49a973 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:55:24 +0000 Subject: [PATCH 06/13] fix(limits): saturating subtraction in check_function_insert The byte-accounting check used self.function_body_bytes + body_bytes - old_body_bytes while the sibling record_function_insert already used saturating_sub. If accounting drifts (e.g. after a snapshot restore recomputes function sizes differently) so old_body_bytes exceeds the running total, the raw subtraction underflow-panics. Mirror the saturating arithmetic. --- crates/bashkit/src/limits.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bashkit/src/limits.rs b/crates/bashkit/src/limits.rs index aa7b82044..ae0609478 100644 --- a/crates/bashkit/src/limits.rs +++ b/crates/bashkit/src/limits.rs @@ -810,7 +810,11 @@ impl MemoryBudget { limits.max_function_count ))); } - let new_bytes = self.function_body_bytes + body_bytes - old_body_bytes; + // saturating_sub mirrors record_function_insert: if accounting ever + // drifts so old_body_bytes exceeds the running total (e.g. after a + // snapshot restore recomputes sizes differently), the check must not + // underflow-panic. + let new_bytes = (self.function_body_bytes + body_bytes).saturating_sub(old_body_bytes); if new_bytes > limits.max_function_body_bytes { return Err(LimitExceeded::Memory(format!( "function body byte limit ({}) exceeded", From e385f32da36e7b02b5e5e0026d5e677966508641 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:55:24 +0000 Subject: [PATCH 07/13] fix(credential): refuse placeholder substitution when repeated Placeholder mode used substring replace(), so a script could repeat the placeholder in a credential-owned header to segment/fingerprint the real secret across a header-reflecting endpoint. Substitute only when the placeholder occurs exactly once; otherwise fail closed, sending placeholders rather than the secret. (The single-echo risk on a fully reflecting approved host remains a documented v1 limitation.) --- crates/bashkit/src/credential.rs | 47 +++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/bashkit/src/credential.rs b/crates/bashkit/src/credential.rs index c3382bc58..acc0df026 100644 --- a/crates/bashkit/src/credential.rs +++ b/crates/bashkit/src/credential.rs @@ -264,7 +264,21 @@ impl CredentialPolicy { .iter() .find(|(name, _)| name.eq_ignore_ascii_case(header_name)) { - *header_value = header_value.replace(placeholder_str, secret_value); + // Substitute only when the placeholder appears + // exactly once. A legitimate value carries one + // placeholder; multiple occurrences are a + // segmentation/fingerprinting attempt (a script + // splitting the value to read the secret back in + // chunks from a header-reflecting endpoint), so + // we refuse and fail closed — the request goes + // out with placeholders, never the real secret. + // (The single-echo risk where an approved host + // reflects the whole header remains a documented + // v1 limitation; see credential-injection.md.) + if header_value.matches(placeholder_str).count() == 1 { + *header_value = + header_value.replacen(placeholder_str, secret_value, 1); + } } } } @@ -458,6 +472,37 @@ mod tests { } } + #[test] + fn test_placeholder_multiple_occurrences_fail_closed() { + // A script repeating the placeholder is attempting to segment the + // secret across a header-reflecting endpoint. We must not substitute + // anything: the request goes out with placeholders, never the secret. + let mut policy = CredentialPolicy::new(); + let placeholder = + policy.add_placeholder("https://api.openai.com", Credential::bearer("sk-real-key")); + + let hook = policy.into_hook(); + let event = HttpRequestEvent { + method: "POST".into(), + url: "https://api.openai.com/v1/chat/completions".into(), + headers: vec![( + "Authorization".into(), + format!("Bearer {placeholder}.{placeholder}"), + )], + }; + + match hook(event) { + HookAction::Continue(e) => { + assert!( + !e.headers[0].1.contains("sk-real-key"), + "secret must not be injected when placeholder is repeated" + ); + assert!(e.headers[0].1.contains("bk_placeholder_")); + } + HookAction::Cancel(_) => panic!("should not cancel"), + } + } + #[test] fn test_placeholder_not_replaced_for_wrong_host() { let mut policy = CredentialPolicy::new(); From 543a5c4f42d17462cb4812b904e8e6b7e2d1c605 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 21:59:18 +0000 Subject: [PATCH 08/13] fix(python): clamp Monty duration to the execution deadline The Python builtin passed only its own PythonLimits.max_duration to Monty, ignoring the caller's ExecutionDeadline. Because the async execution timeout cannot preempt Monty's synchronous start/resume sections, a CPU-bound script could overrun a tighter bash timeout up to Monty's own (default 30s) limit. Clamp the VM duration to the remaining deadline before running, mirroring the TypeScript builtin. --- crates/bashkit/src/builtins/python.rs | 16 ++++++++++-- .../integration/python_security_tests.rs | 25 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/bashkit/src/builtins/python.rs b/crates/bashkit/src/builtins/python.rs index 90d561159..653f37370 100644 --- a/crates/bashkit/src/builtins/python.rs +++ b/crates/bashkit/src/builtins/python.rs @@ -32,7 +32,7 @@ use std::pin::Pin; use std::sync::Arc; use std::time::Duration; -use super::{Builtin, Context, resolve_path}; +use super::{Builtin, Context, ExecutionDeadline, resolve_path}; use crate::error::Result; use crate::fs::{FileSystem, FileType}; use crate::interpreter::ExecResult; @@ -445,13 +445,25 @@ impl Builtin for Python { .map(|(k, v)| (k.clone(), v.clone())), ); + // Clamp Monty's wall-clock budget to the caller's remaining execution + // deadline (like the TypeScript builtin) so a CPU-bound script cannot + // overrun a tighter bash timeout — the async timeout cannot preempt + // Monty's synchronous start/resume sections. + let mut limits = self.limits.clone(); + if let Some(remaining) = ctx + .execution_extension::() + .map(ExecutionDeadline::remaining) + { + limits.max_duration = limits.max_duration.min(remaining); + } + run_python( &code, &filename, ctx.fs.clone(), ctx.cwd, &merged_env, - &self.limits, + &limits, self.external_fns.as_ref(), ) .await diff --git a/crates/bashkit/tests/integration/python_security_tests.rs b/crates/bashkit/tests/integration/python_security_tests.rs index af9991297..42f225879 100644 --- a/crates/bashkit/tests/integration/python_security_tests.rs +++ b/crates/bashkit/tests/integration/python_security_tests.rs @@ -321,6 +321,31 @@ mod whitebox_resource_limits { assert_ne!(r.exit_code, 0, "100ms limit should block long loop"); } + /// The bash-level execution timeout must bound a CPU-bound Python loop even + /// when PythonLimits.max_duration is much larger: the async timeout cannot + /// preempt Monty's synchronous run, so the builtin clamps Monty's duration + /// to the remaining deadline. Without the clamp this would run for the full + /// 60s Python limit. + #[tokio::test] + async fn bash_timeout_clamps_python_duration() { + let bash = Bash::builder() + .python_with_limits(PythonLimits::default().max_duration(Duration::from_secs(60))) + .env("BASHKIT_ALLOW_INPROCESS_PYTHON", "1") + .limits(ExecutionLimits::new().timeout(Duration::from_millis(300))); + let mut bash = bash.build(); + let start = std::time::Instant::now(); + let r = bash + .exec("python3 -c \"while True:\n pass\"") + .await + .unwrap(); + let elapsed = start.elapsed(); + assert_ne!(r.exit_code, 0, "infinite loop should be stopped"); + assert!( + elapsed < Duration::from_secs(15), + "bash timeout did not clamp Python duration: ran {elapsed:?}" + ); + } + #[tokio::test] async fn tight_recursion_blocks_deep_call() { let limits = PythonLimits::default().max_recursion(10); From 451c4a7c53794a5e50f0363c3f27553f9f04037d Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 22:04:31 +0000 Subject: [PATCH 09/13] perf(lexer): bound two lookahead scans to avoid O(n^2) lexing read_word_or_fd_redirect collected all remaining input to inspect a 2-char redirect operator, and looks_like_assoc_assign scanned unbounded leading whitespace. Both made a crafted input (millions of digit-led words, or '$x=(' + megabytes of spaces) O(n) per call and the whole lex O(n^2), bounded only by the wall-clock parser timeout (TM-DOS-024). Cap the digit lookahead to 4 chars and reuse MAX_LOOKAHEAD for the whitespace scan, matching looks_like_brace_expansion. --- crates/bashkit/src/parser/lexer.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bashkit/src/parser/lexer.rs b/crates/bashkit/src/parser/lexer.rs index fd21dcc58..1680251b8 100644 --- a/crates/bashkit/src/parser/lexer.rs +++ b/crates/bashkit/src/parser/lexer.rs @@ -322,8 +322,11 @@ impl<'a> Lexer<'a> { } // Check if it's a single digit followed by > or < - // We need to peek further without consuming - let input_remaining: String = self.chars.clone().collect(); + // We need to peek further without consuming. Only the digit plus a + // 2-char redirect operator (e.g. ">>", "<&", "<<") matter, so bound the + // lookahead — collecting all remaining input here made every + // digit-initial word O(n) and the whole lex O(n^2) (TM-DOS-024). + let input_remaining: String = self.chars.clone().take(4).collect(); // Check patterns: "N>" "N>>" "N>&" "N<" "N<&" if fd_str.len() == 1 @@ -1833,13 +1836,17 @@ impl<'a> Lexer<'a> { /// compound assignment like `([key]=val ...)`. Returns true when the /// first non-whitespace char after `(` is `[`. fn looks_like_assoc_assign(&self) -> bool { + // Cap the lookahead like looks_like_brace_expansion: an uncapped scan + // over leading whitespace made `x=(` followed by megabytes of spaces + // O(n) per call (TM-DOS-024). + const MAX_LOOKAHEAD: usize = 10_000; let mut chars = self.chars.clone(); // Skip the `(` we haven't consumed yet if chars.next() != Some('(') { return false; } // Skip optional whitespace - for ch in chars { + for ch in chars.take(MAX_LOOKAHEAD) { match ch { ' ' | '\t' => continue, '[' => return true, From 8be734aef91604016dddd750e0dee2e1ec1858cb Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 22:07:58 +0000 Subject: [PATCH 10/13] fix(sqlite): sanitize engine open and dot-command error paths sanitize() was applied only to top-level statement errors. The open_pure_memory / open_file_engine failures and dot-command errors (which carry raw turso messages that can include crate-relative paths) were interpolated directly into stderr. Route all four emission points through sanitize() (TM-SQL-011, idempotent on already-clean messages). --- crates/bashkit/src/builtins/sqlite/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/bashkit/src/builtins/sqlite/mod.rs b/crates/bashkit/src/builtins/sqlite/mod.rs index 0bcba350a..32491737e 100644 --- a/crates/bashkit/src/builtins/sqlite/mod.rs +++ b/crates/bashkit/src/builtins/sqlite/mod.rs @@ -427,7 +427,7 @@ impl Builtin for Sqlite { let engine = match SqliteEngine::open_pure_memory() { Ok(e) => e, Err(msg) => { - return Ok(ExecResult::err(format!("sqlite: {msg}\n"), 1)); + return Ok(ExecResult::err(format!("sqlite: {}\n", sanitize(&msg)), 1)); } }; let outcome = run_statements( @@ -443,7 +443,7 @@ impl Builtin for Sqlite { ) .await; if let Err(e) = outcome { - stderr.push_str(&format!("sqlite: {e}\n")); + stderr.push_str(&format!("sqlite: {}\n", sanitize(&e))); exit_code = 1; } } @@ -473,7 +473,9 @@ impl Builtin for Sqlite { if reopen { match open_file_engine(backend, path, &ctx.fs, &self.limits).await { Ok(e) => *guard = Some(e), - Err(msg) => return Ok(ExecResult::err(format!("sqlite: {msg}\n"), 1)), + Err(msg) => { + return Ok(ExecResult::err(format!("sqlite: {}\n", sanitize(&msg)), 1)); + } } } let engine = guard.as_ref().expect("engine populated above"); @@ -491,7 +493,7 @@ impl Builtin for Sqlite { ) .await; if let Err(e) = outcome { - stderr.push_str(&format!("sqlite: {e}\n")); + stderr.push_str(&format!("sqlite: {}\n", sanitize(&e))); exit_code = 1; } From 38d36346fb2139fe077ef20e8b09f49a66898584 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 22:10:19 +0000 Subject: [PATCH 11/13] chore(threat-model): correct DoS spec drift and document new findings - TM-DOS-029/030/031: narrative + 'NEEDED' status said open; they are mitigated/fixed. Update the prose and the verification matrix to match the status table. - TM-DOS-042: the old 'bails out of recursion when budget is hit' claim was false for comma-lists (count charged on return); document the recursion-depth + output-byte caps that actually bound it. - Add TM-DOS-093 (jq unbounded generator) and reconcile the in-code threat ID to the next sequential number. - Document that the FS byte/count quotas do not apply to RealFs ReadWrite mounts (by design; --mount-rw is sandbox-breaking). --- crates/bashkit/src/builtins/jq/mod.rs | 2 +- crates/bashkit/src/builtins/jq/tests.rs | 2 +- specs/threat-model.md | 42 ++++++++++++++++--------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/crates/bashkit/src/builtins/jq/mod.rs b/crates/bashkit/src/builtins/jq/mod.rs index 800a48f9a..eb958cc24 100644 --- a/crates/bashkit/src/builtins/jq/mod.rs +++ b/crates/bashkit/src/builtins/jq/mod.rs @@ -339,7 +339,7 @@ async fn run_jq(ctx: Context<'_>, parsed: JqArgs<'_>) -> Result { let mut has_output = false; let mut all_null_or_false = true; - // THREAT[TM-DOS-095]: jaq evaluation is a synchronous iterator that the + // THREAT[TM-DOS-093]: jaq evaluation is a synchronous iterator that the // async execution timeout cannot preempt. Unbounded generators // (`jq -n 'repeat(1)'`, `range(0;1e18)`) would grow `output` without limit // (OOM) or spin forever producing no output (hang). Cap accumulated output diff --git a/crates/bashkit/src/builtins/jq/tests.rs b/crates/bashkit/src/builtins/jq/tests.rs index 2d280c312..f96839e58 100644 --- a/crates/bashkit/src/builtins/jq/tests.rs +++ b/crates/bashkit/src/builtins/jq/tests.rs @@ -126,7 +126,7 @@ async fn keys_pretty_prints_array() { assert_eq!(result.trim(), "[\n \"a\",\n \"b\"\n]"); } -/// TM-DOS-095: an unbounded generator must not grow output without limit or +/// TM-DOS-093: an unbounded generator must not grow output without limit or /// hang the host. jaq's iterator is synchronous so the execution timeout /// cannot preempt it; the output-byte cap (here the 1 MB default, since the /// test harness supplies no shell/limits) must abort with a clean error. diff --git a/specs/threat-model.md b/specs/threat-model.md index c10c7fcd6..35474cfe4 100644 --- a/specs/threat-model.md +++ b/specs/threat-model.md @@ -136,6 +136,15 @@ the session-level backstop. | TM-DOS-039 | Missing validate_path in VFS methods | `remove`, `stat`, `read_dir`, `copy`, `rename`, `symlink`, `chmod` skip `validate_path()` | All `FileSystem` methods on `InMemoryFs` and `OverlayFs` call `validate_path()`; `MountableFs` validates via the root limits before delegating (TM-DOS-046) | **MITIGATED** | | TM-DOS-040 | Integer truncation on 32-bit | `u64 as usize` casts in network/Python extension silently truncate on 32-bit, bypassing size checks | All `u64`-to-`usize` size-check casts in `network/client.rs` (Content-Length checks) and `bashkit-python/src/lib.rs` (limits propagation) use `usize::try_from(...).unwrap_or(usize::MAX)` so over-`usize` values fail safe instead of wrapping | **MITIGATED** | +> **Scope of the FS quotas above (TM-DOS-005/006/010, `max_file_size` / +> `max_file_count` / `max_total_bytes`)**: these are enforced by the +> in-memory and overlay backends. They do **not** apply to a `RealFs` mount +> in `RealFsMode::ReadWrite` — `RealFs::limits()` returns `unlimited()` and +> writes go straight to the host with no byte/count quota. This is by design +> (`--mount-rw` is sandbox-breaking, see TM-ESC-030), but it means a script +> with a writable real-FS mount can exhaust host disk/inodes. Use +> `--mount-ro` for untrusted scripts. + **TM-DOS-034**: Fixed. `InMemoryFs::append_file()` now uses a single write lock for the entire read-check-write operation, preventing TOCTOU races. See `fs/memory.rs:940-942`. @@ -308,21 +317,25 @@ longer the primary defence. Regression tests: depth exceeded` error. Regression test: `tests/threat_model_tests.rs::yaml_template_depth::tm_dos_052_template_if_depth_bomb`. -**Current Risk**: MEDIUM - Three open DoS vectors (TM-DOS-029, TM-DOS-030, TM-DOS-031) need remediation +**Current Risk**: LOW — TM-DOS-029, TM-DOS-030, and TM-DOS-031 are all mitigated +(see the status table above). Their original write-ups are kept below for history. -**TM-DOS-029**: Arithmetic exponentiation casts `i64` to `u32` (`right as u32`), wrapping negatives. -`i64::pow()` with large exponent panics or hangs. Shift operators panic if `right >= 64` or `right < 0`. -`i64::MIN / -1` panics. All arithmetic panics in debug on overflow. -Fix: use `wrapping_*` operations and clamp shift/exponent ranges. +**TM-DOS-029** (mitigated): Arithmetic exponentiation cast `i64` to `u32` (`right as u32`), +wrapping negatives; `i64::pow()` with a large exponent panicked or hung; shift operators panicked +if `right >= 64` or `right < 0`; `i64::MIN / -1` panicked. Resolved: arithmetic ops use `wrapping_*`, +shift amounts are masked, and the exponent is clamped (`interpreter/mod.rs` arithmetic evaluator). +The same `i64::MIN / -1` guard is now applied in the `expr` builtin (`checked_div`/`checked_rem`). -**TM-DOS-030**: `eval` (line 4613), `source` (line 4548), trap handlers (lines 697, 7795), and alias -expansion (line 3627) all use `Parser::new()` which ignores configured `max_ast_depth` and -`max_parser_operations`. Previously fixed for command substitution (TM-DOS-021) but not these paths. -Fix: use `Parser::with_limits()` everywhere. +**TM-DOS-030** (fixed): `eval`, `source`, trap handlers, and alias expansion previously used +`Parser::new()`, ignoring configured `max_ast_depth` / `max_parser_operations`. Resolved: all of +these paths use `Parser::with_limits()`. Separately, `eval` and alias expansion now run their bodies +via `execute_script_body(.., run_exit_trap=false)` (like `source`) so they do not fire the EXIT trap +— previously they ran the unguarded EXIT trap, letting `trap 'eval :' EXIT` recurse one command per +level until the budget aborted. -**TM-DOS-031**: ExtGlob `+(...)` and `*(...)` handlers recurse without depth limit. Pattern -`+(a|aa)` against `"aaaa..."` creates exponential backtracking via nested `glob_match_impl` calls. -Fix: add depth parameter to `glob_match_impl`, bail when exceeded. +**TM-DOS-031** (mitigated): ExtGlob `+(...)` / `*(...)` handlers recursed without a depth limit. +Resolved: `glob_match_impl` carries a recursion-depth cap and bails when exceeded +(`interpreter/glob.rs`). **Implementation**: `limits.rs`, `builtins/awk.rs`, `builtins/jq/`, `builtins/diff.rs` ```rust @@ -1354,7 +1367,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track | TM-INF-017 | `set` and `declare -p` leak internal markers | Internal state disclosure (_NAMEREF_, _READONLY_, _UPPER_, _LOWER_) | Filter `is_internal_variable()` names from output | | TM-INF-018 | `date` builtin returns real host time | Timezone fingerprinting, timing correlation | `Bash::builder().fixed_epoch(N)` freezes the clock; `Bash::builder().epoch_offset(N)` shifts real-clock by N seconds (mutually exclusive, last call wins). Both implemented via `Date::with_fixed_epoch` / `Date::with_offset_seconds` in `builtins/date.rs`. Default behavior is real clock — embed callers opt in for sandboxing. Regression tests: `tm_inf_018_date::*` in `tests/threat_model_tests.rs`. | **MITIGATED** (opt-in) | | ~~TM-DOS-041~~ | ~~Brace expansion `{N..M}` unbounded range~~ | ~~OOM via `{1..999999999}` allocating billions of strings~~ | Static parser-time check (`MAX_STATIC_BRACE_RANGE = 100_000` in `parser/budget.rs`) rejects oversized literal ranges with `BraceRangeTooLarge`; runtime fallback in `try_expand_range` (`MAX_BRACE_RANGE = 10_000`) treats remaining oversized ranges as literals (**FIXED**) | -| ~~TM-DOS-042~~ | ~~Brace expansion combinatorial explosion~~ | ~~OOM via `{1..100}{1..100}{1..100}` = 1M strings~~ | `expand_braces` caps total emitted strings at `MAX_BRACE_EXPANSION_TOTAL = 100_000` and bails out of the recursion when the budget is hit (**FIXED**) | +| ~~TM-DOS-042~~ | ~~Brace expansion combinatorial explosion~~ | ~~OOM/stack overflow via `{1..100}{1..100}{1..100}` (1M strings) or `{a,b}{a,b}...` (deep recursion)~~ | `expand_braces` caps total emitted strings (`MAX_BRACE_EXPANSION_TOTAL = 100_000`). The count cap alone was insufficient for comma-lists: it is only charged on recursion *return*, so the first DFS path descended one frame per group with the count still zero and stack-overflowed. Now also bounds recursion depth and cumulative output bytes (`MAX_EXPANSION_RESULT_BYTES`) so it degrades to a bounded literal result (**FIXED**) | | ~~TM-DOS-043~~ | ~~Arithmetic overflow in `execute_arithmetic_with_side_effects`~~ | ~~Panic (DoS) in debug mode via `((x+=1))` with x=i64::MAX~~ | ~~Use `wrapping_add/sub/mul`~~ (**FIXED**) | | ~~TM-DOS-044~~ | ~~Lexer `read_command_subst_into` stack overflow~~ | ~~Process crash (SIGABRT) via ~50 nested `$()` in double-quotes~~ | ~~Add depth parameter to `read_command_subst_into()`~~ (**FIXED**) | | ~~TM-DOS-045~~ | ~~OverlayFs `symlink()` bypasses all limits~~ | ~~Unlimited symlink creation despite `max_file_count`~~ | ~~Add `check_write_limits()` + `validate_path()` to symlink~~ — overlay symlink path enforces limits (**FIXED**) | @@ -1387,6 +1400,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track | ~~TM-DOS-090~~ | ~~`shuf` unbounded range/repeat materialization~~ | ~~OOM/CPU exhaustion via huge `--input-range` or `--head-count` before stdout truncation~~ | ~~Sample numeric ranges without full collection and reject output that exceeds `ExecutionLimits` before allocation~~ — `shuf_resource_tests` cover huge range `-n 1` and repeat output caps (**FIXED**) | | TM-DOS-091 | SQLite `.dump` cumulative output bypass | `.dump` built the full schema+rows string before checking `max_output_bytes`; an attacker controlling many tables/rows could cause memory to grow far beyond the configured output cap | `bounded_append()` helper enforces the cap after each schema line and each INSERT row; `dispatch()` receives the *remaining* budget (`max_output_bytes - stdout.len()`) from `run_statements` — **FIXED** via #1869 | | TM-DOS-092 | Subshell snapshot amplification | Deeply nested `( ... )` keeps CoW state snapshots and call-stack clones alive | `max_subshell_depth` counter (default 32) bounds live explicit subshell snapshots | **MITIGATED** | +| TM-DOS-093 | jq unbounded generator OOM/hang | `jq -n 'repeat(1)'` / `range(0;1e18)` — the jaq result loop appended every value to an in-memory string with no byte/value/deadline cap; jaq's iterator is synchronous so the async execution timeout cannot preempt it. jq is a core builtin (no opt-in gate) | Cap accumulated output at the caller's `max_stdout_bytes` and poll `ExecutionDeadline::is_expired()` every 4096 values, aborting with a clean error (`builtins/jq/mod.rs`) — **FIXED** | ### Accepted (Low Priority) @@ -1457,7 +1471,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track | VFS limit enforcement on public API | TM-ESC-012 | `validate_path()` + `check_write_limits()` in `add_file()` / `restore()` | **MITIGATED** | | Custom jaq env function | TM-INF-013, TM-ISO-004 | Read from `ctx.env`/`ctx.variables`, not `std::env` | **DONE** | | Internal variable namespace isolation | TM-INJ-009 | `is_internal_variable()` rejection in every write path; `set` / `declare -p` filter | **MITIGATED** | -| Parser limit propagation | TM-DOS-030 | `Parser::with_limits()` in eval/source/trap/alias | **NEEDED** | +| Parser limit propagation | TM-DOS-030 | `Parser::with_limits()` in eval/source/trap/alias | **MITIGATED** | | ExtGlob depth limit | TM-DOS-031 | Depth parameter in `glob_match_impl` (`interpreter/glob.rs:162,324`) | **MITIGATED** | | Python wrapper input sanitization | TM-PY-023, TM-PY-024 | `shlex.quote()` on every interpolated path; per-call random heredoc delimiter via `secrets.token_hex(8)` | **MITIGATED** | | Tar path validation | TM-INJ-010 | Resolved-path check + `starts_with(&extract_base)` in `archive.rs` | **MITIGATED** | From f85621d31e3fb61f5e3b8a39235f3020f2ab8183 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 22:11:43 +0000 Subject: [PATCH 12/13] test(date): use Display in overflow-assert message The new overflow test used {spec:?}, which the no_debug_fmt_in_builtin_source static guard (TM-INF-022) rejects in builtin sources. spec is a &str, so use Display. --- crates/bashkit/src/builtins/date.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bashkit/src/builtins/date.rs b/crates/bashkit/src/builtins/date.rs index 448b28c51..8a9b07516 100644 --- a/crates/bashkit/src/builtins/date.rs +++ b/crates/bashkit/src/builtins/date.rs @@ -799,7 +799,7 @@ mod tests { "300000000000000000 months ago", ] { let result = run_date(&["-d", spec, "+%Y"]).await; - assert_ne!(result.exit_code, 0, "spec {spec:?} should fail cleanly"); + assert_ne!(result.exit_code, 0, "spec '{spec}' should fail cleanly"); } } From e47cd39eda740ec3b1174d89c13082406f827d5a Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 9 Jun 2026 23:30:52 +0000 Subject: [PATCH 13/13] test(python): make timeout-clamp test robust to outer timeout The clamp test set a 300ms bash timeout and unwrapped the exec result, which could panic (and flake on loaded CI) if the outer execution timeout fired and returned Err(Timeout). Use a 2s timeout and accept either a non-zero Ok exit or a Timeout Err; the elapsed-time bound (well under the 60s Python limit) is the real regression guard. --- .../integration/python_security_tests.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/crates/bashkit/tests/integration/python_security_tests.rs b/crates/bashkit/tests/integration/python_security_tests.rs index 42f225879..4627568f6 100644 --- a/crates/bashkit/tests/integration/python_security_tests.rs +++ b/crates/bashkit/tests/integration/python_security_tests.rs @@ -325,21 +325,26 @@ mod whitebox_resource_limits { /// when PythonLimits.max_duration is much larger: the async timeout cannot /// preempt Monty's synchronous run, so the builtin clamps Monty's duration /// to the remaining deadline. Without the clamp this would run for the full - /// 60s Python limit. + /// 60s Python limit. Use a generous (2s) bash timeout so the clamp normally + /// stops Monty cleanly first; either a non-zero `Ok` exit or a `Timeout` + /// `Err` is acceptable — both prove the loop was bounded, and the + /// elapsed-time assertion (well under the 60s Python limit) is the real + /// regression guard. Accepting both avoids flakiness on loaded runners. #[tokio::test] async fn bash_timeout_clamps_python_duration() { let bash = Bash::builder() .python_with_limits(PythonLimits::default().max_duration(Duration::from_secs(60))) .env("BASHKIT_ALLOW_INPROCESS_PYTHON", "1") - .limits(ExecutionLimits::new().timeout(Duration::from_millis(300))); + .limits(ExecutionLimits::new().timeout(Duration::from_secs(2))); let mut bash = bash.build(); let start = std::time::Instant::now(); - let r = bash - .exec("python3 -c \"while True:\n pass\"") - .await - .unwrap(); + let result = bash.exec("python3 -c \"while True:\n pass\"").await; let elapsed = start.elapsed(); - assert_ne!(r.exit_code, 0, "infinite loop should be stopped"); + // An Err here is the outer execution timeout firing, which is also an + // acceptable bound; the elapsed-time assertion below is the real guard. + if let Ok(r) = result { + assert_ne!(r.exit_code, 0, "infinite loop should be stopped"); + } assert!( elapsed < Duration::from_secs(15), "bash timeout did not clamp Python duration: ran {elapsed:?}"