From b58c901294c1d5c796e04e41d6bf855a1b593e58 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 19:40:12 -0500 Subject: [PATCH 1/6] fix(indirect): truncate array target at first ] to stop operand recursion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An indirect array target such as r='a[0]:-${!r}]' passed validation whole, so desugar re-parsed ${a[0]:-${!r}]} and the unset-element default re-entered indirect expansion without bound (stack overflow, SIGABRT). Truncate the resolved bracket target at the first ] after [ before validation. Real bash resolves a[0] and ignores the rest (verified oracles), so trailing operands — including nested ${!...} — never reach the re-parse. Plain names, a[$i], a[@], assoc subscripts pass through unchanged; $(/backquote/process-substitution guards unchanged. Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 1 + crates/bashkit/src/interpreter/expansion.rs | 15 ++++ .../tests/integration/threat_model_tests.rs | 74 +++++++++++++++++++ knowledge/security/threat-model.md | 1 + 4 files changed, 91 insertions(+) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 1884be6e6..fe85da51b 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,6 +82,7 @@ through configurable limits. | History files (TM-DOS-131) | `history -a` loops, huge `$HISTSIZE`, giant `$HISTFILE` | History list bounded by `max_history_entries`/`max_history_bytes`; `$HISTFILE` reads refused past `max_input_bytes`; VFS only | MITIGATED | | Completion specs and bindings (TM-DOS-132) | `complete`/`bind` in a loop | 1024 specs (64 KiB each) and 1024 binding changes (4 KiB each) per interpreter; never shared between instances | MITIGATED | | Prompt recursion (TM-DOS-133) | `x='${x@P}'; echo "${x@P}"`, mutual cycles or substitution-mediated cycles | Cumulative prompt depth capped at `min(max_ast_depth, 8)`; shared work, aggregate input, cancellation and deadline checks on each prompt; descendant shells inherit depth | MITIGATED | +| Indirect operand recursion (TM-DOS-134) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets truncate at the first `]` (bash parity: `a[0]` resolves, rest ignored), so trailing operands never reach the indirect re-parse | MITIGATED | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index e95595047..df4c69185 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2624,6 +2624,21 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; + // TM-RES-INDIRECT: an indirect array target ends at the first `]` + // after `[` (real bash resolves `a[0]` from `a[0]:-x]` and ignores + // the rest). Truncating before validation keeps trailing + // operator/nesting text — including a nested `${!...}` — from ever + // reaching the desugar re-parse below, which re-entered indirect + // expansion without bound (`r='a[0]:-${!r}]'` overflowed the stack). + // Targets without a sub-`]` (plain names, `a[$i]`, `a[@]`) pass + // through unchanged. + let value = match value.find('[') { + Some(open) => match value[open..].find(']') { + Some(rel) => value[..open + rel + 1].to_string(), + None => value, + }, + None => value, + }; let valid = ident(&value) || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 6ecbcbc02..35c9a2245 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -20,6 +20,80 @@ use std::time::Duration; mod resource_exhaustion { use super::*; + /// TM-RES-INDIRECT: an indirect array target ends at the first `]`. + /// Trailing operator/nesting text is never re-expanded as code, so a + /// default operand cannot re-enter indirect expansion and exhaust the + /// stack (`r='a[0]:-${!r}]'`). Matches real bash: element-or-empty. + #[tokio::test] + async fn indirect_array_target_ends_at_first_close_bracket() { + // Trailing operator/nesting text is ignored, like real bash + // (verified: bash prints `out=[]`, `st=0` for every target below). + for target in [ + "a[0]:-injected]", + "a[0]-injected]", + "a[0]:=injected]", + "a[0]+injected]", + "a[0]/x/injected]", + "a[0][1]", + "a[0]}${x:-injected}[0]", + ] { + let mut bash = Bash::new(); + let result = bash + .exec(&format!( + "r='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" + )) + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n", "target: {target}"); + assert!( + result.stderr.is_empty(), + "target: {target}: {}", + result.stderr + ); + } + // Set element resolves through the truncated target (bash: `1`). + let mut bash = Bash::new(); + let result = bash + .exec("a=(1 2)\nr='a[0]:-injected]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + + /// TM-RES-INDIRECT: self-referential default operand terminates. + /// Pre-fix this exact reproducer overflowed the stack (SIGABRT). + #[tokio::test] + async fn indirect_self_cycle_default_operand_terminates() { + // Exact finding reproducer: unset element + self-cycle in default. + // Real bash prints `out=[]` with status 0 and never crashes. + let mut bash = Bash::new(); + let result = bash + .exec("unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + // Set element: the cycle operand is discarded, element wins. + let mut bash = Bash::new(); + let result = bash + .exec("a=(1 2)\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .await + .unwrap(); + assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + // Mutual cycle through two names terminates as well. + let mut bash = Bash::new(); + let result = bash + .exec( + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + ) + .await + .unwrap(); + assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + /// V1: Test that command limit prevents infinite execution #[tokio::test] async fn threat_infinite_commands_blocked() { diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index bb7631f85..856e8ccc9 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,6 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | +| TM-DOS-134 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow) | The resolved bracket target is truncated at the first `]` after `[` before validation (real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_target_ends_at_first_close_bracket`, `indirect_self_cycle_default_operand_terminates` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101. From 89e45cb22b6d03e97522bde9fa4171956fb70178 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 20:55:20 -0500 Subject: [PATCH 2/6] fix(indirect): preserve nested subscripts with a complete-reference boundary PR 2673 truncated indirect array targets at the first ']', which broke valid nested subscripts (a[b[0]] errored; both real bash versions print two). The boundary now ends at the close of the first complete [...] reference, tracking nested brackets and skipping quoted/escaped text, so trailing operators still never reach the desugar re-parse (self/mutual ${!...} cycles terminate with bash-parity output) while nested and quoted-key references resolve whole. Tests: hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash; assoc direct/indirect agreement. Renumbers the prior TM-DOS-134 rows to TM-DOS-135 (tilde budget work owns 134). Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/expansion.rs | 55 +++++- .../tests/integration/threat_model_tests.rs | 178 ++++++++++++++---- knowledge/security/threat-model.md | 2 +- 4 files changed, 189 insertions(+), 48 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index fe85da51b..b2582a6dd 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,7 +82,7 @@ through configurable limits. | History files (TM-DOS-131) | `history -a` loops, huge `$HISTSIZE`, giant `$HISTFILE` | History list bounded by `max_history_entries`/`max_history_bytes`; `$HISTFILE` reads refused past `max_input_bytes`; VFS only | MITIGATED | | Completion specs and bindings (TM-DOS-132) | `complete`/`bind` in a loop | 1024 specs (64 KiB each) and 1024 binding changes (4 KiB each) per interpreter; never shared between instances | MITIGATED | | Prompt recursion (TM-DOS-133) | `x='${x@P}'; echo "${x@P}"`, mutual cycles or substitution-mediated cycles | Cumulative prompt depth capped at `min(max_ast_depth, 8)`; shared work, aggregate input, cancellation and deadline checks on each prompt; descendant shells inherit depth | MITIGATED | -| Indirect operand recursion (TM-DOS-134) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets truncate at the first `]` (bash parity: `a[0]` resolves, rest ignored), so trailing operands never reach the indirect re-parse | MITIGATED | +| Indirect operand recursion (TM-DOS-135) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets end at the close of the first complete `[...]` reference (bash parity; nested subscripts resolve whole), so trailing operands never reach the indirect re-parse | MITIGATED | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index 42e683879..a91757dba 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2637,6 +2637,40 @@ impl Interpreter { }) } + // End index (exclusive) of the first complete subscript reference that + // opens at `open`: nested brackets nest, quoted or escaped text is + // skipped. `None` when the brackets never balance. All boundary bytes + // are ASCII, so the index is always a char boundary. + fn indirect_array_end(value: &str, open: usize) -> Option { + let bytes = value.as_bytes(); + let mut depth = 0usize; + let mut quote: Option = None; + let mut escaped = false; + let mut i = open; + while i < bytes.len() { + let b = bytes[i]; + // 0x27 is a single quote, 0x22 a double quote, 0x5c a backslash. + if escaped { + escaped = false; + } else if quote.is_some_and(|q| b == q) { + quote = None; + } else if quote.is_none() && (b == 0x27 || b == 0x22) { + quote = Some(b); + } else if quote.is_none() && b == 0x5c { + escaped = true; + } else if quote.is_none() && b == b'[' { + depth += 1; + } else if quote.is_none() && b == b']' && depth > 0 { + depth -= 1; + if depth == 0 { + return Some(i + 1); + } + } + i += 1; + } + None + } + /// The parameter `${!name...}` refers to: the value of `name` (an /// element when it carries a subscript). bash rejects an unset `name` /// ("invalid indirect expansion") and a value that is not a parameter @@ -2673,17 +2707,18 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; - // TM-RES-INDIRECT: an indirect array target ends at the first `]` - // after `[` (real bash resolves `a[0]` from `a[0]:-x]` and ignores - // the rest). Truncating before validation keeps trailing - // operator/nesting text — including a nested `${!...}` — from ever - // reaching the desugar re-parse below, which re-entered indirect - // expansion without bound (`r='a[0]:-${!r}]'` overflowed the stack). - // Targets without a sub-`]` (plain names, `a[$i]`, `a[@]`) pass - // through unchanged. + // TM-RES-INDIRECT: an indirect array target ends at the close of + // the first complete `[...]` reference (real bash resolves `a[0]` + // from `a[0]:-x]` and ignores the rest). The boundary tracks nested + // brackets and quoted text, so valid nested subscripts (`a[b[0]]`, + // `h['a]b']`) survive whole while trailing operator/nesting text — + // including a nested `${!...}` — never reaches the desugar re-parse + // below, which re-entered indirect expansion without bound + // (`r='a[0]:-${!r}]'` overflowed the stack). Targets without a + // balanced `]` (plain names, `a[$i]`, `a[@]`) pass through unchanged. let value = match value.find('[') { - Some(open) => match value[open..].find(']') { - Some(rel) => value[..open + rel + 1].to_string(), + Some(open) => match Self::indirect_array_end(&value, open) { + Some(end) => value[..end].to_string(), None => value, }, None => value, diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 35c9a2245..8b547d3a1 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -10,8 +10,9 @@ use bashkit::{ OverlayFs, SessionLimits, TraceEventDetails, TraceEventKind, TraceMode, }; use std::path::Path; +use std::process::{Command, Stdio}; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; // ============================================================================= // 1. RESOURCE EXHAUSTION TESTS @@ -20,14 +21,93 @@ use std::time::Duration; mod resource_exhaustion { use super::*; - /// TM-RES-INDIRECT: an indirect array target ends at the first `]`. - /// Trailing operator/nesting text is never re-expanded as code, so a - /// default operand cannot re-enter indirect expansion and exhaust the - /// stack (`r='a[0]:-${!r}]'`). Matches real bash: element-or-empty. + /// TM-DOS-135: child-process runner for hostile scripts. + /// Hostile reproducers never run in the test process: pre-fix they + /// overflowed the stack and aborted the whole runner. The CLI binary + /// sits next to the test binary's `deps` directory. + fn bashkit_cli_path() -> std::path::PathBuf { + let exe = std::env::current_exe().expect("current test executable"); + let deps = exe.parent().expect("deps dir"); + let profile = deps.parent().expect("profile dir"); + profile.join(format!("bashkit{}", std::env::consts::EXE_SUFFIX)) + } + + struct BoundedRun { + timed_out: bool, + status_ok: bool, + exit_code: Option, + stdout: String, + stderr: String, + } + + /// Runs a script in a child CLI process under a wall-clock watchdog. + /// Darwin note: Rust std exposes no portable per-child CPU, stack, or + /// memory caps, so the watchdog kill plus the 8 KiB output cap are the + /// enforced bounds here. Records the real producer exit status. + fn run_cli_bounded(script: &str, limit: Duration) -> BoundedRun { + let mut child = Command::new(bashkit_cli_path()) + .args(["-c", script]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn bashkit CLI"); + let start = Instant::now(); + let timed_out = loop { + match child.try_wait().expect("poll child") { + Some(_) => break false, + None => { + if start.elapsed() >= limit { + let _ = child.kill(); + break true; + } + std::thread::sleep(Duration::from_millis(10)); + } + } + }; + let output = child.wait_with_output().expect("collect child output"); + const CAP: usize = 8192; + let mut stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + let mut stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + stdout.truncate(CAP); + stderr.truncate(CAP); + BoundedRun { + timed_out, + status_ok: output.status.success(), + exit_code: output.status.code(), + stdout, + stderr, + } + } + + /// TM-DOS-135: hostile indirect cycles terminate in a bounded child. + /// Exact finding reproducer plus the mutual variant. Real bash prints + /// `out=[]` with status 0 for both and never crashes. + #[test] + fn indirect_hostile_cycles_terminate_in_bounded_child() { + for script in [ + "unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + ] { + let run = run_cli_bounded(script, Duration::from_secs(20)); + assert!(!run.timed_out, "hostile script hung: {script}"); + assert!( + run.status_ok, + "exit={:?} stderr={}", + run.exit_code, run.stderr + ); + assert_eq!(run.stdout, "out=[]\nst=0\n", "script: {script}"); + assert!(run.stderr.is_empty(), "stderr: {}", run.stderr); + } + } + + /// TM-DOS-135: an indirect array target ends at the close of the first + /// complete `[...]` reference; trailing operators are ignored like real + /// bash, while nested subscripts resolve whole. #[tokio::test] - async fn indirect_array_target_ends_at_first_close_bracket() { - // Trailing operator/nesting text is ignored, like real bash - // (verified: bash prints `out=[]`, `st=0` for every target below). + async fn indirect_array_boundary_ignores_trailing_operators() { + // Trailing operator/nesting text resolves the leading element + // (verified against real bash: `out=[1]`, `st=0` for each). for target in [ "a[0]:-injected]", "a[0]-injected]", @@ -40,60 +120,86 @@ mod resource_exhaustion { let mut bash = Bash::new(); let result = bash .exec(&format!( - "r='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" + "a=(1 2)\nr='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" )) .await .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n", "target: {target}"); + assert_eq!(result.stdout, "out=[1]\nst=0\n", "target: {target}"); assert!( result.stderr.is_empty(), "target: {target}: {}", result.stderr ); } - // Set element resolves through the truncated target (bash: `1`). + // Nested subscripts resolve whole (bash: `two`/`two`). let mut bash = Bash::new(); let result = bash - .exec("a=(1 2)\nr='a[0]:-injected]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]'\nprintf \"indirect=[%s]\\n\" \"${!r}\"\nprintf \"direct=[%s]\\n\" \"${a[b[0]]}\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n"); + assert_eq!(result.stdout, "indirect=[two]\ndirect=[two]\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - } - - /// TM-RES-INDIRECT: self-referential default operand terminates. - /// Pre-fix this exact reproducer overflowed the stack (SIGABRT). - #[tokio::test] - async fn indirect_self_cycle_default_operand_terminates() { - // Exact finding reproducer: unset element + self-cycle in default. - // Real bash prints `out=[]` with status 0 and never crashes. + // An operator after a nested reference is ignored (bash: `two`). let mut bash = Bash::new(); let result = bash - .exec("unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]:-D'\necho \"out=[${!r}]\"\necho \"st=$?\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert_eq!(result.stdout, "out=[two]\nst=0\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // Set element: the cycle operand is discarded, element wins. + // Associative references agree between direct and indirect use. + // Portable note: indexed cases above also run against real bash + // below; assoc arrays need bash 4+, so agreement is asserted here. let mut bash = Bash::new(); let result = bash - .exec("a=(1 2)\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"") + .exec("declare -A h=([k]=V [key]=W)\nkey=k\nr='h[$key]'\necho \"dollar=[${!r}] direct=[${h[$key]}]\"\nr='h[key]'\necho \"plain=[${!r}] direct=[${h[key]}]\"\nr='h[@]'\necho \"at=[${!r}] direct=[${h[@]}]\"") .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n"); - assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // Mutual cycle through two names terminates as well. - let mut bash = Bash::new(); - let result = bash - .exec( - "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", - ) - .await - .unwrap(); - assert_eq!(result.stdout, "out=[]\nst=0\n"); + assert_eq!( + result.stdout, + "dollar=[V] direct=[V]\nplain=[W] direct=[W]\nat=[V W] direct=[V W]\n" + ); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); } + // Out of scope: `]` inside an assoc key is unsupported by the + // direct subscript parser as well (`declare` accepts it, reads + // misparse), so no indirect agreement is asserted for that shape. + // The boundary scan still tracks quotes so a complete reference + // stays whole if the parser ever learns quoted brackets. + + /// TM-DOS-135: finite nested indexed indirects match real bash + /// byte-for-byte. Indexed arrays work on bash 3.2+, so this runs on + /// macOS and Linux CI alike. + #[tokio::test] + async fn indirect_nested_indexed_matches_real_bash() { + for script in [ + "a=(zero one two three); b=(2); r='a[b[0]]'; printf 'indirect=[%s]\n' \"${!r}\"; printf 'direct=[%s]\n' \"${a[b[0]]}\"", + "a=(zero one two three); b=(2); r='a[b[0]]:-D'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[0][1]'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[1]EXTRA'; echo \"out=[${!r}]\"", + "a=(x y); i=1; r='a[$i]'; echo \"out=[${!r}]\"", + ] { + let expected = Command::new("bash") + .args(["--noprofile", "--norc", "-c", script]) + .output() + .expect("run real bash"); + assert!( + expected.status.success(), + "real bash failed: {script}: {}", + String::from_utf8_lossy(&expected.stderr) + ); + let mut harness = Bash::new(); + let result = harness.exec(script).await.unwrap(); + assert_eq!( + result.stdout, + String::from_utf8_lossy(&expected.stdout).into_owned(), + "script: {script}" + ); + assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); + } + } + /// V1: Test that command limit prevents infinite execution #[tokio::test] async fn threat_infinite_commands_blocked() { diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 856e8ccc9..bf09fc77b 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,7 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | -| TM-DOS-134 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow) | The resolved bracket target is truncated at the first `]` after `[` before validation (real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_target_ends_at_first_close_bracket`, `indirect_self_cycle_default_operand_terminates` | **MITIGATED** | +| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | The resolved bracket target ends at the close of the first complete `[...]` reference (nesting- and quote-aware; real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_ignores_trailing_operators`, `indirect_hostile_cycles_terminate_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101. From 164aed26e0cb16fb089e70796b5fc7ab3b2e48c5 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 21:21:34 -0500 Subject: [PATCH 3/6] fix(indirect): refuse trailing text unless the array reference is complete Modern bash (5.x) rejects indirect array targets with trailing operator/nesting text (invalid variable name); older 3.2 ignored the tail. Refusal (instead of stripping) keeps nested ${!...} from the desugar re-parse while valid nested subscripts (a[b[0]]) resolve whole, matching bash 5.x in every probed shape. Hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash on both bash generations for complete references; assoc direct/indirect agreement. TM-DOS-135 (tilde owns 134). Co-Authored-By: yolop --- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/expansion.rs | 54 ++++++------- .../tests/integration/threat_model_tests.rs | 76 ++++++++++--------- knowledge/security/threat-model.md | 2 +- 4 files changed, 71 insertions(+), 63 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index b2582a6dd..c7a17f912 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -82,7 +82,7 @@ through configurable limits. | History files (TM-DOS-131) | `history -a` loops, huge `$HISTSIZE`, giant `$HISTFILE` | History list bounded by `max_history_entries`/`max_history_bytes`; `$HISTFILE` reads refused past `max_input_bytes`; VFS only | MITIGATED | | Completion specs and bindings (TM-DOS-132) | `complete`/`bind` in a loop | 1024 specs (64 KiB each) and 1024 binding changes (4 KiB each) per interpreter; never shared between instances | MITIGATED | | Prompt recursion (TM-DOS-133) | `x='${x@P}'; echo "${x@P}"`, mutual cycles or substitution-mediated cycles | Cumulative prompt depth capped at `min(max_ast_depth, 8)`; shared work, aggregate input, cancellation and deadline checks on each prompt; descendant shells inherit depth | MITIGATED | -| Indirect operand recursion (TM-DOS-135) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | Bracket targets end at the close of the first complete `[...]` reference (bash parity; nested subscripts resolve whole), so trailing operands never reach the indirect re-parse | MITIGATED | +| Indirect operand recursion (TM-DOS-135) | `r='a[0]:-${!r}]'` re-entering `${!...}` through an unset-element default | A bracket target is refused unless the first complete `[...]` reference consumes the whole value (bash 5.x parity; nested subscripts resolve whole) | MITIGATED | **Stack Overflow / Recursion:** diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index a91757dba..bb01309f4 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -2707,34 +2707,36 @@ impl Interpreter { .is_some_and(|f| f.is_ascii_alphabetic() || f == '_') && c.all(|ch| ch.is_ascii_alphanumeric() || ch == '_') }; - // TM-RES-INDIRECT: an indirect array target ends at the close of - // the first complete `[...]` reference (real bash resolves `a[0]` - // from `a[0]:-x]` and ignores the rest). The boundary tracks nested - // brackets and quoted text, so valid nested subscripts (`a[b[0]]`, - // `h['a]b']`) survive whole while trailing operator/nesting text — - // including a nested `${!...}` — never reaches the desugar re-parse + // TM-RES-INDIRECT: a bracket indirect target is only valid when + // the first complete `[...]` reference consumes the entire value. + // Modern bash (5.x) rejects trailing operator/nesting text with + // `invalid variable name` (older 3.2 ignored it); rejection also + // keeps a nested `${!...}` from ever reaching the desugar re-parse // below, which re-entered indirect expansion without bound - // (`r='a[0]:-${!r}]'` overflowed the stack). Targets without a - // balanced `]` (plain names, `a[$i]`, `a[@]`) pass through unchanged. - let value = match value.find('[') { - Some(open) => match Self::indirect_array_end(&value, open) { - Some(end) => value[..end].to_string(), - None => value, - }, - None => value, + // (`r='a[0]:-${!r}]'` overflowed the stack). The boundary tracks + // nested brackets and quoted text, so valid nested subscripts + // (`a[b[0]]`) resolve whole, while `a[0]:-x]`, `a[0][1]`, and + // unbalanced shapes are refused. Plain names, `a[$i]`, and `a[@]` + // have no trailing text and pass through unchanged. + let valid_boundary = match value.find('[') { + Some(open) => { + Self::indirect_array_end(&value, open).is_some_and(|end| end == value.len()) + } + None => true, }; - let valid = ident(&value) - || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) - || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") - || value.find('[').is_some_and(|open| { - ident(&value[..open]) - && value.ends_with(']') - && value.len() > open + 2 - && !value.contains("$(") - && !value.contains('`') - && !value.contains("<(") - && !value.contains(">(") - }); + let valid = valid_boundary + && (ident(&value) + || (!value.is_empty() && value.bytes().all(|b| b.is_ascii_digit())) + || matches!(value.as_str(), "@" | "*" | "#" | "?" | "$" | "!" | "-") + || value.find('[').is_some_and(|open| { + ident(&value[..open]) + && value.ends_with(']') + && value.len() > open + 2 + && !value.contains("$(") + && !value.contains('`') + && !value.contains("<(") + && !value.contains(">(") + })); if !valid { // THREAT[TM-DOS-130]: echo a bounded prefix of the bad target. let shown: String = value.chars().take(256).collect(); diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 8b547d3a1..9cf3d67f7 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -34,7 +34,6 @@ mod resource_exhaustion { struct BoundedRun { timed_out: bool, - status_ok: bool, exit_code: Option, stdout: String, stderr: String, @@ -73,41 +72,54 @@ mod resource_exhaustion { stderr.truncate(CAP); BoundedRun { timed_out, - status_ok: output.status.success(), exit_code: output.status.code(), stdout, stderr, } } - /// TM-DOS-135: hostile indirect cycles terminate in a bounded child. - /// Exact finding reproducer plus the mutual variant. Real bash prints - /// `out=[]` with status 0 for both and never crashes. + /// TM-DOS-135: hostile indirect cycles are refused in a bounded child. + /// Exact finding reproducer plus the mutual variant. Both run in a + /// child CLI under a wall-clock watchdog (never in the test runner: + /// pre-fix they overflowed the stack and aborted the process) and are + /// refused with `invalid variable name`, like modern bash (5.x); the + /// real producer exit status is asserted, never relabeled. #[test] - fn indirect_hostile_cycles_terminate_in_bounded_child() { + fn indirect_hostile_cycles_rejected_in_bounded_child() { for script in [ - "unset a\nr='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", - "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"out=[${!r}]\"\necho \"st=$?\"", + "unset a\nr='a[0]:-${!r}]'\necho \"${!r}\"", + "unset a\nr='a[0]:-${!s}]'\ns='a[0]:-${!r}]'\necho \"${!r}\"", ] { let run = run_cli_bounded(script, Duration::from_secs(20)); assert!(!run.timed_out, "hostile script hung: {script}"); + assert_eq!( + run.exit_code, + Some(1), + "script: {script}, stdout: {}, stderr: {}", + run.stdout, + run.stderr + ); + assert!(run.stdout.is_empty(), "stdout: {}", run.stdout); assert!( - run.status_ok, - "exit={:?} stderr={}", - run.exit_code, run.stderr + run.stderr.contains("invalid variable name"), + "stderr: {}", + run.stderr ); - assert_eq!(run.stdout, "out=[]\nst=0\n", "script: {script}"); - assert!(run.stderr.is_empty(), "stderr: {}", run.stderr); + assert!(run.stderr.len() < 1024, "stderr: {}", run.stderr); } } - /// TM-DOS-135: an indirect array target ends at the close of the first - /// complete `[...]` reference; trailing operators are ignored like real - /// bash, while nested subscripts resolve whole. + /// TM-DOS-135: a bracket indirect target is refused unless the first + /// complete `[...]` reference consumes the whole value. Modern bash + /// (5.x) rejects trailing operators with `invalid variable name`; + /// refusal also keeps a nested `${!...}` from ever reaching the desugar + /// re-parse, which re-entered indirect expansion without bound. + /// Valid nested subscripts resolve whole. #[tokio::test] - async fn indirect_array_boundary_ignores_trailing_operators() { - // Trailing operator/nesting text resolves the leading element - // (verified against real bash: `out=[1]`, `st=0` for each). + async fn indirect_array_boundary_rejects_trailing_operators() { + // Trailing operator/nesting text is refused, like bash 5.x. Older + // bash 3.2 ignored the tail, so no real-bash oracle is asserted + // for these shapes; the rejection itself is the contract. for target in [ "a[0]:-injected]", "a[0]-injected]", @@ -116,21 +128,24 @@ mod resource_exhaustion { "a[0]/x/injected]", "a[0][1]", "a[0]}${x:-injected}[0]", + "a[b[0]]:-D", ] { let mut bash = Bash::new(); let result = bash - .exec(&format!( - "a=(1 2)\nr='{target}'\necho \"out=[${{!r}}]\"\necho \"st=$?\"" - )) + .exec(&format!("a=(1 2)\nb=(0)\nr='{target}'\necho \"${{!r}}\"")) .await .unwrap(); - assert_eq!(result.stdout, "out=[1]\nst=0\n", "target: {target}"); + assert_ne!(result.exit_code, 0, "target accepted: {target}"); + assert_eq!(result.exit_code, 1, "target: {target}"); + assert!(result.stdout.is_empty(), "target: {target}"); assert!( - result.stderr.is_empty(), + result.stderr.contains("invalid variable name"), "target: {target}: {}", result.stderr ); + assert!(result.stderr.len() < 1024, "target: {target}"); } + // Nested subscripts resolve whole (bash: `two`/`two`). let mut bash = Bash::new(); let result = bash @@ -139,14 +154,6 @@ mod resource_exhaustion { .unwrap(); assert_eq!(result.stdout, "indirect=[two]\ndirect=[two]\n"); assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); - // An operator after a nested reference is ignored (bash: `two`). - let mut bash = Bash::new(); - let result = bash - .exec("a=(zero one two three)\nb=(2)\nr='a[b[0]]:-D'\necho \"out=[${!r}]\"\necho \"st=$?\"") - .await - .unwrap(); - assert_eq!(result.stdout, "out=[two]\nst=0\n"); - assert!(result.stderr.is_empty(), "stderr: {}", result.stderr); // Associative references agree between direct and indirect use. // Portable note: indexed cases above also run against real bash // below; assoc arrays need bash 4+, so agreement is asserted here. @@ -175,9 +182,8 @@ mod resource_exhaustion { async fn indirect_nested_indexed_matches_real_bash() { for script in [ "a=(zero one two three); b=(2); r='a[b[0]]'; printf 'indirect=[%s]\n' \"${!r}\"; printf 'direct=[%s]\n' \"${a[b[0]]}\"", - "a=(zero one two three); b=(2); r='a[b[0]]:-D'; echo \"out=[${!r}]\"", - "a=(1 2); r='a[0][1]'; echo \"out=[${!r}]\"", - "a=(1 2); r='a[1]EXTRA'; echo \"out=[${!r}]\"", + "a=(1 2); r='a[0]'; echo \"out=[${!r}]\"", + "a=(x y); r='a[@]'; echo \"out=[${!r}]\"", "a=(x y); i=1; r='a[$i]'; echo \"out=[${!r}]\"", ] { let expected = Command::new("bash") diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index bf09fc77b..035d9bcaf 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -302,7 +302,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-132 | Completion spec and key binding growth | `complete -W "$big" nameN` or `bind -x '"\C-xN": cmd'` in a loop keeps per-shell state growing | At most 1024 completion specs of at most 64 KiB option text each, and at most 1024 `bind` changes of at most 4 KiB each, per interpreter; past a cap the builtin fails with status 1. The state is built on first use, lives in the interpreter (never global, so tenants never share it) and is copied into subshells like other shell state. Regression: `completion_specs_are_bounded`, `binding_changes_are_capped` | **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML aliases can expand exponentially during deserialization; deep YAML/JSON, document floods, jaq generators, and YAML re-serialization can consume stack, CPU, or memory beyond the source size | Reject YAML alias tokens with a bounded lexical pass before deserialization; VFS/stdin and aggregate input budgets; serde_yaml_ng recursion cap 128 plus Bashkit depth 100; 4096-document cap; shared jaq work/deadline/output limits; post-serialization stdout cap; YAML+JSON depth/document/input/output regressions plus `yq_fuzz` and arbitrary-input proptest | **MITIGATED** | | TM-DOS-133 | Unbounded prompt expansion | `x='${x@P}'; echo "${x@P}"`, mutual variable cycles or cycles through command substitutions repeatedly create shallow parsers and recursively poll futures | Prompt reparses count cumulative runtime depth capped at `min(max_ast_depth, 8)` before recursive polling; descendants inherit depth. Each prompt charges the shared work and aggregate-input budget, checking cancellation and deadlines before decoding; child parsers share the request budget and fail closed on poison. Depth unwinds on returned errors and resets at host execution boundaries. Regressions: `prompt_resource_tests`, `prompt_checks_cancellation_before_decoding`, `prompt_checks_shared_deadline_before_decoding`, `prompt_error_unwinds_depth` | **MITIGATED** | -| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | The resolved bracket target ends at the close of the first complete `[...]` reference (nesting- and quote-aware; real bash resolves `a[0]` and ignores the rest), so trailing operands — including a nested `${!...}` — never reach the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_ignores_trailing_operators`, `indirect_hostile_cycles_terminate_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | +| TM-DOS-135 | Indirect array-target operand recursion | `r='a[0]:-${!r}]'; echo "${!r}"`: the bracket validator accepted trailing operator/nesting text, so desugar re-parsed `${a[0]:-${!r}]}` and the unset-element default re-entered indirect expansion without bound (stack overflow). Numbered to avoid collision with the adjacent tilde-budget entry | A bracket target is valid only when the first complete `[...]` reference (nesting- and quote-aware) consumes the whole value; any trailing text is refused with `invalid variable name`, like modern bash 5.x (older 3.2 ignored the tail). Refusal keeps a nested `${!...}` from ever reaching the desugar re-parse while valid nested subscripts (`a[b[0]]`) resolve whole; `$(`/backquote/process-substitution guards unchanged. Regressions: `indirect_array_boundary_rejects_trailing_operators`, `indirect_hostile_cycles_rejected_in_bounded_child`, `indirect_nested_indexed_matches_real_bash` | **MITIGATED** | | TM-DOS-107 | Static-analysis command validation amplification | A large comment before thousands of commands makes a whole-script ordered-subsequence scan per command | Build one source-character position index, then validate each analyzed command-name character with a logarithmic position lookup instead of rescanning source | **MITIGATED** | **TM-DOS-051** is historical. The custom parser was deleted with the `yaml` helper; the replacement `yq` parser/evaluator boundary is tracked by TM-DOS-101. From ceba71b0b7f1a932f8e66a914f9c8496f7782360 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 22:20:49 -0500 Subject: [PATCH 4/6] test(indirect): enforce real bounds in the hostile-cycle harness The bounded child runner now drains each pipe on its own reader thread with an 8 KiB sample cap, kills the child past the wall limit or on oversize, and launches through best-effort ulimit CPU/stack caps with exec keeping the CLI as producer. Oversize and timeout are harness failures, never silent cuts. Adds helper self-tests: a hung child must report timeout, an overproducer must report oversize. Hostile self/mutual cycles assert refusal with the real producer exit. --- .../tests/integration/threat_model_tests.rs | 170 +++++++++++++++--- 1 file changed, 147 insertions(+), 23 deletions(-) diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 9cf3d67f7..18c2ddbc4 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -9,6 +9,7 @@ use bashkit::{ Bash, ExecutionLimits, FileSystem, FileSystemExt, FsLimits, InMemoryFs, MemoryLimits, OverlayFs, SessionLimits, TraceEventDetails, TraceEventKind, TraceMode, }; +use std::io::Read; use std::path::Path; use std::process::{Command, Stdio}; use std::sync::Arc; @@ -34,45 +35,138 @@ mod resource_exhaustion { struct BoundedRun { timed_out: bool, + oversized: bool, exit_code: Option, stdout: String, stderr: String, } - /// Runs a script in a child CLI process under a wall-clock watchdog. - /// Darwin note: Rust std exposes no portable per-child CPU, stack, or - /// memory caps, so the watchdog kill plus the 8 KiB output cap are the - /// enforced bounds here. Records the real producer exit status. + /// Per-pipe sample cap. Past the cap the reader stops draining and the + /// run is marked `oversized`, which fails the harness: oversize output + /// is never silently truncated. + const BOUNDED_SAMPLE_CAP: usize = 8192; + + /// Reads one pipe to the sample cap on its own thread (concurrent with + /// the sibling pipe, so a chatty child can never deadlock the runner), + /// then reports its bytes. Past the cap it stops draining: the child + /// blocks on the full pipe until the poll loop kills it. + fn spawn_capped_reader( + id: u8, + pipe: Box, + tx: std::sync::mpsc::Sender<(u8, Vec, bool)>, + ) -> std::thread::JoinHandle<()> { + std::thread::spawn(move || { + let mut pipe = pipe; + let mut buf = Vec::new(); + let mut chunk = [0u8; 4096]; + let mut oversized = false; + loop { + match pipe.read(&mut chunk) { + Ok(0) => break, + Ok(n) => { + buf.extend_from_slice(&chunk[..n]); + if buf.len() > BOUNDED_SAMPLE_CAP { + oversized = true; + break; + } + } + Err(_) => break, + } + } + let _ = tx.send((id, buf, oversized)); + }) + } + + /// Runs a script in a child CLI process with enforced bounds, and + /// records the real producer exit status: + /// - wall clock: the poll loop kills the child past `limit`; + /// - output: one reader thread per pipe stops past the sample cap, the + /// child is killed, and the run is marked `oversized` (a harness + /// failure, never a silent cut); + /// - CPU/stack: the child is launched through `ulimit -t/-s` where the + /// launcher shell supports it (best effort; Linux and macOS do), with + /// `exec` keeping the CLI as the real producer so exit codes and + /// diagnostics stay its own. Rust std exposes no portable per-child + /// CPU/stack/memory caps, and Darwin does not enforce address-space + /// caps: those are documented here, not claimed. fn run_cli_bounded(script: &str, limit: Duration) -> BoundedRun { - let mut child = Command::new(bashkit_cli_path()) + // Best-effort launcher limits: 10 CPU seconds, 4 MiB main-thread + // stack. Failures fall through; the watchdog stays the hard bound. + let launcher = "ulimit -s 4096 2>/dev/null; ulimit -t 10 2>/dev/null; exec \"$@\""; + let mut child = Command::new("bash") + .args(["-c", launcher, "_"]) + .arg(bashkit_cli_path()) .args(["-c", script]) .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .expect("spawn bashkit CLI"); + .expect("spawn bounded CLI"); + let (tx, rx) = std::sync::mpsc::channel(); + let mut readers = Vec::new(); + if let Some(pipe) = child.stdout.take() { + readers.push(spawn_capped_reader(0, Box::new(pipe), tx.clone())); + } + if let Some(pipe) = child.stderr.take() { + readers.push(spawn_capped_reader(1, Box::new(pipe), tx.clone())); + } + drop(tx); + let mut bufs: [Option>; 2] = [None, None]; + let mut oversized = false; + let mut status = None; + let mut timed_out = false; let start = Instant::now(); - let timed_out = loop { - match child.try_wait().expect("poll child") { - Some(_) => break false, - None => { - if start.elapsed() >= limit { - let _ = child.kill(); - break true; - } - std::thread::sleep(Duration::from_millis(10)); + loop { + while let Ok((id, buf, over)) = rx.try_recv() { + bufs[id as usize] = Some(buf); + oversized |= over; + } + if bufs[0].is_some() && bufs[1].is_some() { + break; + } + if oversized { + let _ = child.kill(); + } + if status.is_none() { + status = child.try_wait().expect("poll child"); + } + if status.is_some() && bufs[0].is_some() && bufs[1].is_some() { + break; + } + if status.is_none() && start.elapsed() >= limit { + let _ = child.kill(); + timed_out = true; + } + if (status.is_some() || timed_out) && bufs[0].is_some() && bufs[1].is_some() { + break; + } + std::thread::sleep(Duration::from_millis(10)); + if timed_out && bufs[0].is_some() && bufs[1].is_some() { + break; + } + } + while bufs[0].is_none() || bufs[1].is_none() { + match rx.recv() { + Ok((id, buf, over)) => { + bufs[id as usize] = Some(buf); + oversized |= over; } + Err(_) => break, } - }; - let output = child.wait_with_output().expect("collect child output"); - const CAP: usize = 8192; - let mut stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - let mut stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - stdout.truncate(CAP); - stderr.truncate(CAP); + } + for reader in readers { + let _ = reader.join(); + } + // Always reaped explicitly: `try_wait` may already have seen the + // exit, in which case this returns immediately with the same status. + let reaped = child.wait().expect("reap child"); + let status = Some(status.unwrap_or(reaped)); + let stdout = String::from_utf8_lossy(bufs[0].as_deref().unwrap_or(&[])).into_owned(); + let stderr = String::from_utf8_lossy(bufs[1].as_deref().unwrap_or(&[])).into_owned(); BoundedRun { timed_out, - exit_code: output.status.code(), + oversized, + exit_code: status.and_then(|s| s.code()), stdout, stderr, } @@ -92,6 +186,7 @@ mod resource_exhaustion { ] { let run = run_cli_bounded(script, Duration::from_secs(20)); assert!(!run.timed_out, "hostile script hung: {script}"); + assert!(!run.oversized, "hostile script oversized: {script}"); assert_eq!( run.exit_code, Some(1), @@ -109,6 +204,35 @@ mod resource_exhaustion { } } + /// The bounded runner itself is tested: a hung child must be + /// reported as a timeout, and an overproducer as oversize. Both fail + /// visibly (a boolean the harness asserts on), so a helper regression + /// cannot silently pass hostile coverage. + #[test] + fn bounded_helper_kills_hung_child() { + let run = run_cli_bounded("sleep 30", Duration::from_secs(3)); + assert!( + run.timed_out, + "hung child not reported: {:?}", + run.exit_code + ); + assert!(!run.oversized, "hung child misreported as oversize"); + } + + #[test] + fn bounded_helper_flags_oversize_output() { + let script = + "i=0; while ((i < 4000)); do echo \"padding line $i padding\"; i=$((i + 1)); done"; + let run = run_cli_bounded(script, Duration::from_secs(20)); + assert!(!run.timed_out, "overproducer hung"); + assert!(run.oversized, "overproducer not flagged as oversize"); + assert!( + run.stdout.len() <= BOUNDED_SAMPLE_CAP + 4096, + "sample not capped: {}", + run.stdout.len() + ); + } + /// TM-DOS-135: a bracket indirect target is refused unless the first /// complete `[...]` reference consumes the whole value. Modern bash /// (5.x) rejects trailing operators with `invalid variable name`; From 2df6999233ed1486f847712a477b0da09e6667f7 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 22:47:00 -0500 Subject: [PATCH 5/6] test(indirect): fail-closed limits and single-deadline bounded runner The hostile-cycle harness launcher now refuses without CPU/stack caps (exit 99) instead of suppressing ulimit failures, and reports the effective limits for the harness to assert. One shared wall deadline covers child exit, pipe drain, and reaping: pipe EOF alone never ends the run, retained pipes are reported incomplete within bounds, and oversize still fails visibly. Self-tests cover launcher refusal, early pipe close while live, retained pipes, hung children, and overproducers. --- .../tests/integration/threat_model_tests.rs | 204 ++++++++++++++---- 1 file changed, 165 insertions(+), 39 deletions(-) diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 18c2ddbc4..2f43c034c 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -36,11 +36,25 @@ mod resource_exhaustion { struct BoundedRun { timed_out: bool, oversized: bool, + /// True when the deadline passed with readers unfinished: a foreign + /// descendant retains the pipes. Detection, not reaping: without + /// platform process-group APIs (no new dependencies) a foreign + /// descendant cannot be reaped portably, so the run fails visibly. + incomplete_readers: bool, + /// First stderr line from the launcher (`harness-limits: ...`), + /// proving the CPU/stack caps were applied to this run. Empty when + /// the launcher itself refused (fail-closed exit 99). + limits_note: String, exit_code: Option, stdout: String, stderr: String, } + /// Fail-closed launcher limits: 10 CPU seconds, 4 MiB main-thread + /// stack. Any refusal exits 99 before the producer starts, so a run + /// without limits can never be mistaken for a bounded one. + const BOUNDED_CLI_LIMITS: &str = "ulimit -s 4096 || exit 99; ulimit -t 10 || exit 99"; + /// Per-pipe sample cap. Past the cap the reader stops draining and the /// run is marked `oversized`, which fails the harness: oversize output /// is never silently truncated. @@ -77,31 +91,50 @@ mod resource_exhaustion { }) } - /// Runs a script in a child CLI process with enforced bounds, and - /// records the real producer exit status: - /// - wall clock: the poll loop kills the child past `limit`; - /// - output: one reader thread per pipe stops past the sample cap, the - /// child is killed, and the run is marked `oversized` (a harness - /// failure, never a silent cut); - /// - CPU/stack: the child is launched through `ulimit -t/-s` where the - /// launcher shell supports it (best effort; Linux and macOS do), with - /// `exec` keeping the CLI as the real producer so exit codes and - /// diagnostics stay its own. Rust std exposes no portable per-child - /// CPU/stack/memory caps, and Darwin does not enforce address-space - /// caps: those are documented here, not claimed. + /// Runs a script in the built CLI under `run_command_bounded`. + /// The launcher applies the fail-closed CPU/stack caps, reports the + /// effective values as the first stderr line, then `exec`s the CLI so + /// exit codes and diagnostics stay the real producer's own. fn run_cli_bounded(script: &str, limit: Duration) -> BoundedRun { - // Best-effort launcher limits: 10 CPU seconds, 4 MiB main-thread - // stack. Failures fall through; the watchdog stays the hard bound. - let launcher = "ulimit -s 4096 2>/dev/null; ulimit -t 10 2>/dev/null; exec \"$@\""; - let mut child = Command::new("bash") - .args(["-c", launcher, "_"]) + let launcher = format!( + "{BOUNDED_CLI_LIMITS}; echo \"harness-limits: stack=$(ulimit -s) cpu=$(ulimit -t)\" >&2; exec \"$@\"" + ); + let mut cmd = Command::new("bash"); + cmd.args(["-c", &launcher, "_"]) .arg(bashkit_cli_path()) .args(["-c", script]) .stdin(Stdio::null()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("spawn bounded CLI"); + .stderr(Stdio::piped()); + let mut run = run_command_bounded(cmd, limit); + (run.limits_note, run.stderr) = match run.stderr.split_once('\n') { + Some((first, rest)) if first.starts_with("harness-limits:") => { + (first.to_string(), rest.to_string()) + } + _ => (String::new(), run.stderr), + }; + run + } + + /// Runs an already-built command with one shared wall deadline for + /// child exit, pipe drain, and reaping: + /// - wall clock: past `limit` the child is killed; if it had already + /// exited, the run is not marked timed out; + /// - output: one reader thread per pipe stops past the sample cap, the + /// child is killed, and the run is marked `oversized` (a harness + /// failure, never a silent cut). Readers run concurrently, so a + /// chatty child can never deadlock the runner on a full pipe; + /// - drain: after the break, missing readers get a short grace period. + /// Anything still missing means a foreign descendant retains the + /// pipe: the run is marked `incomplete_readers` and fails visibly + /// instead of waiting forever. Reader threads for retained pipes are + /// dropped, not joined without a deadline (one documented thread + /// leak per such run, only in the retention self-test). + /// - CPU/stack caps are the launcher's job (see above); Rust std + /// exposes no portable per-child CPU/stack/memory caps, and Darwin + /// does not enforce address-space caps: documented here, not claimed. + fn run_command_bounded(mut cmd: Command, limit: Duration) -> BoundedRun { + let mut child = cmd.spawn().expect("spawn bounded child"); let (tx, rx) = std::sync::mpsc::channel(); let mut readers = Vec::new(); if let Some(pipe) = child.stdout.take() { @@ -114,48 +147,54 @@ mod resource_exhaustion { let mut bufs: [Option>; 2] = [None, None]; let mut oversized = false; let mut status = None; + let mut killed = false; let mut timed_out = false; - let start = Instant::now(); + let deadline = Instant::now() + limit; + // Pipe EOF alone never ends the run: only an observed exit does, + // otherwise a child that closes output and keeps running would + // bypass the watchdog into an unbounded wait. loop { while let Ok((id, buf, over)) = rx.try_recv() { bufs[id as usize] = Some(buf); oversized |= over; } - if bufs[0].is_some() && bufs[1].is_some() { + if bufs[0].is_some() && bufs[1].is_some() && status.is_some() { break; } - if oversized { + if (oversized || Instant::now() >= deadline) && !killed { let _ = child.kill(); + killed = true; } if status.is_none() { status = child.try_wait().expect("poll child"); } - if status.is_some() && bufs[0].is_some() && bufs[1].is_some() { - break; - } - if status.is_none() && start.elapsed() >= limit { - let _ = child.kill(); - timed_out = true; - } - if (status.is_some() || timed_out) && bufs[0].is_some() && bufs[1].is_some() { + if Instant::now() >= deadline { + timed_out = status.is_none(); break; } std::thread::sleep(Duration::from_millis(10)); - if timed_out && bufs[0].is_some() && bufs[1].is_some() { - break; - } } - while bufs[0].is_none() || bufs[1].is_none() { - match rx.recv() { + // Bounded drain grace: an exited child with unretained pipes hits + // EOF promptly; anything still missing is a retained pipe. + let grace_end = Instant::now() + Duration::from_secs(2); + while (bufs[0].is_none() || bufs[1].is_none()) && Instant::now() < grace_end { + match rx.recv_timeout(Duration::from_millis(50)) { Ok((id, buf, over)) => { bufs[id as usize] = Some(buf); oversized |= over; } - Err(_) => break, + Err(_) => { + if bufs[0].is_some() && bufs[1].is_some() { + break; + } + } } } - for reader in readers { - let _ = reader.join(); + let incomplete_readers = bufs[0].is_none() || bufs[1].is_none(); + if !incomplete_readers { + for reader in readers { + let _ = reader.join(); + } } // Always reaped explicitly: `try_wait` may already have seen the // exit, in which case this returns immediately with the same status. @@ -166,6 +205,8 @@ mod resource_exhaustion { BoundedRun { timed_out, oversized, + incomplete_readers, + limits_note: String::new(), exit_code: status.and_then(|s| s.code()), stdout, stderr, @@ -187,6 +228,15 @@ mod resource_exhaustion { let run = run_cli_bounded(script, Duration::from_secs(20)); assert!(!run.timed_out, "hostile script hung: {script}"); assert!(!run.oversized, "hostile script oversized: {script}"); + assert!( + !run.incomplete_readers, + "hostile script readers incomplete: {script}" + ); + assert!( + run.limits_note.contains("stack=4096") && run.limits_note.contains("cpu=10"), + "limits not applied: {}", + run.limits_note + ); assert_eq!( run.exit_code, Some(1), @@ -217,6 +267,78 @@ mod resource_exhaustion { run.exit_code ); assert!(!run.oversized, "hung child misreported as oversize"); + assert!( + !run.incomplete_readers, + "hung child misreported as incomplete" + ); + } + + /// The launcher is fail-closed: if `ulimit` refuses, the producer never + /// starts (exit 99) instead of running unbounded. A shadowed failing + /// `ulimit` exercises the refusal with a harmless instant child. + #[test] + fn bounded_launcher_failure_is_fail_closed() { + let probe = [ + "ulimit() { return 1; }; ", + BOUNDED_CLI_LIMITS, + "; echo SURVIVED", + ] + .concat(); + let out = Command::new("bash") + .args(["-c", &probe]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn launcher probe") + .wait_with_output() + .expect("reap launcher probe"); + assert_eq!( + out.status.code(), + Some(99), + "stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + !String::from_utf8_lossy(&out.stdout).contains("SURVIVED"), + "launcher ran the producer without limits" + ); + } + + /// Both pipes hit EOF while the producer lives: only the wall deadline + /// may end the run. The previous loop broke on complete buffers and + /// then waited without a deadline; this case hung it forever. + #[test] + fn bounded_helper_covers_early_pipe_close() { + let run = run_cli_bounded("echo hi; exec 1>&- 2>&-; sleep 30", Duration::from_secs(5)); + assert!(run.timed_out, "live producer with closed pipes not killed"); + assert!(!run.oversized, "early close misreported as oversize"); + assert!( + !run.incomplete_readers, + "closed pipes misreported as incomplete" + ); + assert!(run.stdout.contains("hi"), "stdout: {}", run.stdout); + assert_eq!(run.exit_code, None, "expected SIGKILL, no exit code"); + } + + /// An exited child with descendant-retained pipes: the exit is observed + /// but readers never finish. The helper must report incomplete readers + /// within bounds, not wait forever. Real bash (not the CLI) is the + /// pipe holder here; the orphaned `sleep` is harmless and bounded. + #[test] + fn bounded_helper_flags_retained_pipes() { + let mut cmd = Command::new("bash"); + cmd.args(["-c", "sleep 30 & exit 0"]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let run = run_command_bounded(cmd, Duration::from_secs(5)); + assert!(!run.timed_out, "exited child misreported as timeout"); + assert!( + run.incomplete_readers, + "retained pipes not reported as incomplete" + ); + assert_eq!(run.exit_code, Some(0), "stdout: {}", run.stdout); } #[test] @@ -226,6 +348,10 @@ mod resource_exhaustion { let run = run_cli_bounded(script, Duration::from_secs(20)); assert!(!run.timed_out, "overproducer hung"); assert!(run.oversized, "overproducer not flagged as oversize"); + assert!( + !run.incomplete_readers, + "overproducer misreported as incomplete" + ); assert!( run.stdout.len() <= BOUNDED_SAMPLE_CAP + 4096, "sample not capped: {}", From 32dd7776361d15e3132db4fb22e10536817c62de Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 23:16:29 -0500 Subject: [PATCH 6/6] test(indirect): deterministic deadline verdict in bounded runner Record the wall-deadline verdict from pre-kill observed liveness instead of recomputing it after polling try_wait, so reaping the SIGKILL victim cannot flip a genuine timeout. Covers the verdict with repeated short-deadline runs. --- .../tests/integration/threat_model_tests.rs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 2f43c034c..e415566ce 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -147,12 +147,14 @@ mod resource_exhaustion { let mut bufs: [Option>; 2] = [None, None]; let mut oversized = false; let mut status = None; - let mut killed = false; let mut timed_out = false; let deadline = Instant::now() + limit; // Pipe EOF alone never ends the run: only an observed exit does, // otherwise a child that closes output and keeps running would - // bypass the watchdog into an unbounded wait. + // bypass the watchdog into an unbounded wait. The deadline verdict + // is recorded from pre-kill observed liveness and never recomputed + // after killing: reaping the SIGKILL victim must not flip a genuine + // timeout to false on scheduling luck. loop { while let Ok((id, buf, over)) = rx.try_recv() { bufs[id as usize] = Some(buf); @@ -161,15 +163,18 @@ mod resource_exhaustion { if bufs[0].is_some() && bufs[1].is_some() && status.is_some() { break; } - if (oversized || Instant::now() >= deadline) && !killed { + if oversized { + // `kill` is idempotent; errors (already exited) are ignored. let _ = child.kill(); - killed = true; } if status.is_none() { status = child.try_wait().expect("poll child"); } if Instant::now() >= deadline { - timed_out = status.is_none(); + if status.is_none() && !oversized { + let _ = child.kill(); + timed_out = true; + } break; } std::thread::sleep(Duration::from_millis(10)); @@ -257,7 +262,9 @@ mod resource_exhaustion { /// The bounded runner itself is tested: a hung child must be /// reported as a timeout, and an overproducer as oversize. Both fail /// visibly (a boolean the harness asserts on), so a helper regression - /// cannot silently pass hostile coverage. + /// cannot silently pass hostile coverage. The short-deadline case runs + /// repeatedly: the timeout verdict must be deterministic, never a + /// scheduling coin flip. #[test] fn bounded_helper_kills_hung_child() { let run = run_cli_bounded("sleep 30", Duration::from_secs(3)); @@ -271,6 +278,12 @@ mod resource_exhaustion { !run.incomplete_readers, "hung child misreported as incomplete" ); + // Repeated short deadlines: every run must report the timeout. + for _ in 0..3 { + let run = run_cli_bounded("sleep 30", Duration::from_secs(2)); + assert!(run.timed_out, "short deadline flaked"); + assert!(!run.oversized, "short deadline misreported as oversize"); + } } /// The launcher is fail-closed: if `ulimit` refuses, the producer never