From 0e42fc61c0fad02b09cf50ebd30e52a827a81eee Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 09:54:02 -0500 Subject: [PATCH 1/2] fix(arrays): release mapfile budgets and surface refusals --- .../__test__/bashkit-wasm.test.mjs | 31 ++++ crates/bashkit/src/builtins/mapfile.rs | 7 + crates/bashkit/src/interpreter/mod.rs | 69 ++++---- .../array_budget_security_tests.rs | 147 ++++++++++++++++++ .../tests/integration/threat_model_tests.rs | 32 ++-- .../tests/spec_cases/bash/arrays.test.sh | 20 +++ docs/security.md | 8 + knowledge/foundations/builtins.md | 20 +++ knowledge/operations/limitations.md | 1 + knowledge/security/threat-model.md | 2 +- 10 files changed, 288 insertions(+), 49 deletions(-) diff --git a/crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs b/crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs index 3026da0be..6e752bb47 100644 --- a/crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs +++ b/crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs @@ -491,3 +491,34 @@ test("multiple independent instances do not share state", () => { assert.equal(b.executeSync("echo [${SHARED-none}]").stdout, "[none]\n"); assert.equal(b.exists("/marker.txt"), false); }); + +// Independent brush-web reproduction: cumulative reads must not exhaust the +// retained array-entry budget or corrupt later arrays in the same instance. +test("mapfile: repeated large reads preserve session arrays", async () => { + const bash = new Bash(); + const r = await bash.execute(` + seq 50000 > /tmp/s + for i in 1 2 3; do + mapfile -t a < /tmp/s + echo "mapfile=$? n=\${#a[@]}" + done + b=(x y z); echo "b=\${#b[@]}" + `); + assert.equal(r.exitCode, 0); + assert.equal(r.stderr, ""); + assert.equal(r.stdout, "mapfile=0 n=50000\n".repeat(3) + "b=3\n"); + const next = await bash.execute("c=(still works); echo \"${c[*]}\""); + assert.equal(next.exitCode, 0); + assert.equal(next.stdout, "still works\n"); +}); + +test("mapfile: entry exhaustion fails visibly and the session recovers", async () => { + const bash = new Bash(); + await assert.rejects( + bash.execute("seq 50001 > /tmp/rows; seq 50000 >> /tmp/rows; mapfile -t a < /tmp/rows"), + /array entry limit \(100000\) exceeded/, + ); + const next = await bash.execute("unset a; b=(x y z); echo ${#b[@]}"); + assert.equal(next.exitCode, 0); + assert.equal(next.stdout, "3\n"); +}); diff --git a/crates/bashkit/src/builtins/mapfile.rs b/crates/bashkit/src/builtins/mapfile.rs index 4bbe46b34..d348ffdcf 100644 --- a/crates/bashkit/src/builtins/mapfile.rs +++ b/crates/bashkit/src/builtins/mapfile.rs @@ -2,6 +2,10 @@ //! //! Mutates arrays via [`BuiltinSideEffect::SetIndexedArray`](super::BuiltinSideEffect). //! +//! Decision: debug-stack arrays are interpreter-owned, uncharged metadata. +//! Like bash, reject them as destinations rather than refunding their entries +//! as user-array capacity when clearing the destination. +//! //! Decision: `-u FD` other than 0 and the `-C`/`-c` callback are rejected with //! status 2 instead of being ignored; reading the wrong input silently would //! be worse than a clear error (see L-MAPFILE-001 in operations/limitations). @@ -101,6 +105,9 @@ impl Builtin for Mapfile { Ok(o) => o, Err(msg) => return Ok(ExecResult::err(format!("mapfile: {msg}\n"), 2)), }; + if matches!(o.name.as_str(), "FUNCNAME" | "BASH_SOURCE" | "BASH_LINENO") { + return Ok(ExecResult::err(String::new(), 1)); + } let input = ctx.stdin.map(|stdin| &**stdin).unwrap_or(""); let mut result = ExecResult::ok(String::new()); diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 2d01f7f18..efe621a6f 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -10,6 +10,10 @@ // validating string contents. This is safe because we check for non-empty strings. #![allow(clippy::unwrap_used)] +// Decision: builtin array mutations use the same retained entry/byte accounting +// as assignments. Clearing refunds both charges; indexed merges check each live +// element in place. Refused entry/byte writes fail the host execution visibly. + // Decision: function definition filenames share Arc storage and live in // ScopedState beside functions. Charge filename/key bytes per function before // admission; replace/refund them with the definition so forks, rollback and @@ -11593,18 +11597,13 @@ impl Interpreter { // THREAT[TM-DOS-114]: `unset arr` must return the array's entry // slots and bytes to the budget. Without this a set/unset cycle // drifts until a healthy script is refused. - let released = self - .arrays_mut() - .remove(name) - .map_or((0, 0), |arr| (arr.len(), Self::indexed_array_bytes(&arr))); + self.remove_indexed_array(name); let released_assoc = self .assoc_arrays_mut() .remove(name) .map_or((0, 0), |arr| (arr.len(), Self::assoc_array_bytes(&arr))); - self.memory_budget - .record_array_remove(released.0 + released_assoc.0); - self.memory_budget - .release_array_bytes(released.1 + released_assoc.1); + self.memory_budget.record_array_remove(released_assoc.0); + self.memory_budget.release_array_bytes(released_assoc.1); self.clear_var_attrs(name); self.remove_nameref(name); } @@ -12469,10 +12468,14 @@ impl Interpreter { builtins::BuiltinSideEffect::SetIndexedArray { name, entries } => { // Merges into the existing array; mapfile sends RemoveArray // first unless `-O` asked to keep the other elements. - let mut arr = self.arrays_mut().remove(name).unwrap_or_default(); - arr.extend(entries.iter().cloned()); - if !arr.is_empty() { - self.insert_array_checked(name.clone(), arr); + // Keep retained storage visible to admission so overwrites + // refund their old values and refused writes preserve them. + if !entries.is_empty() && self.env.contains_key(name) { + self.env_mut().remove(name); + } + for (idx, value) in entries { + self.set_indexed_element_checked(name, *idx, value.clone()); + self.check_cancelled()?; } } builtins::BuiltinSideEffect::SignalSelf(signal) => { @@ -12482,7 +12485,7 @@ impl Interpreter { self.builtin_discard = true; } builtins::BuiltinSideEffect::RemoveArray(name) => { - self.arrays_mut().remove(name); + self.remove_indexed_array(name); } builtins::BuiltinSideEffect::ShiftPositional(n) => { // bash: a count above `$#` shifts nothing and fails. @@ -14017,6 +14020,16 @@ impl Interpreter { } } + /// Removing retained storage must release both session budget charges. + /// Shared by unset and builtin replacement, including empty mapfile input. + fn remove_indexed_array(&mut self, name: &str) { + if let Some(arr) = self.arrays_mut().remove(name) { + self.memory_budget.record_array_remove(arr.len()); + self.memory_budget + .release_array_bytes(Self::indexed_array_bytes(&arr)); + } + } + /// Bytes charged for an indexed array's contents. Indexed keys are machine /// integers already covered by the entry count, so only values are charged. fn indexed_array_bytes(arr: &HashMap) -> usize { @@ -14032,22 +14045,20 @@ impl Interpreter { /// Charge an array write against the entry count and the shared /// retained-byte budget. Returns `false` when the write must be skipped. /// - /// THREAT[TM-DOS-114]: an over-budget *byte* write records the first - /// rejection so execution fails visibly rather than dropping the value. - /// Hitting `max_array_entries` keeps its established silent-skip behaviour — - /// that counter is a separate contract (TM-DOS-060) with its own tests, and - /// widening it here would change unrelated semantics. + /// THREAT[TM-DOS-060] THREAT[TM-DOS-114]: entry and byte refusals + /// record the first rejection so execution fails visibly rather than + /// silently dropping values and reporting success. fn admit_array_write( &mut self, new_entries: usize, added_bytes: usize, removed_bytes: usize, ) -> bool { - if self + if let Err(error) = self .memory_budget .check_array_entries(new_entries, &self.memory_limits) - .is_err() { + self.memory_limit_error.get_or_insert(error); return false; } if let Err(error) = @@ -14079,12 +14090,11 @@ impl Interpreter { .map_or((0, 0), |a| (a.len(), Self::indexed_array_bytes(a))); let net = new_entries.saturating_sub(old_entries); let new_bytes = Self::indexed_array_bytes(&arr); - if net > 0 - && self - .memory_budget - .check_array_entries(net, &self.memory_limits) - .is_err() + if let Err(error) = self + .memory_budget + .check_array_entries(net, &self.memory_limits) { + self.memory_limit_error.get_or_insert(error); return false; } if let Err(error) = @@ -14113,12 +14123,11 @@ impl Interpreter { .map_or((0, 0), |a| (a.len(), Self::assoc_array_bytes(a))); let net = new_entries.saturating_sub(old_entries); let new_bytes = Self::assoc_array_bytes(&arr); - if net > 0 - && self - .memory_budget - .check_array_entries(net, &self.memory_limits) - .is_err() + if let Err(error) = self + .memory_budget + .check_array_entries(net, &self.memory_limits) { + self.memory_limit_error.get_or_insert(error); return false; } if let Err(error) = diff --git a/crates/bashkit/tests/integration/array_budget_security_tests.rs b/crates/bashkit/tests/integration/array_budget_security_tests.rs index 26d7a5a55..f4c62e3ff 100644 --- a/crates/bashkit/tests/integration/array_budget_security_tests.rs +++ b/crates/bashkit/tests/integration/array_budget_security_tests.rs @@ -167,3 +167,150 @@ async fn default_limits_admit_a_realistic_array_workload() { assert_eq!(result.exit_code, 0, "stderr: {}", result.stderr); assert_eq!(result.stdout, "2000 row-2000\n"); } + +/// Repeated replacement must charge retained state, not cumulative read volume. +#[tokio::test] +async fn mapfile_replacement_releases_entries_and_bytes() { + for options in ["-t", "-t -O 0"] { + let mut bash = Bash::builder() + .memory_limits( + MemoryLimits::new() + .max_array_entries(4) + .max_total_variable_bytes(4096), + ) + .build(); + let script = format!( + "export a=old; for i in {{1..12}}; do mapfile {options} a <<<'one\ntwo\nthree'; echo ${{#a[@]}}; done; b=(ok); echo ${{b[0]}}; if env | grep -q '^a='; then echo leaked; fi" + ); + let result = bash.exec(&script).await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, format!("{}ok\n", "3\n".repeat(12))); + let result = bash + .exec("mapfile -t a <<<'last'; b=(x y); echo ${a[*]} ${#b[@]}") + .await + .unwrap(); + assert_eq!(result.stdout, "last 2\n"); + } +} + +#[tokio::test] +async fn mapfile_empty_replacement_releases_budget() { + let mut bash = Bash::builder() + .memory_limits(MemoryLimits::new().max_array_entries(3)) + .build(); + let result = bash + .exec("a=(one two three); mapfile -t a /tmp/s; for i in 1 2 3; do mapfile -t a < /tmp/s; echo \"mapfile=$? n=${#a[@]}\"; done; b=(x y z); echo \"b=${#b[@]}\"").await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!( + result.stdout, + "mapfile=0 n=50000\nmapfile=0 n=50000\nmapfile=0 n=50000\nb=3\n" + ); + let result = bash.exec("c=(still works); echo ${c[*]}").await.unwrap(); + assert_eq!(result.stdout, "still works\n"); +} + +/// Debug-stack metadata is uncharged: mapfile must not refund it as user data. +#[tokio::test] +async fn mapfile_cannot_refund_interpreter_metadata() { + for name in ["FUNCNAME", "BASH_SOURCE", "BASH_LINENO"] { + for options in ["-t", "-t -O 0"] { + let mut bash = Bash::builder() + .memory_limits(MemoryLimits::new().max_array_entries(3)) + .build(); + let script = + format!("a=(one two three); f() {{ mapfile {options} {name} /tmp/f` across many `N` values, or `cmd {v}>/tmp/f` in a loop (bash keeps `{var}` fds open), grows `exec_fd_table`/coproc fd buffers for the session | `ExecutionLimits::max_file_descriptors` (default 1024) caps persistent custom descriptors; reused fds and standard 0/1/2 don't count. A command's own `N<= 3) fd takes a slot for that command only and a refused open closes the ones it already opened (`here_doc_fds_count_against_fd_cap_and_close`). A `coproc` needs two free slots and also takes a background-job slot (TM-DOS-122); its pipes hold at most 4 KiB plus one command's output before the writer waits | **MITIGATED** | | TM-DOS-109 | Silent truncation at builtin caps | A builtin that stops at a resource cap but exits 0 with no diagnostic (awk loops/recursion/getline caps, `seq`, `yes`, `numfmt`, `history`), or an awk `sprintf` expression that reports a format cap yet returns an empty value, gives scripts and agents incomplete output as success (#2446, #2456, like sed #1005) | Reaching a builtin cap is never silent: `builtins::limits::cap_exceeded` keeps the partial output, writes `: limit () exceeded; output truncated` and exits 1; awk caps and formatting errors are gawk-style fatal errors (`awk: fatal: ...`, exit 2, END skipped). Regression tests: `builtin_cap_security_tests` | **MITIGATED** | | TM-DOS-110 | In-builtin memory growth aborts the host | A value that keeps growing inside awk (`s = s s`, `gsub` with a long replacement, `sprintf("%s%s%s", s, s, s)`, `$1000000000 = 1`, filling arrays, deep recursion shadowing large values) or jq (`until(false; . + .)`, `"x" * 1e12`, `[range(1e12)]`, a non-emitting `until(false; .)` growing evaluator state) allocates until the allocator fails and the embedding process aborts with SIGABRT (#2444) | awk: every string is checked against `AWK_MAX_STRING_BYTES` (16 MiB) *before* it is allocated (concat, `SUBSEP` keys, `sub`/`gsub`/`gensub` as the result grows, `sprintf`/`printf`, `print` joins, rebuilt `$0`); `$N` assignment is capped at `AWK_MAX_FIELD_INDEX`; variables, array elements and values parked in call frames are accounted and capped at `ExecutionLimits::max_live_intermediate_bytes` (default 32 MB); each is a fatal awk error (exit 2) and the script carries on. jq: jaq-json is vendored with a live-memory meter charging every string and array/object body to the same limit, checked before growth (`jq: error: value size limit (N bytes) exceeded`, exit 5), sticky across `try`; value operations poll the execution deadline so non-emitting loops stop at the timeout; output conversion is capped by `max_stdout_bytes`; `--stream` events are produced incrementally with shared ancestor keys, metered cursor/path/event bodies and pre-allocation checks, including `--slurp`, and every traversal step checks shared work/cancellation/deadline limits. Regression tests: stream memory/work coverage in `execution_budget_tests` and `memory_growth_security_tests`. Jq evaluator contexts are now counted (64 live-context ceiling) and every context clone polls the deadline, so pure tail recursion times out and non-tail recursion fails before host stack exhaustion (#2465). The unwind-based abort requires `panic = "unwind"`; panic-abort embedders need process isolation for untrusted jq | **MITIGATED** | -| TM-DOS-114 | Arrays bypass the retained-memory byte budget | `max_array_entries` counted entries but never their keys or values, so `x` doubled to 4 MiB and parked in ten array slots retained ~40 MB past the 10 MB variable-byte budget and exited 0 (#2462) | Array keys and values charge `max_total_variable_bytes` alongside scalars (`MemoryBudget::array_bytes`; indexed keys are machine integers already covered by the entry count, associative keys are attacker-controlled so they count). Element writes, `+=` appends, bulk `a=(...)` replacement, and `declare -A` compound writes are checked before insertion; bytes are released on element unset, whole-array unset, replacement, and local scope pop. An over-budget write records the first memory-limit error and fails execution (TM-DOS-111 mechanism) rather than dropping the value; reaching `max_array_entries` keeps its established silent-skip behaviour (TM-DOS-060). Indexed compound assignments also keep expanded keys, values, fields and their container capacity leased against `max_live_intermediate_bytes` until consumed; duplicate indices cannot evade this transient cap. Scalar storage is borrowed during expansion so pending copies reserve capacity before allocation. Consumed command-substitution output releases its temporary lease once the pending word buffer owns the bytes. Value-first expansion and ordered subscript evaluation remain intact. Regression tests: `array_budget_security_tests`, `execution_budget_tests::indexed_array_*` | **MITIGATED** | +| TM-DOS-114 | Arrays bypass the retained-memory byte budget | `max_array_entries` counted entries but never their keys or values, so `x` doubled to 4 MiB and parked in ten array slots retained ~40 MB past the 10 MB variable-byte budget and exited 0 (#2462) | Array keys and values charge `max_total_variable_bytes` alongside scalars (`MemoryBudget::array_bytes`; indexed keys are machine integers already covered by the entry count, associative keys are attacker-controlled so they count). Element writes, `+=` appends, bulk `a=(...)` replacement, and `declare -A` compound writes are checked before insertion; bytes are released on element unset, whole-array unset, replacement, and local scope pop. An over-budget write records the first memory-limit error and fails execution (TM-DOS-111 mechanism) rather than dropping the value; Entry-limit refusals use the same visible resource-error path (TM-DOS-060). `mapfile` clearing releases both entry and byte charges, including empty input; indexed builtin updates use checked element writes against existing retained storage so `-O` overwrites refund replaced values without removing the destination first. `mapfile` rejects the uncharged `FUNCNAME`/`BASH_SOURCE`/`BASH_LINENO` metadata arrays, preventing false refunds of user capacity. Indexed compound assignments also keep expanded keys, values, fields and their container capacity leased against `max_live_intermediate_bytes` until consumed; duplicate indices cannot evade this transient cap. Scalar storage is borrowed during expansion so pending copies reserve capacity before allocation. Consumed command-substitution output releases its temporary lease once the pending word buffer owns the bytes. Value-first expansion and ordered subscript evaluation remain intact. Regression tests: `array_budget_security_tests`, `execution_budget_tests::indexed_array_*` | **MITIGATED** | | TM-DOS-115 | `$( Date: Thu, 15 Oct 2026 10:05:28 -0500 Subject: [PATCH 2/2] test(arrays): isolate resource caps from execution deadlines --- .../tests/integration/array_budget_security_tests.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/bashkit/tests/integration/array_budget_security_tests.rs b/crates/bashkit/tests/integration/array_budget_security_tests.rs index f4c62e3ff..a32427a48 100644 --- a/crates/bashkit/tests/integration/array_budget_security_tests.rs +++ b/crates/bashkit/tests/integration/array_budget_security_tests.rs @@ -15,13 +15,17 @@ //! visible error instead of dropping the write. use bashkit::{Bash, ExecutionLimits, MemoryLimits}; +use std::time::Duration; + +// Resource ceilings must trip before deadlines under instrumented builds. +const CAP_TEST_TIMEOUT: Duration = Duration::from_secs(600); /// 64 KiB of headroom: enough for a handful of small entries, far below the /// megabyte-scale values these tests park in arrays. fn bounded() -> Bash { Bash::builder() .memory_limits(MemoryLimits::new().max_total_variable_bytes(64 * 1024)) - .limits(ExecutionLimits::new()) + .limits(ExecutionLimits::new().timeout(CAP_TEST_TIMEOUT)) .build() } @@ -197,6 +201,7 @@ async fn mapfile_replacement_releases_entries_and_bytes() { async fn mapfile_empty_replacement_releases_budget() { let mut bash = Bash::builder() .memory_limits(MemoryLimits::new().max_array_entries(3)) + .limits(ExecutionLimits::new().timeout(CAP_TEST_TIMEOUT)) .build(); let result = bash .exec("a=(one two three); mapfile -t a