From d03b3a81c34da34d6349a0da5e78ea8515650f2a Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 2 Apr 2026 14:27:12 +0000 Subject: [PATCH] fix(interpreter): save/restore memory_budget in subshell and command substitution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #993 — memory_budget was not saved/restored alongside variables, arrays, and functions in subshell, command substitution, and script execution paths. This caused budget desync where the budget either inflated (preventing legitimate variable creation) or undercounted (allowing limit bypass). Also adds recompute_from_state to MemoryBudget for restore_shell_state snapshots. --- crates/bashkit/src/interpreter/mod.rs | 21 +++++++++ crates/bashkit/src/limits.rs | 34 +++++++++++++++ .../bash/memory_budget_desync.test.sh | 43 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 crates/bashkit/tests/spec_cases/bash/memory_budget_desync.test.sh diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index c79612c9..4e6d333e 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1002,6 +1002,21 @@ impl Interpreter { self.last_exit_code = state.last_exit_code; self.aliases = state.aliases.clone(); self.traps = state.traps.clone(); + // Recompute memory budget from restored state to prevent desync + let func_count = self.functions.len(); + let func_bytes: usize = self + .functions + .values() + .map(|f| format!("{:?}", f.body).len()) + .sum(); + self.memory_budget = crate::limits::MemoryBudget::recompute_from_state( + &self.variables, + &self.arrays, + &self.assoc_arrays, + func_count, + func_bytes, + Self::is_internal_variable, + ); } /// Get a reference to the current execution counters. @@ -1377,6 +1392,7 @@ impl Interpreter { let saved_exit = self.last_exit_code; let saved_aliases = self.aliases.clone(); let saved_coproc = self.coproc_buffers.clone(); + let saved_memory_budget = self.memory_budget.clone(); let mut result = self.execute_command_sequence(commands).await; @@ -1420,6 +1436,7 @@ impl Interpreter { self.last_exit_code = saved_exit; self.aliases = saved_aliases; self.coproc_buffers = saved_coproc; + self.memory_budget = saved_memory_budget; // Consume Exit control flow at subshell boundary — exit only // terminates the subshell, not the parent shell. @@ -3982,6 +3999,7 @@ impl Interpreter { let saved_exit = self.last_exit_code; let saved_aliases = self.aliases.clone(); let saved_coproc = self.coproc_buffers.clone(); + let saved_memory_budget = self.memory_budget.clone(); // Child only sees exported variables (env), not all shell variables. // Reset last_exit_code so $? starts at 0 (matches real bash subprocess). @@ -4024,6 +4042,7 @@ impl Interpreter { self.last_exit_code = saved_exit; self.aliases = saved_aliases; self.coproc_buffers = saved_coproc; + self.memory_budget = saved_memory_budget; self.bash_source_stack = saved_source_stack; self.pipeline_stdin = prev_pipeline_stdin; @@ -5968,6 +5987,7 @@ impl Interpreter { let saved_assoc = self.assoc_arrays.clone(); let saved_aliases = self.aliases.clone(); let saved_cwd = self.cwd.clone(); + let saved_memory_budget = self.memory_budget.clone(); let mut stdout = String::new(); for cmd in commands { let cmd_result = self.execute_command(cmd).await?; @@ -5999,6 +6019,7 @@ impl Interpreter { self.assoc_arrays = saved_assoc; self.aliases = saved_aliases; self.cwd = saved_cwd; + self.memory_budget = saved_memory_budget; self.counters.pop_function(); self.subst_generation += 1; let trimmed = stdout.trim_end_matches('\n'); diff --git a/crates/bashkit/src/limits.rs b/crates/bashkit/src/limits.rs index 251028c1..ff1a6a73 100644 --- a/crates/bashkit/src/limits.rs +++ b/crates/bashkit/src/limits.rs @@ -692,6 +692,40 @@ impl MemoryBudget { self.function_count = self.function_count.saturating_sub(1); self.function_body_bytes = self.function_body_bytes.saturating_sub(body_bytes); } + + /// Recompute budget from actual variable/array state. + /// + /// Used after `restore_shell_state` where the budget was not serialized + /// alongside the snapshot. `is_internal` should return true for variable + /// names that are internal markers (not user-visible). + pub fn recompute_from_state( + variables: &std::collections::HashMap, + arrays: &std::collections::HashMap>, + assoc_arrays: &std::collections::HashMap>, + function_count: usize, + function_body_bytes: usize, + is_internal: F, + ) -> Self + where + F: Fn(&str) -> bool, + { + let mut budget = Self::default(); + for (k, v) in variables { + if !is_internal(k) { + budget.variable_count += 1; + budget.variable_bytes += k.len() + v.len(); + } + } + for arr in arrays.values() { + budget.array_entries += arr.len(); + } + for arr in assoc_arrays.values() { + budget.array_entries += arr.len(); + } + budget.function_count = function_count; + budget.function_body_bytes = function_body_bytes; + budget + } } #[cfg(test)] diff --git a/crates/bashkit/tests/spec_cases/bash/memory_budget_desync.test.sh b/crates/bashkit/tests/spec_cases/bash/memory_budget_desync.test.sh new file mode 100644 index 00000000..2f948640 --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/memory_budget_desync.test.sh @@ -0,0 +1,43 @@ +# Memory budget desync after subshell/command-substitution state restoration +# Regression tests for issue #993 + +### budget_accurate_after_command_substitutions +# Memory budget should not inflate after many command substitutions. +# After 100 command substitutions that create variables internally, +# the parent shell should still be able to create variables. +for i in $(seq 1 100); do + x=$(echo val) +done +# If budget were inflated, this would silently fail +testvar="works" +echo "$testvar" +### expect +works +### end + +### budget_enforced_after_subshell +# Memory budget should remain accurate after subshell execution. +# Variables created in subshell should not affect parent budget. +( + for i in $(seq 1 50); do + eval "sub_v$i=$i" + done +) +# Parent should still be able to create variables +parentvar="ok" +echo "$parentvar" +### expect +ok +### end + +### subshell_vars_do_not_leak_budget +# Creating and destroying variables in subshells should not +# accumulate phantom budget entries. +for i in $(seq 1 200); do + (eval "tmp_$i=value") +done +result="success" +echo "$result" +### expect +success +### end