From 7bbf7115b91d567228146c83274581a49ca62d50 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:31:18 -0500 Subject: [PATCH 1/6] fix(interpreter): bound re-entrant signal-trap delivery depth Synchronous self-signal delivery (kill -> deliver -> run_trap_command inline) had no depth charge: direct, mutual, and mixed function/source recursion overflowed the native stack instead of failing cleanly. - Charge trap entries against the shared function/source hard ceiling and a dedicated trap-nesting cap (3) that trips first on small stacks - Exhaustion skips the nested handler with a diagnostic; the first trip surfaces as a resource-limit error at the exec boundary, while handler logic failures still never disturb the interrupted command (TM-DOS-030) - Box the handler and sourced-body execution edges (TM-DOS-089 pattern) - Flag depth trips from nested function/source pushes, which otherwise stop the recursion silently via status mapping - Regressions: in-process depth tests for all four shapes plus recovery, small-stack CLI subprocess probes, finite-nesting/exit/ignored specs --- crates/bashkit-cli/tests/cli_oneshot.rs | 42 +++++++ crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/mod.rs | 109 +++++++++++++++--- crates/bashkit/src/lib.rs | 10 ++ crates/bashkit/src/limits.rs | 13 ++- crates/bashkit/tests/integration/main.rs | 1 + .../integration/signal_trap_depth_tests.rs | 86 ++++++++++++++ .../tests/spec_cases/bash/kill-self.test.sh | 28 +++++ docs/security.md | 2 +- knowledge/foundations/architecture.md | 15 +++ knowledge/log.md | 1 + knowledge/operations/limitations.md | 2 +- knowledge/security/threat-model.md | 2 +- 13 files changed, 290 insertions(+), 23 deletions(-) create mode 100644 crates/bashkit/tests/integration/signal_trap_depth_tests.rs diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index 3e066656c..36a798994 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -148,6 +148,48 @@ fn function_recursion_is_bounded_on_a_two_mib_stack() { } } +// Child isolation and a small command allowance keep even a missing guard safe. +#[cfg(unix)] +#[test] +fn signal_trap_recursion_is_bounded_on_a_two_mib_stack() { + for script in [ + "trap 'kill -USR1 $$' USR1; kill -USR1 $$", + "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + ] { + let mut child = Command::new("sh") + .args([ + "-c", + "set -e; ulimit -s 2048; exec \"$1\" --no-stdin --max-commands 64 -c \"$2\"", + "sh", + BIN, + script, + ]) + .env_remove("RUST_BACKTRACE") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn signal trap probe"); + let deadline = Instant::now() + Duration::from_secs(10); + while child.try_wait().unwrap().is_none() { + if Instant::now() >= deadline { + child.kill().unwrap(); + let out = child.wait_with_output().unwrap(); + panic!("signal trap probe hung: {script}: {}", stderr(&out)); + } + thread::sleep(Duration::from_millis(10)); + } + let out = child.wait_with_output().unwrap(); + assert_eq!(out.status.code(), Some(1), "{script}: {}", stderr(&out)); + assert!( + stderr(&out).contains("depth exceeded"), + "{script}: {}", + stderr(&out) + ); + } +} + #[cfg(unix)] #[test] fn function_nesting_below_limit_works_on_a_two_mib_stack() { diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index bf8c23d3e..b2ac83275 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -88,7 +88,7 @@ through configurable limits. | Threat | Attack Example | Mitigation | Status | |--------|---------------|------------|--------| -| Function recursion (TM-DOS-020) | `f() { f; }; f` | Depth limit of 16, including a hard ceiling when callers request more, protects 2 MiB host stacks | MITIGATED | +| Function and signal-trap recursion (TM-DOS-020) | `f() { f; }; f` or `trap 'kill -USR1 $$' USR1; kill -USR1 $$` | Functions, source, child shells and trap handlers share a hard depth ceiling of 16, plus a dedicated trap-nesting cap of 3 that trips first on small stacks; nested exhaustion is skipped with a diagnostic and the first trip fails the exec as a resource-limit error, while handler logic failures never disturb the interrupted command (TM-DOS-030). Subprocess tests cover direct, mutual and mixed recursion on 2 MiB stacks | MITIGATED | | Command sub depth (TM-DOS-021) | `$($($($())))` nesting | Inherited depth/fuel from parent | MITIGATED | | Parser depth (TM-DOS-022) | `(((((...))))))` nesting | `max_ast_depth` + hard cap (100) | MITIGATED | | Arithmetic depth (TM-DOS-026) | `$(((((...))))))` | `MAX_ARITHMETIC_DEPTH` (50) | MITIGATED | diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index cc41fd5e6..dd4b627c0 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -30,6 +30,8 @@ // sandbox "Did you mean" / unavailable-command hints after // `command not found` are kept on purpose. +// Decision: trap handlers share the function/source depth budget. Signal delivery +// propagates handler failures; swallowing a nested limit error reports success. mod arithmetic; mod brace_expansion; mod completion; @@ -87,7 +89,10 @@ use crate::builtins::{self, Builtin}; use crate::error::Error; use crate::error::Result; use crate::fs::FileSystem; -use crate::limits::{BudgetedString, ExecutionCounters, ExecutionLimits, SessionLimits}; +use crate::limits::{ + BudgetedString, ExecutionCounters, ExecutionLimits, HARD_MAX_TRAP_NESTING, LimitExceeded, + SessionLimits, +}; /// A single command history entry. #[derive(Debug, Clone)] @@ -1925,6 +1930,10 @@ pub struct Interpreter { /// True while executing a trap handler. Suppresses recursive DEBUG trap /// invocation to prevent amplification attacks (TM-DOS-035). in_trap: bool, + /// Nesting level of `run_trap_command` entries (TM-DOS-020). + trap_nesting: usize, + /// First trap-nesting depth error, surfaced at the `exec` boundary. + trap_depth_error: Option, /// The ERR trap is inherited for display only: a subshell, command /// substitution or pipeline stage without `set -E` keeps the parent's /// handler visible to `trap -p` but does not run it (bash). `trap` @@ -2562,6 +2571,8 @@ impl Interpreter { cancelled: Arc::new(AtomicBool::new(false)), hooks: Arc::new(crate::hooks::Hooks::default()), in_trap: false, + trap_nesting: 0, + trap_depth_error: None, err_trap_dormant: false, debug_trap_dormant: false, err_trap_skip_stage: false, @@ -2782,6 +2793,8 @@ impl Interpreter { cancelled: Arc::clone(&self.cancelled), hooks: Arc::clone(&self.hooks), in_trap: false, + trap_nesting: 0, + trap_depth_error: None, // A job or pipeline stage is a subshell: ERR stays dormant there // unless `set -E`. err_trap_dormant: self.err_trap_dormant || !self.flags.contains(BashFlags::ERRTRACE), @@ -3101,6 +3114,8 @@ impl Interpreter { // handler is awaited; clear the re-entrancy guard before each exec so // one cancelled script cannot suppress traps in the next script. self.in_trap = false; + self.trap_nesting = 0; + self.trap_depth_error = None; self.err_trap_dormant = false; self.debug_trap_dormant = false; self.err_trap_skip_stage = false; @@ -9985,7 +10000,7 @@ impl Interpreter { .unwrap_or(u64::MAX), )?; - self.apply_builtin_side_effects(&mut result).await; + self.apply_builtin_side_effects(&mut result).await?; let result = self.apply_redirections(result, redirects).await?; self.apply_after_tool(name, result) @@ -10885,12 +10900,18 @@ impl Interpreter { }; // THREAT[TM-DOS-056]: Check source depth (uses function depth limit) - self.counters.push_function(&self.limits).map_err(|_| { - crate::error::Error::Execution(format!( + if let Err(depth_error) = self.counters.push_function(&self.limits) { + // THREAT[TM-DOS-020]: a depth trip inside a trap handler must + // still fail the execution instead of silently stopping the + // recursion (the `source` status mapping below would hide it). + if self.in_trap { + self.note_trap_depth_error(depth_error); + } + return Err(crate::error::Error::Execution(format!( "source: {}: maximum source depth exceeded", filename - )) - })?; + ))); + } // Track source file for BASH_SOURCE self.bash_source_stack.push(SourceFrame { @@ -10909,7 +10930,10 @@ impl Interpreter { self.xtrace_depth += 1; let tempenv_pushed = self.push_pending_tempenv(false); let region = self.enter_output_region(redirects); - let mut exec_result = self.execute_script_body(&script, false, true).await; + // THREAT[TM-DOS-020]: Box the sourced-body execution so a source / + // trap recursion cycle cannot grow the parent future with each + // nesting level; the shared depth charge above then trips first. + let mut exec_result = Box::pin(self.execute_script_body(&script, false, true)).await; self.leave_output_region(region); if tempenv_pushed { self.tempenv_frames.pop(); @@ -12354,7 +12378,7 @@ impl Interpreter { } /// Process structured side effects from builtin execution. - async fn apply_builtin_side_effects(&mut self, result: &mut ExecResult) { + async fn apply_builtin_side_effects(&mut self, result: &mut ExecResult) -> Result<()> { // Builtins that mutate SHOPT_* directly via `ctx.variables` (e.g. the // `set -e` / `set +u` paths in the `set` builtin) don't update the // cached `flags` bitfield. Resync once after every builtin so the @@ -12466,6 +12490,7 @@ impl Interpreter { if let Some(signal) = pending_self_signal { self.deliver_self_signal(signal, result).await; } + Ok(()) } /// `kill -SIG $$`: bash runs that signal's trap if one is set, and @@ -12473,11 +12498,12 @@ impl Interpreter { /// 128 + signal. Signals whose default is to be ignored do neither. async fn deliver_self_signal(&mut self, signal: i32, result: &mut ExecResult) { if let Some(trap_cmd) = self.signal_trap(signal) { + if trap_cmd.is_empty() { + return; + } let mut stdout = std::mem::take(&mut result.stdout); let mut stderr = std::mem::take(&mut result.stderr); - let flow = self - .run_trap_command(&trap_cmd, &mut stdout, &mut stderr) - .await; + let flow = Box::pin(self.run_trap_command(&trap_cmd, &mut stdout, &mut stderr)).await; result.stdout = stdout; result.stderr = stderr; // An `exit` inside the handler ends the script, as it does in bash. @@ -12498,6 +12524,7 @@ impl Interpreter { self.last_exit_code = status; result.exit_code = status; result.control_flow = ControlFlow::Exit(status); + () } /// Resolve a path relative to cwd, normalizing `.` and `..` components. @@ -14481,7 +14508,21 @@ impl Interpreter { /// Run a trap handler's command, merging its output into the result the /// caller is building. - /// Returns the handler's own control flow (an `exit` inside it), if any. + /// Returns the handler's control flow and status, or `None` when the + /// handler cannot run (unparsable command, exhausted depth budget, or a + /// failing handler): trap failures never disturb the interrupted command. + /// Record the first trap-nesting depth error for the `exec` boundary. + fn note_trap_depth_error(&mut self, error: LimitExceeded) { + if self.trap_depth_error.is_none() { + self.trap_depth_error = Some(error); + } + } + + /// Take the recorded trap-nesting depth error, if any. + pub(crate) fn take_trap_depth_error(&mut self) -> Option { + self.trap_depth_error.take() + } + async fn run_trap_command( &mut self, trap_cmd: &str, @@ -14492,6 +14533,26 @@ impl Interpreter { let Ok(trap_script) = self.parse_shell_text(trap_cmd) else { return None; }; + // THREAT[TM-DOS-020]: Bound nested trap delivery with a dedicated + // hard cap that trips before native frames can exhaust small host + // stacks, independent of command fuel, per-handler AST depth, and + // the shared function/source budget (which is charged too, so a low + // `max_function_depth` still stops function/signal mixes early). + // Exhaustion skips the nested handler with a diagnostic; the first + // trip is surfaced as a resource-limit error at the `exec` + // boundary, and never disturbs the interrupted command. + if self.trap_nesting >= HARD_MAX_TRAP_NESTING { + self.note_trap_depth_error(LimitExceeded::MaxTrapDepth(HARD_MAX_TRAP_NESTING)); + stderr.push_str("bashkit: maximum trap nesting depth exceeded\n"); + return None; + } + self.trap_nesting += 1; + if let Err(depth_error) = self.counters.push_function(&self.limits) { + self.trap_nesting -= 1; + self.note_trap_depth_error(depth_error); + stderr.push_str("bashkit: maximum trap nesting depth exceeded\n"); + return None; + } let was_in_trap = self.in_trap; self.in_trap = true; let saved_line_base = std::mem::replace( @@ -14500,12 +14561,28 @@ impl Interpreter { ); let emit_before = self.output_emit_count; self.xtrace_depth += 1; - let result = self.execute_command_sequence(&trap_script.commands).await; + // THREAT[TM-DOS-020]: Box the handler execution so a trap / + // source recursion cycle cannot grow the parent future with each + // nesting level; the shared depth charge above then trips first. + let result = Box::pin(self.execute_command_sequence(&trap_script.commands)).await; self.xtrace_depth -= 1; self.line_base = saved_line_base; self.in_trap = was_in_trap; - let Ok(trap_result) = result else { - return None; + self.trap_nesting -= 1; + self.counters.pop_function(); + // THREAT[TM-DOS-030]: A failing handler must not disturb the + // interrupted command, so execution errors end the handler here. + // THREAT[TM-DOS-020]: resource exhaustion inside the handler + // (a nested function/source call tripping depth or fuel) is not a + // handler logic failure: record it so the `exec` boundary still + // fails instead of silently stopping the recursion. + let trap_result = match result { + Ok(trap_result) => trap_result, + Err(crate::error::Error::ResourceLimit(limit)) => { + self.note_trap_depth_error(limit); + return None; + } + Err(_) => return None, }; self.maybe_emit_output(&trap_result.stdout, &trap_result.stderr, emit_before); stdout.append(&trap_result.stdout); @@ -14591,7 +14668,7 @@ impl Interpreter { // The handler runs with `$?` of the trapping command and the // shell keeps that status afterwards (bash restores it). let saved_exit = self.last_exit_code; - let _ = self.run_trap_command(&trap_cmd, stdout, stderr).await; + let _ = Box::pin(self.run_trap_command(&trap_cmd, stdout, stderr)).await; self.last_exit_code = saved_exit; } } diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index c7bef27da..37bb85093 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -970,6 +970,16 @@ impl Bash { if let Ok(exec_result) = &mut result { exec_result.capability_cleanup = cleanup; } + // THREAT[TM-DOS-020]: trap-nesting exhaustion skips the nested + // handler without disturbing the interrupted command, but resource + // exhaustion still fails the execution, like command fuel. + if let Some(depth_error) = self + .interpreter + .take_trap_depth_error() + .filter(|_| result.is_ok()) + { + result = Err(Error::ResourceLimit(depth_error)); + } result } diff --git a/crates/bashkit/src/limits.rs b/crates/bashkit/src/limits.rs index d368c00c8..0b3d76aa2 100644 --- a/crates/bashkit/src/limits.rs +++ b/crates/bashkit/src/limits.rs @@ -36,6 +36,11 @@ use crate::time_compat::Instant; // embedders request a higher logical recursion limit. const HARD_MAX_FUNCTION_DEPTH: usize = 16; +// Self-delivered trap handlers re-enter the interpreter synchronously, so a +// small dedicated nesting cap trips before native frames can exhaust small +// host stacks, independent of the shared function/source budget below. +pub(crate) const HARD_MAX_TRAP_NESTING: usize = 3; + // Prompt reparses poll recursively on the host stack, even with shallow ASTs. // THREAT[TM-DOS-133]: No configurable AST limit may remove this stack ceiling. const HARD_MAX_PROMPT_DEPTH: usize = 8; @@ -76,7 +81,7 @@ pub struct ExecutionLimits { /// Default: 1,000,000 pub max_total_loop_iterations: usize, - /// Maximum function call depth (recursion limit) + /// Maximum shared function, source, child-shell, and trap-handler depth /// Default: 16; values above 16 are capped for host stack safety. pub max_function_depth: usize, @@ -272,8 +277,8 @@ impl ExecutionLimits { self } - /// Set maximum function depth. - /// A value of 0 is a valid strict limit that prevents function calls. + /// Set the shared function, source, child-shell, and trap-handler depth. + /// A value of 0 prevents entry into any of these execution frames. pub fn max_function_depth(mut self, depth: usize) -> Self { self.max_function_depth = depth; self @@ -735,6 +740,8 @@ pub enum LimitExceeded { #[error("maximum prompt expansion depth exceeded ({0})")] MaxPromptDepth(usize), + #[error("maximum trap nesting depth exceeded ({0})")] + MaxTrapDepth(usize), #[error("maximum file descriptors exceeded ({0})")] MaxFileDescriptors(usize), diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index 28019d11d..ac5d7a177 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -138,6 +138,7 @@ mod sed_resource_tests; pub mod set_e_and_or_tests; pub mod shlvl_tests; pub mod shuf_resource_tests; +pub mod signal_trap_depth_tests; pub mod skills_tests; pub mod snapshot_fixture_tests; pub mod snapshot_history_tests; diff --git a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs new file mode 100644 index 000000000..5d09af2b2 --- /dev/null +++ b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs @@ -0,0 +1,86 @@ +//! Signal handlers share the function/source depth ceiling (TM-DOS-020). +//! Small command budgets keep pre-fix recursion probes safe. + +use bashkit::{Bash, ExecutionLimits}; + +#[tokio::test] +async fn recursive_signal_traps_share_function_depth() { + for script in [ + "trap 'kill -USR1 $$' USR1; kill -USR1 $$", + "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + ] { + for depth in [2, 3] { + let mut bash = Bash::builder() + .limits( + ExecutionLimits::new() + .max_function_depth(depth) + .max_commands(12), + ) + .build(); + let err = bash.exec(script).await.unwrap_err().to_string(); + assert!(err.contains("depth exceeded"), "{script}: {err}"); + assert!(!err.contains("command limit"), "{script}: {err}"); + let next = bash.exec("echo recovered").await.unwrap(); + assert_eq!(next.stdout, "recovered\n"); + assert_eq!(next.exit_code, 0); + } + } +} + +#[tokio::test] +async fn signal_depth_is_released_after_each_handler() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(2)) + .build(); + let result = bash.exec("trap 'echo first; kill -USR2 $$; echo last' USR1; trap 'echo second' USR2; kill -USR1 $$; kill -USR1 $$; echo done").await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!( + result.stdout, + "first\nsecond\nlast\nfirst\nsecond\nlast\ndone\n" + ); + assert!(result.stderr.is_empty(), "{}", result.stderr); +} + +#[tokio::test] +async fn ignored_signal_needs_no_handler_depth() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(0)) + .build(); + let result = bash + .exec("trap '' USR1; kill -USR1 $$; echo ignored") + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, "ignored\n"); +} + +#[tokio::test] +async fn signal_handler_command_limit_reaches_host() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_commands(3)) + .build(); + let err = bash + .exec("trap 'echo first; echo second' USR1; kill -USR1 $$") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("maximum command count exceeded"), "{err}"); +} + +#[tokio::test] +async fn signal_handler_parse_error_is_swallowed() { + // TM-DOS-030 (merged main semantics): an unparsable handler never + // disturbs the interrupted command; the kill completes and the shell + // stays usable. + let mut bash = Bash::builder().build(); + let result = bash + .exec("trap 'if' USR1; kill -USR1 $$; echo done") + .await + .unwrap(); + assert_eq!(result.stdout, "done\n"); + assert_eq!(result.exit_code, 0); + let next = bash.exec("echo recovered").await.unwrap(); + assert_eq!(next.stdout, "recovered\n"); +} diff --git a/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh b/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh index f54d81952..d644bdeac 100644 --- a/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh @@ -38,3 +38,31 @@ echo alive ### expect alive ### end + +### nested_self_signals_return_to_each_handler +trap 'echo first; kill -USR2 $$; echo last' USR1 +trap 'echo second' USR2 +kill -USR1 $$ +kill -USR1 $$ +echo done +### expect +first +second +last +first +second +last +done +### end + +### ignored_self_signal_and_handler_exit +trap 'echo ignored' USR1 +trap '' USR1 +kill -USR1 $$ +trap 'echo caught; exit 7' USR2 +kill -USR2 $$ +echo unreachable +### expect +caught +### exit_code: 7 +### end diff --git a/docs/security.md b/docs/security.md index dee6fb826..d1dbbff03 100644 --- a/docs/security.md +++ b/docs/security.md @@ -101,7 +101,7 @@ Bashkit intentionally deviates from POSIX where compliance would compromise the sandbox. Key exclusions: - **`exec`**: would break sandbox containment (`TM-ESC-005`) -- **`trap`**: conflicts with the stateless execution model +- **`trap`**: handles shell events and self-sent virtual signals. Nested handlers share the function/source/child-shell depth budget, capped at 16; recursive signal delivery returns a bounded error - **Real process spawning**: all subprocess commands stay within the virtual interpreter (`TM-ESC-015`) These decisions are documented in [`knowledge/operations/limitations.md`](../knowledge/operations/limitations.md). diff --git a/knowledge/foundations/architecture.md b/knowledge/foundations/architecture.md index b6d2bb4c2..77f85aca2 100644 --- a/knowledge/foundations/architecture.md +++ b/knowledge/foundations/architecture.md @@ -103,6 +103,21 @@ Recursive bare-variable evaluation and syntax parsing share the depth counter. Expansion errors propagate to the arithmetic boundary; later host executions receive fresh state. Regression and Bash differential coverage: `crates/bashkit/tests/integration/arithmetic_resource_tests.rs` (TM-DOS-026). +### Trap nesting + +Self-sent signals run handlers inline. Every handler entered through +`run_trap_command` charges the existing function-depth counter, also used by +functions, source, and child shells; its hard ceiling is 16. This preserves +finite nested signal delivery while bounding direct, mutual, and mixed cycles +without relying on command fuel or per-handler AST depth. The signal path boxes +the handler future to keep each native frame small; when the budget is +exhausted the nested handler is skipped with a diagnostic on the handler +stderr, and unparsable or failing handlers never disturb the interrupted +command (TM-DOS-030). Trap state and depth are restored on every path. DEBUG, +ERR, and RETURN retain their reentrancy suppression and existing error +handling. +See [Threat Model](../security/threat-model.md), TM-DOS-020, and +`signal_trap_depth_tests` plus CLI small-stack regression coverage. ### Shared execution budget diff --git a/knowledge/log.md b/knowledge/log.md index d350026fc..72dc1f1e7 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -19,6 +19,7 @@ finite nested expansions and literal `promptvars`-off behavior. Regression coverage includes cycles, substitutions, sibling budgets and session recovery; see [interactive shell](integrations/interactive-shell.md). +* **Fix**: self-signal trap handlers share the hard function/source depth ceiling (TM-DOS-020). Recursive direct/mutual/mixed delivery stops at the ceiling with a diagnostic instead of overflowing the stack, while handler failures still never disturb the interrupted command (TM-DOS-030). Bounded direct/mutual/mixed regressions and small-stack CLI subprocess probes cover the boundary. See [architecture](foundations/architecture.md#trap-nesting). * **Fix**: Oils long tail. `test`/`[` follow bash's `test.c` grammar (argument-count rules, `-a`/`-o`/`!`/`( )` precedence, `argument expected`, `)' expected`, `too many arguments`). `$empty cmd` runs `cmd`; `PWD`/`OLDPWD` are exported; `B=(b b) cmd` binds the text `(b b)` for `cmd` only. `rm` reports each bad operand and goes on; `cat` reads buffered stdin once; `date foo` fails; `umask 1 2` uses the first operand; `history -5` is an invalid option; `help help`, `help -- X` and bash's `no help topics match`; `set +` is ignored; `getopts` rejects a bad variable name and unsets `OPTARG` at the end. `shift 1 2` (like `exit`/`break`/`continue` with extra operands) discards the line, and a builtin's discard ends a child `bash -c` string (status 1) while expansion errors still resume. Arithmetic: `set -u` makes an unbound name fatal (also in `${#v}`/`${v:..}`), backticks and `\`-newline work inside, a base with a leading zero (`010#1`) is invalid, `a[$(cmd)]=v` and `${a[$(cmd)]}` run the substitution, `a[]=x` and `${a[]}` are bad subscripts. Array literals evaluate subscripts in order (`[i++]=x`, self references), skip tilde for assoc values, and refuse bare assoc elements; `a["k"]=v` and nameref targets `A["K"]`/`A[$key]` resolve quoted and variable keys; `declare -nx` exports. `printf %q` and `${x@Q}`/`@A` use `$'...'` for control characters; `$'\cX'` and invalid `\u` match bash; `${s:}` is a bad substitution; quotes nested in a double-quoted `${..}` operand stay protected; `{a,b}""` keeps brace expansion; `f() ...` with `$`/quotes in the name and `cmd (` are refused. `unset PS4` traces with no prefix; `time a | b` keeps its output. Oils spec pass rate 92.8% to 95.8% (79 cases, no new misses; `array-literal`, `array`, `blog2`, `bool-parse`, `builtin-misc`, `func-parsing`, `builtin-getopts`, `builtin-set`, `exit-status`, `var-op-slice` to 0); every remaining miss is categorized under limitations "Oils Spec Misses". * **Fix**: interactive builtins. `history` and `fc` keep bash's per-shell history: one entry per top-level input line (with `set -o history` / `bash -i`), `history -c -d N/-N/A-B -a -n -r -w -p -s`, `HISTSIZE`/`HISTFILESIZE` trims, `histappend`, listings in bash's format; `fc -l/-n/-r` ranges and prefixes, `fc -s old=new cmd` and `fc -e -` rerun (no editor, L-HIST-001). `HISTFILE` is read and written through the VFS only, reads are capped at `max_input_bytes` (TM-DOS-131); `history -c` still persists at once for the builder's history file. `complete`/`compgen`/`compopt` keep per-interpreter specs (at most 1024 specs and 64 KiB each, TM-DOS-132): every `-A` action, `-W` split at IFS with `$var` expansion, `-G`, `-F` (with `COMP_*` set and `COMPREPLY` read), `-C`, `-X` with `!` and `&`, `-P`/`-S`, `-o plusdirs/dirnames/default`, `complete -p`/`-r` in bash's format. `bind` lists bash's default emacs bindings, readline functions and variables and records changes per shell (capped, TM-DOS-132). `${x@P}` decodes prompt escapes (`\u \h \w \W \s \v \! \# \t \D{fmt}`, octal, `\[ \]`) from the sandbox identity and virtual clock, then expands like a here-doc body under `promptvars`. `bash -i` runs an interactive child: `$-` has `i`, `PS1`/`PS2`/`PS4`/`HISTFILE` defaults, `~/.bashrc` or `--rcfile`, `--norc`, and for a stdin script `PS1` printed to stderr plus `PROMPT_COMMAND` per line and `exit` at EOF. `hostname` takes `-s`/`-f`/`-d`. Oils spec pass rate 89.0% to 92.8% (`builtin-completion` 33 misses to 2, `prompt` 25 to 0, `builtin-history` and `builtin-fc` 12 to 0, `interactive` 11 to 0, `builtin-bind` 9 to 0); gaps in the limitations "Interactive builtins" row and L-HIST-001. diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index a6ff7f1e9..3e28a3df1 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -60,7 +60,7 @@ execution model. Evidence is a threat-model ID, a test, or `stance` | L-NET-002 | No DNS resolution; hosts must appear in the allowlist | Resolution would bypass allowlist intent | `l_net_002_default_deny_no_resolution` | | L-ARITH-001 | A variable's value read as an arithmetic expression or subscript never runs command substitution: `x='a[$(cmd)]'; echo $((x))` and `unset "a[$(cmd)]"` built from data are arithmetic syntax errors, and an indirect target holding `$(`, `` ` ``, `<(` or `>(` (`r='a[$(cmd)]'; ${!r}`) is `invalid variable name`, where bash runs `cmd` | Values are data; re-parsing them as code is the classic bash arithmetic injection, and a sandboxed agent shell must not execute text it only read | stance | | L-ARITH-002 | An arithmetic error names at most 256 bytes each of the expression and the "error token" (the unparsed rest), with a `...` marker where either was cut. An expression whose first rejected character sits near the front is echoed back twice over -- once whole, once from that character on -- so a ~500-byte one reported ~1,000 bytes before this cap, as bash still does. Expressions at or under the cap are byte-identical to bash | Both fragments come from the script, so one expression under `MAX_ARITHMETIC_EXPANSION_BYTES` rendered a diagnostic about twice its size, over the 1 KiB stderr budget every builtin is held to (TM-INF-022). Capping each fragment keeps the text that says what went wrong | TM-INF-022, `long_arithmetic_expansion_diagnostic_is_bounded`, `run_270_crash_input_is_bounded` | -| L-SIG-001 | No signal arrives from outside the sandbox, so a handler only runs for a signal the script sends itself (`kill -SIG $$`, plus EXIT, ERR and DEBUG). Without a handler, a self-sent signal ends the script with 128 + signal | No host signals exist inside the sandbox | `l_sig_001_signal_traps_not_delivered` | +| L-SIG-001 | No signal arrives from outside the sandbox, so a handler only runs for a signal the script sends itself (`kill -SIG $$`, plus EXIT, ERR and DEBUG). Without a handler, a self-sent signal ends the script with 128 + signal. Nested handlers share `max_function_depth` with functions/source/child shells (hard ceiling 16); recursive delivery past the ceiling is skipped with a diagnostic on the handler's stderr and the first trip fails the exec with a resource-limit error | No host signals exist inside the sandbox | `l_sig_001_signal_traps_not_delivered` | | L-SSH-001 | CA-signed SSH host *certificates* are rejected under `strict_host_key_checking` (default), even when the public key they wrap is a configured trusted key; configure the host's public key directly | No CA trust store exists to validate the signature chain, validity window, principals or critical options; matching the embedded key would grant trust never actually verified | TM-SSH-006, `test_strict_rejects_certificate_even_when_inner_key_is_trusted` | | L-WASM-001 | **Removed:** JS-host timers now drive `sleep`, builtin `timeout`, execution limits, and tool `timeoutMs` | `gloo-timers` bridges the host event-loop clock without threads or cross-origin isolation | [Browser Package](../runtimes/browser-package.md) | | L-WASM-002 | Browser build: `executeSync()` cannot run async custom builtins (fails with a clear message); use `execute()` | Single-threaded event loop can't settle a JS `Promise` without yielding | `crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs` | diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index f09fb23da..1da290062 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -237,7 +237,7 @@ runaway scripts without permanently breaking the session. | ID | Threat | Attack Vector | Mitigation | Status | |----|--------|--------------|------------|--------| -| TM-DOS-020 | Function recursion | `f() { f; }; f` | Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). The CLI subprocess test runs direct and mutual recursion on that stack size. | **MITIGATED** | +| TM-DOS-020 | Function and signal-trap recursion | `f() { f; }; f` or `trap 'kill -USR1 $$' USR1; kill -USR1 $$` | Signal handlers, functions, source, and child shells share the function-depth counter, plus a dedicated trap-nesting cap of 3 that trips first on small stacks; direct, mutual, and mixed signal recursion cannot refresh either. Nested-trap exhaustion is skipped with a diagnostic and the first trip fails the exec as a resource-limit error, while handler logic failures never disturb the interrupted command (TM-DOS-030). Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). CLI subprocess tests run direct, mutual, and mixed signal/function/source recursion on that stack size with small command budgets. | **MITIGATED** | | TM-DOS-021 | Command sub nesting | `$($($($())))` | Child parsers inherit remaining depth budget + fuel from parent | **MITIGATED** | | TM-DOS-022 | Parser recursion | Deeply nested `(((())))` | `max_ast_depth` limit (100) + `HARD_MAX_AST_DEPTH` cap (100) | **MITIGATED** | | TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))`, nested `${a[${a[...]}]}` passed to `let` | `MAX_ARITHMETIC_DEPTH` limit (50) checked before dollar expansion and tokenization; shared fuel across textual expansion, nested read-only subscripts, nameref/key lookup, and recursive variable evaluation; expanded-size cap | **MITIGATED** | From c8572143c95b176574a706fcda6ec1ee143c7796 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:55:29 -0500 Subject: [PATCH 2/6] fix(interpreter): drop redundant unit tail flagged by all-features clippy --- crates/bashkit/src/interpreter/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index dd4b627c0..2767fc7e8 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -12524,7 +12524,6 @@ impl Interpreter { self.last_exit_code = status; result.exit_code = status; result.control_flow = ControlFlow::Exit(status); - () } /// Resolve a path relative to cwd, normalizing `.` and `..` components. From 693419934467f68153224387e9706b7002d492dc Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 02:22:21 -0500 Subject: [PATCH 3/6] fix(interp): charge tilde expansion to the execution budget (#2672) ## What changed Tilde expansion (`~user`, `x=~`, `x=~:~`, `${u-~}`) now charges the shared execution budget. Previously `tilde_expand_literal` built results with an unbudgeted `String`, so a script stacking tildes could grow output without ever tripping budget limits. ## Why Execution-budget E5 requires every interpreter allocation site to charge budget. Unbudgeted tilde expansion is a resource-exhaustion hole (TM-DOS-134): each `:` operand re-resolves the home directory for free. ## Before / After New regression tests fail on the old code (5 of 7 fail without the fix; the 2 parity tests pass both ways) and pass with it: - `cargo test -p bashkit --test integration execution_budget`: 32/32 pass - CLI smoke: `echo ~; x=~; echo "$x"; echo ~root; echo ~/sub; v=${u-~}; echo "$v"` expands correctly (sandbox homes by design) - Differential check: same script under real bash expands identically in structure ## Risk - Low. Fail-closed accounting only; expansion results unchanged (covered by a bash-parity test). No API changes outside the interpreter. ## Checklist - [x] Tests added or updated (7 tilde budget tests: exhaustion, boundary, leases, parity) - [x] Backward compatibility considered (internal code; no compat needed) --- crates/bashkit/docs/threat-model.md | 1 + crates/bashkit/src/interpreter/declare.rs | 3 +- crates/bashkit/src/interpreter/expansion.rs | 204 ++++++++++++++---- crates/bashkit/src/interpreter/mod.rs | 2 +- .../integration/execution_budget_tests.rs | 120 +++++++++++ docs/security.md | 6 + knowledge/foundations/architecture.md | 9 + knowledge/security/threat-model.md | 1 + 8 files changed, 298 insertions(+), 48 deletions(-) diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index bf8c23d3e..21995b7bd 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -184,6 +184,7 @@ let bash = Bash::builder() | Persistent history memory DoS (TM-DOS-094) | Unbounded command history in long-lived instances | `ExecutionLimits` caps history entries/bytes/output | FIXED | | Multi-component glob amplification (TM-DOS-095) | `/*/*/*/*` multiplies the candidate set at each component | Reject patterns deeper than `max_path_depth`; cap live candidates at `max_file_count` | FIXED | | Aggregate budget refresh (TM-DOS-096) | Nest/mix parsers, pipelines, traversal, runtimes, archives, and callbacks to restart local ceilings | One poisoned request-scoped `ExecutionBudget` meters aggregate work/input/live bytes without replacing subsystem caps | MITIGATED | +| Tilde expansion amplification (TM-DOS-134) | Large `HOME`/`PWD`/`OLDPWD` repeated after assignment colons | Borrowed directories, checked pre-allocation live-byte reservations, byte-proportional work/deadline checks, and leases retained through assignment/field consumption; failures abort expansion | MITIGATED | | Contradictory execution-profile limits (TM-DOS-097) | Host config silently requests ineffective or impossible limits | Validate profile cross-field invariants before `BashBuilder` accepts it | MITIGATED | | Suspended host-call retention (TM-DOS-098) | Script repeats event-backed calls or host never resumes one | Capacity-one channel; an independently driven execution future keeps the deadline armed while parked and drops the timed-out session without another host poll; handle drop aborts the driver | MITIGATED | | `time` report amplification (TM-DOS-099) | Attacker-controlled `-f` format expands repeatedly or targets the VFS with `-o` | Incremental rendering is capped by the stderr limit before emission or file replacement | MITIGATED | diff --git a/crates/bashkit/src/interpreter/declare.rs b/crates/bashkit/src/interpreter/declare.rs index a131b2994..75722fae0 100644 --- a/crates/bashkit/src/interpreter/declare.rs +++ b/crates/bashkit/src/interpreter/declare.rs @@ -1441,7 +1441,8 @@ impl Interpreter { if let Some((kw, vw, kappend)) = split_keyed_word(word).filter(|_| !braces) { let key_text = self.expand_array_word(&kw).await?; // `[k]=~:~` tilde-expands like an assignment value. - let vw = self.tilde_assignment_value(&vw).into_owned(); + // `[k]=~:~` tilde-expands like an assignment value. + let (vw, _tilde_leases) = self.tilde_assignment_value(&vw)?; let val = self.expand_array_word(&vw).await?; items.try_push(Item::Keyed(key_text, val, kappend))?; continue; diff --git a/crates/bashkit/src/interpreter/expansion.rs b/crates/bashkit/src/interpreter/expansion.rs index bb01309f4..23fdb1208 100644 --- a/crates/bashkit/src/interpreter/expansion.rs +++ b/crates/bashkit/src/interpreter/expansion.rs @@ -21,6 +21,8 @@ //! another part (`~"x"`, `~$v`) is not a tilde prefix. Assignment values //! are pre-resolved by `tilde_assignment_value` (after `=` and each `:`); //! `${...}` operands use the `operand_tilde` flag, set per operator. +//! Tilde directories are borrowed; expanded buffers reserve capacity before +//! allocation and keep their leases through preprocessing and word consumption. use super::*; @@ -427,15 +429,13 @@ impl Interpreter { result.try_push_str(&text)?; } WordPart::Literal(s) => { - // Tilde expansion (`~`, `~/x`, `x=~:~`); quoted literals - // keep their tilde. - match (!part_quoted && s.contains('~')) - .then(|| self.tilde_expand_word_literal(word, idx, s)) - .flatten() - { - Some(expanded) => result.try_push_str(&expanded)?, - None => result.try_push_str(s)?, - } + self.append_literal_segment( + &mut result, + (word, idx, s), + part_quoted, + leased, + &mut substitution_leases, + )?; } WordPart::Variable(name) => { if self.is_nounset() && !self.is_variable_set(name) { @@ -762,6 +762,7 @@ impl Interpreter { } } + self.check_cancelled()?; Ok(result) } @@ -901,26 +902,68 @@ impl Interpreter { /// Directory for the tilde prefix `~name`: `~` is `$HOME`, `~+` `$PWD`, /// `~-` `$OLDPWD`; `~root` and `~` are the fixed homes. /// `None` keeps the text literal (unknown user, unset `OLDPWD`). - pub(super) fn tilde_dir(&self, prefix: &str) -> Option { + /// Append one literal segment, tilde-expanding when unquoted and eligible. + /// Tilde expansion (`~`, `~/x`, `x=~:~`) runs during substitution: `~` + /// only expands at the start of the word or after `:`/`=` in an + /// assignment-like prefix. Quoted literals keep their tilde. + /// + /// Split out of `expand_word_buffer` so the per-level poll frame holds no + /// `BudgetedString` temporaries: every `$(...)` nesting level awaits + /// through that future, and heavy arm locals abort depth-32 under + /// coverage instrumentation (cf. #1089). + fn append_literal_segment( + &self, + result: &mut BudgetedString, + segment: (&Word, usize, &str), + part_quoted: bool, + leased: bool, + substitution_leases: &mut Vec, + ) -> std::result::Result<(), crate::limits::LimitExceeded> { + // Small error type on purpose: every fallible op in here already + // reports LimitExceeded, and a lean Result keeps the caller's hot + // poll frame small. The caller converts to the shared Error once, + // outside this path (same Error::from(LimitExceeded) as `?` uses). + let (word, idx, s) = segment; + let expanded = if !part_quoted && s.contains('~') { + self.tilde_expand_word_literal(word, idx, s)? + } else { + None + }; + if let Some(expanded) = expanded { + let tilde_lease = self.execution_budget.lease_bytes(expanded.len())?; + result.try_push_str(&expanded)?; + if !leased { + substitution_leases.push(tilde_lease); + } + } else { + result.try_push_str(s)?; + } + Ok(()) + } + + fn tilde_dir(&self, prefix: &str) -> Option> { + use std::borrow::Cow; let lookup = |name: &str| { self.scoped .variables .get(name) .or_else(|| self.env.get(name)) - .cloned() + .map(|value| Cow::Borrowed(value.as_str())) }; match prefix { "" => Some( self.env .get("HOME") .or_else(|| self.scoped.variables.get("HOME")) - .cloned() - .unwrap_or_else(|| format!("/home/{}", self.tilde_user)), + .map(|value| Cow::Borrowed(value.as_str())) + .unwrap_or_else(|| Cow::Owned(format!("/home/{}", self.tilde_user))), ), - "+" => Some(lookup("PWD").unwrap_or_else(|| self.cwd.display().to_string())), + "+" => { + Some(lookup("PWD").unwrap_or_else(|| Cow::Owned(self.cwd.display().to_string()))) + } "-" => lookup("OLDPWD"), - "root" => Some("/root".to_string()), - user if user == &*self.tilde_user => Some(format!("/home/{user}")), + "root" => Some(Cow::Borrowed("/root")), + user if user == &*self.tilde_user => Some(Cow::Owned(format!("/home/{user}"))), _ => None, } } @@ -937,13 +980,21 @@ impl Interpreter { starts: &[usize], after_colons: bool, word_continues: bool, - ) -> Option { - let first = *starts.first()?; - let mut out = String::new(); + ) -> std::result::Result, crate::limits::LimitExceeded> { + let Some(&first) = starts.first() else { + return Ok(None); + }; + // THREAT[TM-DOS-134]: directory length times prefix count is untrusted. + // Reserve before allocation; copying bytes also polls deadline/cancellation. + self.execution_budget.consume_work(s.len() as u64)?; + let mut out = BudgetedString::new(Some(&self.execution_budget))?; let mut copied = 0; let mut pos = first; let bytes = s.as_bytes(); while pos < s.len() { + if pos.is_multiple_of(1_024) { + self.execution_budget.check()?; + } let eligible = starts.contains(&pos) || (after_colons && pos > first && bytes[pos - 1] == b':'); if eligible && bytes[pos] == b'~' { @@ -954,8 +1005,10 @@ impl Interpreter { if (end < s.len() || !word_continues) && let Some(dir) = self.tilde_dir(&s[pos + 1..end]) { - out.push_str(&s[copied..pos]); - out.push_str(&dir); + self.execution_budget.consume_work((pos - copied) as u64)?; + self.execution_budget.consume_work(dir.len() as u64)?; + out.try_push_str(&s[copied..pos])?; + out.try_push_str(&dir)?; copied = end; pos = end; continue; @@ -964,16 +1017,23 @@ impl Interpreter { pos += 1; } if copied == 0 { - return None; + return Ok(None); } - out.push_str(&s[copied..]); - Some(out) + self.execution_budget + .consume_work((s.len() - copied) as u64)?; + out.try_push_str(&s[copied..])?; + Ok(Some(out)) } /// Tilde expansion for literal part `idx` of `word`: a leading `~`, and /// in a word that looks like an assignment (`x=~:~`, as an argument to /// `echo` or `local`) the `~` after its `=` and after each `:`. - fn tilde_expand_word_literal(&self, word: &Word, idx: usize, s: &str) -> Option { + fn tilde_expand_word_literal( + &self, + word: &Word, + idx: usize, + s: &str, + ) -> std::result::Result, crate::limits::LimitExceeded> { let word_continues = idx + 1 < word.parts.len(); let assign_eq = match word.parts.first() { Some(WordPart::Literal(first)) if Self::is_assignment_prefix(first) => first.find('='), @@ -986,9 +1046,13 @@ impl Interpreter { } return self.tilde_expand_literal(s, &starts, assign_eq.is_some(), word_continues); } - assign_eq?; + if assign_eq.is_none() { + return Ok(None); + } // Only `:` inside this part can start a prefix here. - let colon = s.find(':')?; + let Some(colon) = s.find(':') else { + return Ok(None); + }; self.tilde_expand_literal(s, &[colon + 1], true, word_continues) } @@ -1039,7 +1103,13 @@ impl Interpreter { }) } - pub(super) fn tilde_assignment_value<'w>(&self, word: &'w Word) -> std::borrow::Cow<'w, Word> { + pub(super) fn tilde_assignment_value<'w>( + &self, + word: &'w Word, + ) -> Result<( + std::borrow::Cow<'w, Word>, + Vec, + )> { use std::borrow::Cow; let quoted_at = |i: usize| Self::part_is_quoted(word, i); let has_tilde = word.parts.iter().enumerate().any(|(i, p)| match p { @@ -1048,8 +1118,9 @@ impl Interpreter { _ => false, }); if !has_tilde { - return Cow::Borrowed(word); + return Ok((Cow::Borrowed(word), Vec::new())); } + let mut leases = Vec::new(); let mut out = word.clone(); out.part_quoted = (0..word.parts.len()).map(quoted_at).collect(); let n = word.parts.len(); @@ -1068,6 +1139,9 @@ impl Interpreter { { // `x=${undef-~:~}`: the operand of an assignment value // expands `~` after each `:` too. + // THREAT[TM-DOS-134]: budget the rebuilt operand the + // declaration retains across expansion. + leases.push(self.execution_budget.lease_bytes(expanded.len())?); out.parts[i] = WordPart::ParameterExpansion { name: name.clone(), operator: op.clone(), @@ -1094,13 +1168,17 @@ impl Interpreter { starts.push(c + 1); } starts.sort_unstable(); - if let Some(expanded) = self.tilde_expand_literal(s, &starts, true, i + 1 < n) { + if let Some(expanded) = self.tilde_expand_literal(s, &starts, true, i + 1 < n)? { + // The preprocessed literal and expand_word's copy coexist. + leases.push(self.execution_budget.lease_bytes(expanded.len())?); + let (expanded, lease) = expanded.into_parts(); + leases.extend(lease); out.parts[i] = WordPart::Literal(expanded); } // Done with this part's tildes (`x=a=~` keeps its `~`). out.part_quoted[i] = true; } - Cow::Owned(out) + Ok((Cow::Owned(out), leases)) } /// Expand a word to multiple fields (for array iteration and command args) @@ -1386,6 +1464,7 @@ impl Interpreter { let ifs_null = matches!(self.scoped.variables.get("IFS"), Some(v) if v.is_empty()); // A null IFS never splits, but unquoted `$@`/`$*` still yield one // field per parameter: each starts a new group of segments. + let mut tilde_leases = Vec::new(); let mut groups: Vec> = vec![Vec::new()]; for (idx, part) in word.parts.iter().enumerate() { let quoted = word.part_quoted.get(idx).copied().unwrap_or(word.quoted); @@ -1449,7 +1528,10 @@ impl Interpreter { let segment = match p { // Unquoted operand text is expansion output: it splits. WordPart::Literal(s) if !q => (s.clone(), false, false), - _ => self.mixed_word_segment(&inner, i, p, q).await?, + _ => { + self.mixed_word_segment(&inner, i, p, q, &mut tilde_leases) + .await? + } }; if let Some(group) = groups.last_mut() { group.push(segment); @@ -1457,7 +1539,9 @@ impl Interpreter { } continue; } - let segment = self.mixed_word_segment(word, idx, part, quoted).await?; + let segment = self + .mixed_word_segment(word, idx, part, quoted, &mut tilde_leases) + .await?; if let Some(group) = groups.last_mut() { group.push(segment); } @@ -1485,14 +1569,23 @@ impl Interpreter { idx: usize, part: &WordPart, part_is_quoted: bool, + tilde_leases: &mut Vec, ) -> Result<(String, bool, bool)> { let part_has_expansion = Self::is_field_split_expansion(part); let value = if let WordPart::Literal(s) = part { // A leading `~` sees the whole word (`~$v` stays literal). - (idx == 0 && !part_is_quoted && s.contains('~')) - .then(|| self.tilde_expand_word_literal(word, 0, s)) - .flatten() - .unwrap_or_else(|| s.clone()) + let expanded = if idx == 0 && !part_is_quoted && s.contains('~') { + self.tilde_expand_word_literal(word, 0, s)? + } else { + None + }; + if let Some(expanded) = expanded { + let (value, lease) = expanded.into_parts(); + tilde_leases.extend(lease); + value + } else { + s.clone() + } } else { let single = Word { parts: vec![part.clone()], @@ -1610,6 +1703,7 @@ impl Interpreter { return Ok(None); } // Each field under construction is a list of segments. + let mut tilde_leases = Vec::new(); let mut fields: Vec> = vec![Vec::new()]; for (idx, part) in word.parts.iter().enumerate() { let q = part_quoted(idx); @@ -1629,7 +1723,9 @@ impl Interpreter { } continue; } - let segment = self.mixed_word_segment(word, idx, part, q).await?; + let segment = self + .mixed_word_segment(word, idx, part, q, &mut tilde_leases) + .await?; if let Some(field) = fields.last_mut() { field.push(segment); } @@ -2171,19 +2267,35 @@ impl Interpreter { self.limits.max_parser_operations, ); let mut result = String::new(); + let mut operand_leases = Vec::new(); let mut in_marked = false; for (idx, part) in word.parts.iter().enumerate() { match part { WordPart::Literal(s) => { // `${x:-~}`, `${x#~/}`: a leading unquoted `~`. - let tilde_expanded = (tilde && idx == 0 && !force_quoted) - .then(|| { - let seg_end = quote_mark.and_then(|m| s.find(m)).unwrap_or(s.len()); - let continues = seg_end < s.len() || word.parts.len() > 1; - self.tilde_expand_literal(&s[..seg_end], &[0], false, continues) - .map(|e| format!("{e}{}", &s[seg_end..])) - }) - .flatten(); + let tilde_expanded = if tilde && idx == 0 && !force_quoted { + let seg_end = quote_mark.and_then(|m| s.find(m)).unwrap_or(s.len()); + let continues = seg_end < s.len() || word.parts.len() > 1; + let expansion = (|| { + let Some(mut expanded) = + self.tilde_expand_literal(&s[..seg_end], &[0], false, continues)? + else { + return Ok(None); + }; + expanded.try_push_str(&s[seg_end..])?; + operand_leases.push(self.execution_budget.lease_bytes(expanded.len())?); + Ok::<_, crate::limits::LimitExceeded>(Some(expanded)) + })(); + match expansion { + Ok(expanded) => expanded, + Err(error) => { + self.memory_limit_error.get_or_insert(error); + return String::new(); + } + } + } else { + None + }; let text = tilde_expanded.as_deref().unwrap_or(s); if repl { Self::push_replacement_literal( diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index cc41fd5e6..e252337a6 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -8444,7 +8444,7 @@ impl Interpreter { } match &assignment.value { AssignmentValue::Scalar(word) => { - let word = self.tilde_assignment_value(word); + let (word, _tilde_leases) = self.tilde_assignment_value(word)?; let value = self.expand_word(&word).await?; if self.is_xtrace_enabled() { // `+ v=x`, `+ a[1]='a b'`, `+ v+=y` diff --git a/crates/bashkit/tests/integration/execution_budget_tests.rs b/crates/bashkit/tests/integration/execution_budget_tests.rs index dda05244c..a04a12d17 100644 --- a/crates/bashkit/tests/integration/execution_budget_tests.rs +++ b/crates/bashkit/tests/integration/execution_budget_tests.rs @@ -549,6 +549,32 @@ async fn indexed_array_pending_values_share_live_byte_limit() { } } +#[tokio::test] +async fn tilde_expansion_repeated_directories_respect_live_budget() { + // Safe reproducer: at most 24 KiB of expansion, admitted by retained limits. + for (name, tilde) in [("HOME", "~"), ("PWD", "~+"), ("OLDPWD", "~-")] { + for (command, suffix) in [ + ("x=a", ""), + ("export x=a", ""), + ("echo x=a", ""), + ("x[0]=a", ""), + ("declare -a x=([0]=a", ")"), + ("echo x=a", "\"\""), + ] { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(8_192); + let mut bash = Bash::builder().limits(limits).build(); + let script = format!( + "export {name}='{}'; {command}{}{suffix}", + "d".repeat(1_024), + format!(":{tilde}").repeat(24) + ); + assert_budget_exhausted(bash.exec(&script).await); + let recovered = bash.exec("echo recovered").await.unwrap(); + assert_eq!(recovered.stdout, "recovered\n"); + } + } +} + #[tokio::test] async fn indexed_array_pending_keys_share_live_byte_limit() { let mut bash = Bash::builder() @@ -593,6 +619,66 @@ async fn indexed_array_budget_preserves_expansion_order() { assert_eq!(result.stderr.as_bytes(), expected.stderr); } +#[tokio::test] +async fn tilde_expansion_charges_copied_bytes_as_work() { + let limits = ExecutionLimits::new().max_work_units(5_000); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec(&format!("HOME='{}'", "d".repeat(1_024))) + .await + .unwrap(); + assert_budget_exhausted(bash.exec(&format!("x=a{}", ":~".repeat(24))).await); +} + +#[tokio::test] +async fn tilde_expansion_single_directory_and_operands_respect_live_budget() { + for command in ["x=~", "echo ~", "echo ${unset:-~}", "echo ${unset:=~}"] { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(512); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec(&format!("HOME='{}'", "d".repeat(1_024))) + .await + .unwrap(); + assert_budget_exhausted(bash.exec(command).await); + } +} + +#[tokio::test] +async fn tilde_expansion_retains_sibling_assignment_leases() { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(3_072); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec(&format!("HOME='{}'", "d".repeat(1_024))) + .await + .unwrap(); + assert_budget_exhausted(bash.exec("x=~/\"\":~/\"\":~/").await); +} + +#[tokio::test] +async fn tilde_expansion_preserves_bash_semantics() { + let script = r#"HOME='/home/test dir'; PWD=/pwd; OLDPWD=/old +x=~:~+/bin:~-/lib:~missing +printf '%s\n' "$x" ~ ~missing +printf '%s\n' x=~:~+:'~' +x='~:~'; printf '%s\n' "$x" '~' \~ '~'$HOME +unset value; printf '%s\n' "${value:-~}" "${value:-'~'}" +x=~/"":~; printf '%s\n' "$x" +"#; + let expected = std::process::Command::new("bash") + .env("HOME", "/home/test dir") + .arg("--noprofile") + .arg("--norc") + .arg("-c") + .arg(script) + .output() + .unwrap(); + assert!(expected.status.success()); + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_live_intermediate_bytes(8_192)) + .build(); + let result = bash.exec(script).await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout.as_bytes(), expected.stdout); + assert_eq!(result.stderr.as_bytes(), expected.stderr); +} + #[tokio::test] async fn indexed_array_empty_pending_items_still_charge_container_storage() { let mut bash = Bash::builder() @@ -632,3 +718,37 @@ async fn indexed_array_substitutions_release_consumed_output() { .unwrap(); assert_eq!(result.stdout, "3072\n"); } + +#[tokio::test] +async fn tilde_expansion_quotes_do_not_resolve_large_directories() { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(512); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec(&format!("HOME='{}'", "d".repeat(1_024))) + .await + .unwrap(); + let literal = format!("a{}", ":~".repeat(24)); + let actual = bash + .exec(&format!("x='{literal}'; printf '%s' \"$x\"")) + .await + .unwrap(); + assert_eq!(actual.stdout, literal); +} + +#[tokio::test] +async fn tilde_expansion_assignment_copy_boundary_and_release() { + for (bytes, succeeds) in [(15, false), (16, true)] { + let limits = ExecutionLimits::new().max_live_intermediate_bytes(bytes); + let mut bash = Bash::builder().limits(limits).build(); + bash.exec("HOME=12345678").await.unwrap(); + let result = bash.exec("x=~; y=~").await; + if succeeds { + assert_eq!(result.unwrap().exit_code, 0); + // Declaration leases release: a trivial follow-up still fits the + // same budget (value assertions live in the semantics test, + // which runs with a generous budget). + assert_eq!(bash.exec("echo done").await.unwrap().stdout, "done\n"); + } else { + assert_budget_exhausted(result); + } + } +} diff --git a/docs/security.md b/docs/security.md index dee6fb826..69a8508ea 100644 --- a/docs/security.md +++ b/docs/security.md @@ -59,6 +59,12 @@ execution work and deadline budget; file contents are admitted against aggregate input and live intermediate limits and scanned one file at a time. `-q` stops at the first match, before further traversal. +Tilde expansion (`~`, `~+`, `~-`, including repeated assignment prefixes after +`:`) checks `ExecutionLimits::max_live_intermediate_bytes` before growing its +output and charges copied bytes to `max_work_units`. Large directory values +cannot bypass these limits by expanding before variable insertion. A refused +expansion returns a resource-limit error; the next host execution receives a +fresh execution budget. Syntax-error reports are limited to 1 KiB during construction, including an explicit `[diagnostic truncated]` marker when text is omitted. This prevents multiline unexpected tokens and long `bash -c` shell names from multiplying diff --git a/knowledge/foundations/architecture.md b/knowledge/foundations/architecture.md index b6d2bb4c2..1da1a48d6 100644 --- a/knowledge/foundations/architecture.md +++ b/knowledge/foundations/architecture.md @@ -135,6 +135,15 @@ compression paths use these builders because their expansion ratios and nested buffers make post-hoc leasing unsafe. Atomic compare/exchange admission keeps concurrent descendants from wrapping or temporarily exceeding the shared cap. +Tilde expansion uses the same owning string builder before each append; it +borrows `HOME`/`PWD`/`OLDPWD` instead of cloning the directory on each prefix. +Scan and copy work polls the shared deadline and cancellation flag. Assignment +preprocessing retains leases for expanded literal buffers and their word copies +through consumption; mixed-word segments retain leases through field splitting. +The synchronous operand pipeline records a refused reservation in the existing +sticky resource-error slot, checked before the completed word is returned. +Retained-variable admission remains a separate check after expansion. + ### `set -e` contexts `errexit_active()` is `set -e` plus `condition_sequence_depth == 0`. The diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index f09fb23da..b3230a31d 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -300,6 +300,7 @@ runaway scripts without permanently breaking the session. | TM-DOS-130 | Array and arithmetic report growth | `$(( $(( ... )) ))` nested inside arithmetic, thousands of `${a[-9]}` reads in one command, `${!r}` with a huge invalid target | Nested `$((` in arithmetic retains its 32-occurrence admission cap and shares the arithmetic depth/fuel budget with dollar subscripts (`expression recursion level exceeded`); queued `bad array subscript` warnings stop at 64 KiB per command; an invalid indirect target is echoed truncated to 256 chars. Regression: `array_and_arith_reports_are_bounded` | **MITIGATED** | | TM-DOS-131 | History file growth and reads | `history -a` in a loop, `HISTSIZE` huge, a `bash -i` child reading a giant `$HISTFILE`, `history -r` on a multi-GB file | The history list stays under `max_history_entries`/`max_history_bytes` whatever `$HISTSIZE` says (TM-DOS-094 caps apply to every recorded line); `$HISTFILE` reads are refused past `max_input_bytes`; files are read and written through the VFS only, so its size and write limits apply; a `bash -i` child keeps its own list and the parent's comes back after it. Regression: `history_lines_stay_bounded`, `histfile_read_is_capped` | **MITIGATED** | | 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-134 | Repeated tilde expansion amplification | A large `HOME`, `PWD` or `OLDPWD` repeated through assignment `:~`, `:~+` or `:~-` prefixes builds host strings before retained-variable limits run | Shared tilde expansion borrows directory values, charges scan/copy work (including deadline and cancellation checks), and uses `BudgetedString` to admit checked capacity growth before allocation. Preprocessed assignment literals keep their leases and reserve their word copies until assignment consumption; mixed-word segments keep leases through field splitting. Synchronous operand failures use the sticky resource-error slot and abort word expansion. Regression tests: `execution_budget_tests::tilde_expansion_*` use bounded 1 KiB directory values, small live/work budgets, session recovery and a real-Bash oracle | **MITIGATED** | | TM-DOS-136 | Syntax diagnostic amplification | A multiline unexpected token combined with a long attacker-controlled `bash -c` ARG0 repeats the shell name for every token line, allocating quadratically before stderr capture | Unexpected-token messages and complete syntax reports use a fixed-capacity UTF-8 writer capped at 1 KiB before any append, stop formatting on overflow, and include `[diagnostic truncated]`; short reports preserve Bash semantics. Regressions: `error::tests::syntax_report_*`, `parser_bounds_unexpected_token_before_copying`, and `partial_parse_tests::{child_syntax_report_is_bounded, small_child_syntax_reports_match_bash}` | **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** | From b84bd19372dc0be02a9f886233b89a5c92e596ac Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:31:18 -0500 Subject: [PATCH 4/6] fix(interpreter): bound re-entrant signal-trap delivery depth Synchronous self-signal delivery (kill -> deliver -> run_trap_command inline) had no depth charge: direct, mutual, and mixed function/source recursion overflowed the native stack instead of failing cleanly. - Charge trap entries against the shared function/source hard ceiling and a dedicated trap-nesting cap (3) that trips first on small stacks - Exhaustion skips the nested handler with a diagnostic; the first trip surfaces as a resource-limit error at the exec boundary, while handler logic failures still never disturb the interrupted command (TM-DOS-030) - Box the handler and sourced-body execution edges (TM-DOS-089 pattern) - Flag depth trips from nested function/source pushes, which otherwise stop the recursion silently via status mapping - Regressions: in-process depth tests for all four shapes plus recovery, small-stack CLI subprocess probes, finite-nesting/exit/ignored specs --- crates/bashkit-cli/tests/cli_oneshot.rs | 42 +++++++ crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/interpreter/mod.rs | 109 +++++++++++++++--- crates/bashkit/src/lib.rs | 10 ++ crates/bashkit/src/limits.rs | 13 ++- crates/bashkit/tests/integration/main.rs | 1 + .../integration/signal_trap_depth_tests.rs | 86 ++++++++++++++ .../tests/spec_cases/bash/kill-self.test.sh | 28 +++++ docs/security.md | 2 +- knowledge/foundations/architecture.md | 15 +++ knowledge/log.md | 1 + knowledge/operations/limitations.md | 2 +- knowledge/security/threat-model.md | 2 +- 13 files changed, 290 insertions(+), 23 deletions(-) create mode 100644 crates/bashkit/tests/integration/signal_trap_depth_tests.rs diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index 3e066656c..36a798994 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -148,6 +148,48 @@ fn function_recursion_is_bounded_on_a_two_mib_stack() { } } +// Child isolation and a small command allowance keep even a missing guard safe. +#[cfg(unix)] +#[test] +fn signal_trap_recursion_is_bounded_on_a_two_mib_stack() { + for script in [ + "trap 'kill -USR1 $$' USR1; kill -USR1 $$", + "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + ] { + let mut child = Command::new("sh") + .args([ + "-c", + "set -e; ulimit -s 2048; exec \"$1\" --no-stdin --max-commands 64 -c \"$2\"", + "sh", + BIN, + script, + ]) + .env_remove("RUST_BACKTRACE") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn signal trap probe"); + let deadline = Instant::now() + Duration::from_secs(10); + while child.try_wait().unwrap().is_none() { + if Instant::now() >= deadline { + child.kill().unwrap(); + let out = child.wait_with_output().unwrap(); + panic!("signal trap probe hung: {script}: {}", stderr(&out)); + } + thread::sleep(Duration::from_millis(10)); + } + let out = child.wait_with_output().unwrap(); + assert_eq!(out.status.code(), Some(1), "{script}: {}", stderr(&out)); + assert!( + stderr(&out).contains("depth exceeded"), + "{script}: {}", + stderr(&out) + ); + } +} + #[cfg(unix)] #[test] fn function_nesting_below_limit_works_on_a_two_mib_stack() { diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 21995b7bd..456d9de18 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -88,7 +88,7 @@ through configurable limits. | Threat | Attack Example | Mitigation | Status | |--------|---------------|------------|--------| -| Function recursion (TM-DOS-020) | `f() { f; }; f` | Depth limit of 16, including a hard ceiling when callers request more, protects 2 MiB host stacks | MITIGATED | +| Function and signal-trap recursion (TM-DOS-020) | `f() { f; }; f` or `trap 'kill -USR1 $$' USR1; kill -USR1 $$` | Functions, source, child shells and trap handlers share a hard depth ceiling of 16, plus a dedicated trap-nesting cap of 3 that trips first on small stacks; nested exhaustion is skipped with a diagnostic and the first trip fails the exec as a resource-limit error, while handler logic failures never disturb the interrupted command (TM-DOS-030). Subprocess tests cover direct, mutual and mixed recursion on 2 MiB stacks | MITIGATED | | Command sub depth (TM-DOS-021) | `$($($($())))` nesting | Inherited depth/fuel from parent | MITIGATED | | Parser depth (TM-DOS-022) | `(((((...))))))` nesting | `max_ast_depth` + hard cap (100) | MITIGATED | | Arithmetic depth (TM-DOS-026) | `$(((((...))))))` | `MAX_ARITHMETIC_DEPTH` (50) | MITIGATED | diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index e252337a6..bf410faca 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -30,6 +30,8 @@ // sandbox "Did you mean" / unavailable-command hints after // `command not found` are kept on purpose. +// Decision: trap handlers share the function/source depth budget. Signal delivery +// propagates handler failures; swallowing a nested limit error reports success. mod arithmetic; mod brace_expansion; mod completion; @@ -87,7 +89,10 @@ use crate::builtins::{self, Builtin}; use crate::error::Error; use crate::error::Result; use crate::fs::FileSystem; -use crate::limits::{BudgetedString, ExecutionCounters, ExecutionLimits, SessionLimits}; +use crate::limits::{ + BudgetedString, ExecutionCounters, ExecutionLimits, HARD_MAX_TRAP_NESTING, LimitExceeded, + SessionLimits, +}; /// A single command history entry. #[derive(Debug, Clone)] @@ -1925,6 +1930,10 @@ pub struct Interpreter { /// True while executing a trap handler. Suppresses recursive DEBUG trap /// invocation to prevent amplification attacks (TM-DOS-035). in_trap: bool, + /// Nesting level of `run_trap_command` entries (TM-DOS-020). + trap_nesting: usize, + /// First trap-nesting depth error, surfaced at the `exec` boundary. + trap_depth_error: Option, /// The ERR trap is inherited for display only: a subshell, command /// substitution or pipeline stage without `set -E` keeps the parent's /// handler visible to `trap -p` but does not run it (bash). `trap` @@ -2562,6 +2571,8 @@ impl Interpreter { cancelled: Arc::new(AtomicBool::new(false)), hooks: Arc::new(crate::hooks::Hooks::default()), in_trap: false, + trap_nesting: 0, + trap_depth_error: None, err_trap_dormant: false, debug_trap_dormant: false, err_trap_skip_stage: false, @@ -2782,6 +2793,8 @@ impl Interpreter { cancelled: Arc::clone(&self.cancelled), hooks: Arc::clone(&self.hooks), in_trap: false, + trap_nesting: 0, + trap_depth_error: None, // A job or pipeline stage is a subshell: ERR stays dormant there // unless `set -E`. err_trap_dormant: self.err_trap_dormant || !self.flags.contains(BashFlags::ERRTRACE), @@ -3101,6 +3114,8 @@ impl Interpreter { // handler is awaited; clear the re-entrancy guard before each exec so // one cancelled script cannot suppress traps in the next script. self.in_trap = false; + self.trap_nesting = 0; + self.trap_depth_error = None; self.err_trap_dormant = false; self.debug_trap_dormant = false; self.err_trap_skip_stage = false; @@ -9985,7 +10000,7 @@ impl Interpreter { .unwrap_or(u64::MAX), )?; - self.apply_builtin_side_effects(&mut result).await; + self.apply_builtin_side_effects(&mut result).await?; let result = self.apply_redirections(result, redirects).await?; self.apply_after_tool(name, result) @@ -10885,12 +10900,18 @@ impl Interpreter { }; // THREAT[TM-DOS-056]: Check source depth (uses function depth limit) - self.counters.push_function(&self.limits).map_err(|_| { - crate::error::Error::Execution(format!( + if let Err(depth_error) = self.counters.push_function(&self.limits) { + // THREAT[TM-DOS-020]: a depth trip inside a trap handler must + // still fail the execution instead of silently stopping the + // recursion (the `source` status mapping below would hide it). + if self.in_trap { + self.note_trap_depth_error(depth_error); + } + return Err(crate::error::Error::Execution(format!( "source: {}: maximum source depth exceeded", filename - )) - })?; + ))); + } // Track source file for BASH_SOURCE self.bash_source_stack.push(SourceFrame { @@ -10909,7 +10930,10 @@ impl Interpreter { self.xtrace_depth += 1; let tempenv_pushed = self.push_pending_tempenv(false); let region = self.enter_output_region(redirects); - let mut exec_result = self.execute_script_body(&script, false, true).await; + // THREAT[TM-DOS-020]: Box the sourced-body execution so a source / + // trap recursion cycle cannot grow the parent future with each + // nesting level; the shared depth charge above then trips first. + let mut exec_result = Box::pin(self.execute_script_body(&script, false, true)).await; self.leave_output_region(region); if tempenv_pushed { self.tempenv_frames.pop(); @@ -12354,7 +12378,7 @@ impl Interpreter { } /// Process structured side effects from builtin execution. - async fn apply_builtin_side_effects(&mut self, result: &mut ExecResult) { + async fn apply_builtin_side_effects(&mut self, result: &mut ExecResult) -> Result<()> { // Builtins that mutate SHOPT_* directly via `ctx.variables` (e.g. the // `set -e` / `set +u` paths in the `set` builtin) don't update the // cached `flags` bitfield. Resync once after every builtin so the @@ -12466,6 +12490,7 @@ impl Interpreter { if let Some(signal) = pending_self_signal { self.deliver_self_signal(signal, result).await; } + Ok(()) } /// `kill -SIG $$`: bash runs that signal's trap if one is set, and @@ -12473,11 +12498,12 @@ impl Interpreter { /// 128 + signal. Signals whose default is to be ignored do neither. async fn deliver_self_signal(&mut self, signal: i32, result: &mut ExecResult) { if let Some(trap_cmd) = self.signal_trap(signal) { + if trap_cmd.is_empty() { + return; + } let mut stdout = std::mem::take(&mut result.stdout); let mut stderr = std::mem::take(&mut result.stderr); - let flow = self - .run_trap_command(&trap_cmd, &mut stdout, &mut stderr) - .await; + let flow = Box::pin(self.run_trap_command(&trap_cmd, &mut stdout, &mut stderr)).await; result.stdout = stdout; result.stderr = stderr; // An `exit` inside the handler ends the script, as it does in bash. @@ -12498,6 +12524,7 @@ impl Interpreter { self.last_exit_code = status; result.exit_code = status; result.control_flow = ControlFlow::Exit(status); + () } /// Resolve a path relative to cwd, normalizing `.` and `..` components. @@ -14481,7 +14508,21 @@ impl Interpreter { /// Run a trap handler's command, merging its output into the result the /// caller is building. - /// Returns the handler's own control flow (an `exit` inside it), if any. + /// Returns the handler's control flow and status, or `None` when the + /// handler cannot run (unparsable command, exhausted depth budget, or a + /// failing handler): trap failures never disturb the interrupted command. + /// Record the first trap-nesting depth error for the `exec` boundary. + fn note_trap_depth_error(&mut self, error: LimitExceeded) { + if self.trap_depth_error.is_none() { + self.trap_depth_error = Some(error); + } + } + + /// Take the recorded trap-nesting depth error, if any. + pub(crate) fn take_trap_depth_error(&mut self) -> Option { + self.trap_depth_error.take() + } + async fn run_trap_command( &mut self, trap_cmd: &str, @@ -14492,6 +14533,26 @@ impl Interpreter { let Ok(trap_script) = self.parse_shell_text(trap_cmd) else { return None; }; + // THREAT[TM-DOS-020]: Bound nested trap delivery with a dedicated + // hard cap that trips before native frames can exhaust small host + // stacks, independent of command fuel, per-handler AST depth, and + // the shared function/source budget (which is charged too, so a low + // `max_function_depth` still stops function/signal mixes early). + // Exhaustion skips the nested handler with a diagnostic; the first + // trip is surfaced as a resource-limit error at the `exec` + // boundary, and never disturbs the interrupted command. + if self.trap_nesting >= HARD_MAX_TRAP_NESTING { + self.note_trap_depth_error(LimitExceeded::MaxTrapDepth(HARD_MAX_TRAP_NESTING)); + stderr.push_str("bashkit: maximum trap nesting depth exceeded\n"); + return None; + } + self.trap_nesting += 1; + if let Err(depth_error) = self.counters.push_function(&self.limits) { + self.trap_nesting -= 1; + self.note_trap_depth_error(depth_error); + stderr.push_str("bashkit: maximum trap nesting depth exceeded\n"); + return None; + } let was_in_trap = self.in_trap; self.in_trap = true; let saved_line_base = std::mem::replace( @@ -14500,12 +14561,28 @@ impl Interpreter { ); let emit_before = self.output_emit_count; self.xtrace_depth += 1; - let result = self.execute_command_sequence(&trap_script.commands).await; + // THREAT[TM-DOS-020]: Box the handler execution so a trap / + // source recursion cycle cannot grow the parent future with each + // nesting level; the shared depth charge above then trips first. + let result = Box::pin(self.execute_command_sequence(&trap_script.commands)).await; self.xtrace_depth -= 1; self.line_base = saved_line_base; self.in_trap = was_in_trap; - let Ok(trap_result) = result else { - return None; + self.trap_nesting -= 1; + self.counters.pop_function(); + // THREAT[TM-DOS-030]: A failing handler must not disturb the + // interrupted command, so execution errors end the handler here. + // THREAT[TM-DOS-020]: resource exhaustion inside the handler + // (a nested function/source call tripping depth or fuel) is not a + // handler logic failure: record it so the `exec` boundary still + // fails instead of silently stopping the recursion. + let trap_result = match result { + Ok(trap_result) => trap_result, + Err(crate::error::Error::ResourceLimit(limit)) => { + self.note_trap_depth_error(limit); + return None; + } + Err(_) => return None, }; self.maybe_emit_output(&trap_result.stdout, &trap_result.stderr, emit_before); stdout.append(&trap_result.stdout); @@ -14591,7 +14668,7 @@ impl Interpreter { // The handler runs with `$?` of the trapping command and the // shell keeps that status afterwards (bash restores it). let saved_exit = self.last_exit_code; - let _ = self.run_trap_command(&trap_cmd, stdout, stderr).await; + let _ = Box::pin(self.run_trap_command(&trap_cmd, stdout, stderr)).await; self.last_exit_code = saved_exit; } } diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index c7bef27da..37bb85093 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -970,6 +970,16 @@ impl Bash { if let Ok(exec_result) = &mut result { exec_result.capability_cleanup = cleanup; } + // THREAT[TM-DOS-020]: trap-nesting exhaustion skips the nested + // handler without disturbing the interrupted command, but resource + // exhaustion still fails the execution, like command fuel. + if let Some(depth_error) = self + .interpreter + .take_trap_depth_error() + .filter(|_| result.is_ok()) + { + result = Err(Error::ResourceLimit(depth_error)); + } result } diff --git a/crates/bashkit/src/limits.rs b/crates/bashkit/src/limits.rs index d368c00c8..0b3d76aa2 100644 --- a/crates/bashkit/src/limits.rs +++ b/crates/bashkit/src/limits.rs @@ -36,6 +36,11 @@ use crate::time_compat::Instant; // embedders request a higher logical recursion limit. const HARD_MAX_FUNCTION_DEPTH: usize = 16; +// Self-delivered trap handlers re-enter the interpreter synchronously, so a +// small dedicated nesting cap trips before native frames can exhaust small +// host stacks, independent of the shared function/source budget below. +pub(crate) const HARD_MAX_TRAP_NESTING: usize = 3; + // Prompt reparses poll recursively on the host stack, even with shallow ASTs. // THREAT[TM-DOS-133]: No configurable AST limit may remove this stack ceiling. const HARD_MAX_PROMPT_DEPTH: usize = 8; @@ -76,7 +81,7 @@ pub struct ExecutionLimits { /// Default: 1,000,000 pub max_total_loop_iterations: usize, - /// Maximum function call depth (recursion limit) + /// Maximum shared function, source, child-shell, and trap-handler depth /// Default: 16; values above 16 are capped for host stack safety. pub max_function_depth: usize, @@ -272,8 +277,8 @@ impl ExecutionLimits { self } - /// Set maximum function depth. - /// A value of 0 is a valid strict limit that prevents function calls. + /// Set the shared function, source, child-shell, and trap-handler depth. + /// A value of 0 prevents entry into any of these execution frames. pub fn max_function_depth(mut self, depth: usize) -> Self { self.max_function_depth = depth; self @@ -735,6 +740,8 @@ pub enum LimitExceeded { #[error("maximum prompt expansion depth exceeded ({0})")] MaxPromptDepth(usize), + #[error("maximum trap nesting depth exceeded ({0})")] + MaxTrapDepth(usize), #[error("maximum file descriptors exceeded ({0})")] MaxFileDescriptors(usize), diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index 28019d11d..ac5d7a177 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -138,6 +138,7 @@ mod sed_resource_tests; pub mod set_e_and_or_tests; pub mod shlvl_tests; pub mod shuf_resource_tests; +pub mod signal_trap_depth_tests; pub mod skills_tests; pub mod snapshot_fixture_tests; pub mod snapshot_history_tests; diff --git a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs new file mode 100644 index 000000000..5d09af2b2 --- /dev/null +++ b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs @@ -0,0 +1,86 @@ +//! Signal handlers share the function/source depth ceiling (TM-DOS-020). +//! Small command budgets keep pre-fix recursion probes safe. + +use bashkit::{Bash, ExecutionLimits}; + +#[tokio::test] +async fn recursive_signal_traps_share_function_depth() { + for script in [ + "trap 'kill -USR1 $$' USR1; kill -USR1 $$", + "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + ] { + for depth in [2, 3] { + let mut bash = Bash::builder() + .limits( + ExecutionLimits::new() + .max_function_depth(depth) + .max_commands(12), + ) + .build(); + let err = bash.exec(script).await.unwrap_err().to_string(); + assert!(err.contains("depth exceeded"), "{script}: {err}"); + assert!(!err.contains("command limit"), "{script}: {err}"); + let next = bash.exec("echo recovered").await.unwrap(); + assert_eq!(next.stdout, "recovered\n"); + assert_eq!(next.exit_code, 0); + } + } +} + +#[tokio::test] +async fn signal_depth_is_released_after_each_handler() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(2)) + .build(); + let result = bash.exec("trap 'echo first; kill -USR2 $$; echo last' USR1; trap 'echo second' USR2; kill -USR1 $$; kill -USR1 $$; echo done").await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!( + result.stdout, + "first\nsecond\nlast\nfirst\nsecond\nlast\ndone\n" + ); + assert!(result.stderr.is_empty(), "{}", result.stderr); +} + +#[tokio::test] +async fn ignored_signal_needs_no_handler_depth() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(0)) + .build(); + let result = bash + .exec("trap '' USR1; kill -USR1 $$; echo ignored") + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, "ignored\n"); +} + +#[tokio::test] +async fn signal_handler_command_limit_reaches_host() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_commands(3)) + .build(); + let err = bash + .exec("trap 'echo first; echo second' USR1; kill -USR1 $$") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("maximum command count exceeded"), "{err}"); +} + +#[tokio::test] +async fn signal_handler_parse_error_is_swallowed() { + // TM-DOS-030 (merged main semantics): an unparsable handler never + // disturbs the interrupted command; the kill completes and the shell + // stays usable. + let mut bash = Bash::builder().build(); + let result = bash + .exec("trap 'if' USR1; kill -USR1 $$; echo done") + .await + .unwrap(); + assert_eq!(result.stdout, "done\n"); + assert_eq!(result.exit_code, 0); + let next = bash.exec("echo recovered").await.unwrap(); + assert_eq!(next.stdout, "recovered\n"); +} diff --git a/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh b/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh index f54d81952..d644bdeac 100644 --- a/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/kill-self.test.sh @@ -38,3 +38,31 @@ echo alive ### expect alive ### end + +### nested_self_signals_return_to_each_handler +trap 'echo first; kill -USR2 $$; echo last' USR1 +trap 'echo second' USR2 +kill -USR1 $$ +kill -USR1 $$ +echo done +### expect +first +second +last +first +second +last +done +### end + +### ignored_self_signal_and_handler_exit +trap 'echo ignored' USR1 +trap '' USR1 +kill -USR1 $$ +trap 'echo caught; exit 7' USR2 +kill -USR2 $$ +echo unreachable +### expect +caught +### exit_code: 7 +### end diff --git a/docs/security.md b/docs/security.md index 69a8508ea..8ee66e1f0 100644 --- a/docs/security.md +++ b/docs/security.md @@ -107,7 +107,7 @@ Bashkit intentionally deviates from POSIX where compliance would compromise the sandbox. Key exclusions: - **`exec`**: would break sandbox containment (`TM-ESC-005`) -- **`trap`**: conflicts with the stateless execution model +- **`trap`**: handles shell events and self-sent virtual signals. Nested handlers share the function/source/child-shell depth budget, capped at 16; recursive signal delivery returns a bounded error - **Real process spawning**: all subprocess commands stay within the virtual interpreter (`TM-ESC-015`) These decisions are documented in [`knowledge/operations/limitations.md`](../knowledge/operations/limitations.md). diff --git a/knowledge/foundations/architecture.md b/knowledge/foundations/architecture.md index 1da1a48d6..f8025b46a 100644 --- a/knowledge/foundations/architecture.md +++ b/knowledge/foundations/architecture.md @@ -103,6 +103,21 @@ Recursive bare-variable evaluation and syntax parsing share the depth counter. Expansion errors propagate to the arithmetic boundary; later host executions receive fresh state. Regression and Bash differential coverage: `crates/bashkit/tests/integration/arithmetic_resource_tests.rs` (TM-DOS-026). +### Trap nesting + +Self-sent signals run handlers inline. Every handler entered through +`run_trap_command` charges the existing function-depth counter, also used by +functions, source, and child shells; its hard ceiling is 16. This preserves +finite nested signal delivery while bounding direct, mutual, and mixed cycles +without relying on command fuel or per-handler AST depth. The signal path boxes +the handler future to keep each native frame small; when the budget is +exhausted the nested handler is skipped with a diagnostic on the handler +stderr, and unparsable or failing handlers never disturb the interrupted +command (TM-DOS-030). Trap state and depth are restored on every path. DEBUG, +ERR, and RETURN retain their reentrancy suppression and existing error +handling. +See [Threat Model](../security/threat-model.md), TM-DOS-020, and +`signal_trap_depth_tests` plus CLI small-stack regression coverage. ### Shared execution budget diff --git a/knowledge/log.md b/knowledge/log.md index d350026fc..72dc1f1e7 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -19,6 +19,7 @@ finite nested expansions and literal `promptvars`-off behavior. Regression coverage includes cycles, substitutions, sibling budgets and session recovery; see [interactive shell](integrations/interactive-shell.md). +* **Fix**: self-signal trap handlers share the hard function/source depth ceiling (TM-DOS-020). Recursive direct/mutual/mixed delivery stops at the ceiling with a diagnostic instead of overflowing the stack, while handler failures still never disturb the interrupted command (TM-DOS-030). Bounded direct/mutual/mixed regressions and small-stack CLI subprocess probes cover the boundary. See [architecture](foundations/architecture.md#trap-nesting). * **Fix**: Oils long tail. `test`/`[` follow bash's `test.c` grammar (argument-count rules, `-a`/`-o`/`!`/`( )` precedence, `argument expected`, `)' expected`, `too many arguments`). `$empty cmd` runs `cmd`; `PWD`/`OLDPWD` are exported; `B=(b b) cmd` binds the text `(b b)` for `cmd` only. `rm` reports each bad operand and goes on; `cat` reads buffered stdin once; `date foo` fails; `umask 1 2` uses the first operand; `history -5` is an invalid option; `help help`, `help -- X` and bash's `no help topics match`; `set +` is ignored; `getopts` rejects a bad variable name and unsets `OPTARG` at the end. `shift 1 2` (like `exit`/`break`/`continue` with extra operands) discards the line, and a builtin's discard ends a child `bash -c` string (status 1) while expansion errors still resume. Arithmetic: `set -u` makes an unbound name fatal (also in `${#v}`/`${v:..}`), backticks and `\`-newline work inside, a base with a leading zero (`010#1`) is invalid, `a[$(cmd)]=v` and `${a[$(cmd)]}` run the substitution, `a[]=x` and `${a[]}` are bad subscripts. Array literals evaluate subscripts in order (`[i++]=x`, self references), skip tilde for assoc values, and refuse bare assoc elements; `a["k"]=v` and nameref targets `A["K"]`/`A[$key]` resolve quoted and variable keys; `declare -nx` exports. `printf %q` and `${x@Q}`/`@A` use `$'...'` for control characters; `$'\cX'` and invalid `\u` match bash; `${s:}` is a bad substitution; quotes nested in a double-quoted `${..}` operand stay protected; `{a,b}""` keeps brace expansion; `f() ...` with `$`/quotes in the name and `cmd (` are refused. `unset PS4` traces with no prefix; `time a | b` keeps its output. Oils spec pass rate 92.8% to 95.8% (79 cases, no new misses; `array-literal`, `array`, `blog2`, `bool-parse`, `builtin-misc`, `func-parsing`, `builtin-getopts`, `builtin-set`, `exit-status`, `var-op-slice` to 0); every remaining miss is categorized under limitations "Oils Spec Misses". * **Fix**: interactive builtins. `history` and `fc` keep bash's per-shell history: one entry per top-level input line (with `set -o history` / `bash -i`), `history -c -d N/-N/A-B -a -n -r -w -p -s`, `HISTSIZE`/`HISTFILESIZE` trims, `histappend`, listings in bash's format; `fc -l/-n/-r` ranges and prefixes, `fc -s old=new cmd` and `fc -e -` rerun (no editor, L-HIST-001). `HISTFILE` is read and written through the VFS only, reads are capped at `max_input_bytes` (TM-DOS-131); `history -c` still persists at once for the builder's history file. `complete`/`compgen`/`compopt` keep per-interpreter specs (at most 1024 specs and 64 KiB each, TM-DOS-132): every `-A` action, `-W` split at IFS with `$var` expansion, `-G`, `-F` (with `COMP_*` set and `COMPREPLY` read), `-C`, `-X` with `!` and `&`, `-P`/`-S`, `-o plusdirs/dirnames/default`, `complete -p`/`-r` in bash's format. `bind` lists bash's default emacs bindings, readline functions and variables and records changes per shell (capped, TM-DOS-132). `${x@P}` decodes prompt escapes (`\u \h \w \W \s \v \! \# \t \D{fmt}`, octal, `\[ \]`) from the sandbox identity and virtual clock, then expands like a here-doc body under `promptvars`. `bash -i` runs an interactive child: `$-` has `i`, `PS1`/`PS2`/`PS4`/`HISTFILE` defaults, `~/.bashrc` or `--rcfile`, `--norc`, and for a stdin script `PS1` printed to stderr plus `PROMPT_COMMAND` per line and `exit` at EOF. `hostname` takes `-s`/`-f`/`-d`. Oils spec pass rate 89.0% to 92.8% (`builtin-completion` 33 misses to 2, `prompt` 25 to 0, `builtin-history` and `builtin-fc` 12 to 0, `interactive` 11 to 0, `builtin-bind` 9 to 0); gaps in the limitations "Interactive builtins" row and L-HIST-001. diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index a6ff7f1e9..3e28a3df1 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -60,7 +60,7 @@ execution model. Evidence is a threat-model ID, a test, or `stance` | L-NET-002 | No DNS resolution; hosts must appear in the allowlist | Resolution would bypass allowlist intent | `l_net_002_default_deny_no_resolution` | | L-ARITH-001 | A variable's value read as an arithmetic expression or subscript never runs command substitution: `x='a[$(cmd)]'; echo $((x))` and `unset "a[$(cmd)]"` built from data are arithmetic syntax errors, and an indirect target holding `$(`, `` ` ``, `<(` or `>(` (`r='a[$(cmd)]'; ${!r}`) is `invalid variable name`, where bash runs `cmd` | Values are data; re-parsing them as code is the classic bash arithmetic injection, and a sandboxed agent shell must not execute text it only read | stance | | L-ARITH-002 | An arithmetic error names at most 256 bytes each of the expression and the "error token" (the unparsed rest), with a `...` marker where either was cut. An expression whose first rejected character sits near the front is echoed back twice over -- once whole, once from that character on -- so a ~500-byte one reported ~1,000 bytes before this cap, as bash still does. Expressions at or under the cap are byte-identical to bash | Both fragments come from the script, so one expression under `MAX_ARITHMETIC_EXPANSION_BYTES` rendered a diagnostic about twice its size, over the 1 KiB stderr budget every builtin is held to (TM-INF-022). Capping each fragment keeps the text that says what went wrong | TM-INF-022, `long_arithmetic_expansion_diagnostic_is_bounded`, `run_270_crash_input_is_bounded` | -| L-SIG-001 | No signal arrives from outside the sandbox, so a handler only runs for a signal the script sends itself (`kill -SIG $$`, plus EXIT, ERR and DEBUG). Without a handler, a self-sent signal ends the script with 128 + signal | No host signals exist inside the sandbox | `l_sig_001_signal_traps_not_delivered` | +| L-SIG-001 | No signal arrives from outside the sandbox, so a handler only runs for a signal the script sends itself (`kill -SIG $$`, plus EXIT, ERR and DEBUG). Without a handler, a self-sent signal ends the script with 128 + signal. Nested handlers share `max_function_depth` with functions/source/child shells (hard ceiling 16); recursive delivery past the ceiling is skipped with a diagnostic on the handler's stderr and the first trip fails the exec with a resource-limit error | No host signals exist inside the sandbox | `l_sig_001_signal_traps_not_delivered` | | L-SSH-001 | CA-signed SSH host *certificates* are rejected under `strict_host_key_checking` (default), even when the public key they wrap is a configured trusted key; configure the host's public key directly | No CA trust store exists to validate the signature chain, validity window, principals or critical options; matching the embedded key would grant trust never actually verified | TM-SSH-006, `test_strict_rejects_certificate_even_when_inner_key_is_trusted` | | L-WASM-001 | **Removed:** JS-host timers now drive `sleep`, builtin `timeout`, execution limits, and tool `timeoutMs` | `gloo-timers` bridges the host event-loop clock without threads or cross-origin isolation | [Browser Package](../runtimes/browser-package.md) | | L-WASM-002 | Browser build: `executeSync()` cannot run async custom builtins (fails with a clear message); use `execute()` | Single-threaded event loop can't settle a JS `Promise` without yielding | `crates/bashkit-wasm/__test__/bashkit-wasm.test.mjs` | diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index b3230a31d..76ff71d0c 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -237,7 +237,7 @@ runaway scripts without permanently breaking the session. | ID | Threat | Attack Vector | Mitigation | Status | |----|--------|--------------|------------|--------| -| TM-DOS-020 | Function recursion | `f() { f; }; f` | Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). The CLI subprocess test runs direct and mutual recursion on that stack size. | **MITIGATED** | +| TM-DOS-020 | Function and signal-trap recursion | `f() { f; }; f` or `trap 'kill -USR1 $$' USR1; kill -USR1 $$` | Signal handlers, functions, source, and child shells share the function-depth counter, plus a dedicated trap-nesting cap of 3 that trips first on small stacks; direct, mutual, and mixed signal recursion cannot refresh either. Nested-trap exhaustion is skipped with a diagnostic and the first trip fails the exec as a resource-limit error, while handler logic failures never disturb the interrupted command (TM-DOS-030). Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). CLI subprocess tests run direct, mutual, and mixed signal/function/source recursion on that stack size with small command budgets. | **MITIGATED** | | TM-DOS-021 | Command sub nesting | `$($($($())))` | Child parsers inherit remaining depth budget + fuel from parent | **MITIGATED** | | TM-DOS-022 | Parser recursion | Deeply nested `(((())))` | `max_ast_depth` limit (100) + `HARD_MAX_AST_DEPTH` cap (100) | **MITIGATED** | | TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))`, nested `${a[${a[...]}]}` passed to `let` | `MAX_ARITHMETIC_DEPTH` limit (50) checked before dollar expansion and tokenization; shared fuel across textual expansion, nested read-only subscripts, nameref/key lookup, and recursive variable evaluation; expanded-size cap | **MITIGATED** | From 834f9fc2a89d254ba33e844aa2e3a2c180582632 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:55:29 -0500 Subject: [PATCH 5/6] fix(interpreter): drop redundant unit tail flagged by all-features clippy --- crates/bashkit/src/interpreter/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index bf410faca..3d480aa00 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -12524,7 +12524,6 @@ impl Interpreter { self.last_exit_code = status; result.exit_code = status; result.control_flow = ControlFlow::Exit(status); - () } /// Resolve a path relative to cwd, normalizing `.` and `..` components. From ed20af1234193586bcaefaf8d803eca0485a6663 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 02:37:35 -0500 Subject: [PATCH 6/6] fix(tests): route trap probes through fail-closed launcher, pin admission boundary Address review on the signal-trap depth fix: CLI hostile-recursion probes now enforce stack (2 MiB), CPU, output, and wall bounds during execution with concurrent capped readers, bounded reap, and preserved timeout/oversize/incomplete-reader state; an abort can never pass. In-process tests pin the exact admission boundary (three direct handlers via the dedicated cap, two at max_function_depth(2)). Correct the stale module comment and document both limits. --- crates/bashkit-cli/tests/cli_oneshot.rs | 216 +++++++++++++++--- crates/bashkit/src/interpreter/mod.rs | 6 +- .../integration/signal_trap_depth_tests.rs | 32 +++ docs/security.md | 2 +- 4 files changed, 218 insertions(+), 38 deletions(-) diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index 36a798994..324da9446 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -15,8 +15,9 @@ // `knowledge/operations/limitations.md`, so lifting one has to update the row // and the test together. -use std::io::Write; +use std::io::{Read, Write}; use std::process::{Command, Output, Stdio}; +use std::sync::mpsc; use std::thread; use std::time::{Duration, Instant}; @@ -148,45 +149,190 @@ fn function_recursion_is_bounded_on_a_two_mib_stack() { } } -// Child isolation and a small command allowance keep even a missing guard safe. +// Decision: hostile-recursion probes run through this fail-closed launcher, +// not plain `.output()`. It enforces stack (2 MiB), CPU (10 s), output +// (64 KiB per stream), and wall (20 s) bounds *during* execution: reader +// threads drain both pipes concurrently with byte caps, the poll loop kills +// on oversize/timeout, reap and reader completion are bounded, and timeout / +// oversize / incomplete-reader state is preserved on the result instead of +// hanging or OOMing the parent. A child abort surfaces as its real exit +// status, never as a passing test. +#[cfg(unix)] +struct ProbeRun { + exit_code: Option, + signaled: bool, + timed_out: bool, + oversized: bool, + incomplete_readers: bool, + limits_note: String, + stdout: Vec, + stderr: Vec, +} + +#[cfg(unix)] +fn run_bounded_probe(script: &str) -> ProbeRun { + const STREAM_CAP: usize = 64 * 1024; + let mut child = Command::new("sh") + .args([ + "-c", + "ulimit -s 2048 || exit 99; ulimit -t 10 || exit 99; echo \"harness-limits: stack=$(ulimit -s) cpu=$(ulimit -t)\" >&2; exec \"$1\" --no-stdin --max-commands 64 -c \"$2\"", + "sh", + BIN, + script, + ]) + .env_remove("RUST_BACKTRACE") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn signal trap probe"); + let stdout_pipe: Box = Box::new(child.stdout.take().expect("probe stdout")); + let stderr_pipe: Box = Box::new(child.stderr.take().expect("probe stderr")); + // Concurrent capped readers: a hostile child must neither fill a pipe + // (parent/child deadlock) nor exhaust parent memory (unbounded output). + // A reader stops at the cap; the wall watchdog then reaps the child and + // the oversize flag fails the test. + let (tx, rx) = mpsc::channel::<(bool, Vec, bool)>(); + for (pipe, is_stdout) in [(stdout_pipe, true), (stderr_pipe, false)] { + let tx = tx.clone(); + thread::spawn(move || { + let mut pipe = pipe; + let mut buf = Vec::new(); + let mut chunk = [0u8; 8192]; + let mut oversized = false; + loop { + if buf.len() >= STREAM_CAP { + oversized = true; + break; + } + match pipe.read(&mut chunk) { + Ok(0) => break, + Ok(n) => buf.extend_from_slice(&chunk[..n]), + Err(_) => break, + } + } + let _ = tx.send((is_stdout, buf, oversized)); + }); + } + drop(tx); + let start = Instant::now(); + let mut timed_out = false; + loop { + match child.try_wait().expect("poll signal trap probe") { + Some(_) => break, + None if start.elapsed() >= Duration::from_secs(20) => { + timed_out = true; + let _ = child.kill(); + break; + } + None => thread::sleep(Duration::from_millis(50)), + } + } + // Bounded reap: a kill that does not land still ends the test instead + // of hanging the suite. Capture the real exit status here. + let status = child.wait().expect("reap signal trap probe"); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut oversized = false; + let mut incomplete_readers = false; + for _ in 0..2 { + match rx.recv_timeout(Duration::from_secs(5)) { + Ok((true, buf, over)) => { + stdout = buf; + oversized |= over; + } + Ok((false, buf, over)) => { + stderr = buf; + oversized |= over; + } + Err(_) => incomplete_readers = true, + } + } + use std::os::unix::process::ExitStatusExt; + let stderr_text = String::from_utf8_lossy(&stderr).into_owned(); + let limits_note = stderr_text + .lines() + .find(|line| line.starts_with("harness-limits:")) + .unwrap_or("") + .to_owned(); + ProbeRun { + exit_code: status.code(), + signaled: status.signal().is_some(), + timed_out, + oversized, + incomplete_readers, + limits_note, + stdout, + stderr, + } +} + +#[cfg(unix)] +fn dump_probe_logs(case: &str, run: &ProbeRun) -> (String, String) { + let safe: String = case + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' }) + .take(32) + .collect(); + let dir = std::env::temp_dir(); + let out_path = dir.join(format!("bashkit-signal-probe-{safe}.stdout")); + let err_path = dir.join(format!("bashkit-signal-probe-{safe}.stderr")); + let _ = std::fs::write(&out_path, &run.stdout); + let _ = std::fs::write(&err_path, &run.stderr); + ( + out_path.display().to_string(), + err_path.display().to_string(), + ) +} + +// Signal-trap self-delivery (`kill -USR1 $$` from inside a USR1 handler) +// recurses through the interpreter instead of returning to the host loop. +// The dedicated nesting cap plus the shared depth budget stop +// direct/mutual/function/source cycles with a resource-limit error instead +// of overflowing the child stack, and finite nesting keeps working. Each +// hostile shape runs in a fail-closed child (2 MiB stack, CPU/wall/output +// caps); an abort is never a passing test. See TM-DOS-020 and +// `signal_trap_depth_tests`. #[cfg(unix)] #[test] fn signal_trap_recursion_is_bounded_on_a_two_mib_stack() { - for script in [ - "trap 'kill -USR1 $$' USR1; kill -USR1 $$", - "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", - "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", - "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + for (case, script) in [ + ("direct", "trap 'kill -USR1 $$' USR1; kill -USR1 $$"), + ( + "mutual", + "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + ), + ( + "function", + "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + ), + ( + "source", + "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + ), ] { - let mut child = Command::new("sh") - .args([ - "-c", - "set -e; ulimit -s 2048; exec \"$1\" --no-stdin --max-commands 64 -c \"$2\"", - "sh", - BIN, - script, - ]) - .env_remove("RUST_BACKTRACE") - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .expect("spawn signal trap probe"); - let deadline = Instant::now() + Duration::from_secs(10); - while child.try_wait().unwrap().is_none() { - if Instant::now() >= deadline { - child.kill().unwrap(); - let out = child.wait_with_output().unwrap(); - panic!("signal trap probe hung: {script}: {}", stderr(&out)); - } - thread::sleep(Duration::from_millis(10)); + let run = run_bounded_probe(script); + if run.timed_out + || run.oversized + || run.incomplete_readers + || run.signaled + || run.exit_code != Some(1) + || !run.limits_note.contains("stack=2048") + || !run.limits_note.contains("cpu=10") + || !String::from_utf8_lossy(&run.stderr).contains("depth exceeded") + { + let (out_path, err_path) = dump_probe_logs(case, &run); + panic!( + "{case}: fail-closed probe violated: exit={:?} signaled={} timed_out={} oversized={} incomplete_readers={} limits={:?} stdout={:?} stderr={:?} logs={out_path} {err_path}", + run.exit_code, + run.signaled, + run.timed_out, + run.oversized, + run.incomplete_readers, + run.limits_note, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr), + ); } - let out = child.wait_with_output().unwrap(); - assert_eq!(out.status.code(), Some(1), "{script}: {}", stderr(&out)); - assert!( - stderr(&out).contains("depth exceeded"), - "{script}: {}", - stderr(&out) - ); } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 3d480aa00..b80d96940 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -30,8 +30,10 @@ // sandbox "Did you mean" / unavailable-command hints after // `command not found` are kept on purpose. -// Decision: trap handlers share the function/source depth budget. Signal delivery -// propagates handler failures; swallowing a nested limit error reports success. +// Decision: trap handlers share the function/source depth budget plus a +// dedicated trap-nesting cap. Nested delivery skips an exhausted handler +// with a diagnostic; handler logic failures never disturb the interrupted +// command (TM-DOS-030), while the first resource-limit trip fails the exec. mod arithmetic; mod brace_expansion; mod completion; diff --git a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs index 5d09af2b2..67a3ac5cb 100644 --- a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs +++ b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs @@ -84,3 +84,35 @@ async fn signal_handler_parse_error_is_swallowed() { let next = bash.exec("echo recovered").await.unwrap(); assert_eq!(next.stdout, "recovered\n"); } + +#[tokio::test] +async fn trap_nesting_admits_exactly_three_direct_handlers() { + // Admission boundary: the dedicated cap (3) admits three nested handler + // executions, then trips the fourth entry with MaxTrapDepth. + let mut bash = Bash::builder().build(); + let err = bash + .exec("trap 'echo ENTRY >> /log; kill -USR1 $$' USR1; kill -USR1 $$") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("maximum trap nesting depth exceeded"), "{err}"); + let log = bash.exec("cat /log").await.unwrap(); + assert_eq!(log.stdout.lines().count(), 3, "{}", log.stdout); +} + +#[tokio::test] +async fn shared_depth_admits_exactly_two_handlers_at_depth_two() { + // Admission boundary: with max_function_depth(2), two handler executions + // are admitted before the shared budget trips the third entry. + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(2)) + .build(); + let err = bash + .exec("trap 'echo ENTRY >> /log; kill -USR1 $$' USR1; kill -USR1 $$") + .await + .unwrap_err() + .to_string(); + assert!(err.contains("maximum function depth exceeded"), "{err}"); + let log = bash.exec("cat /log").await.unwrap(); + assert_eq!(log.stdout.lines().count(), 2, "{}", log.stdout); +} diff --git a/docs/security.md b/docs/security.md index 8ee66e1f0..bfcbecc00 100644 --- a/docs/security.md +++ b/docs/security.md @@ -107,7 +107,7 @@ Bashkit intentionally deviates from POSIX where compliance would compromise the sandbox. Key exclusions: - **`exec`**: would break sandbox containment (`TM-ESC-005`) -- **`trap`**: handles shell events and self-sent virtual signals. Nested handlers share the function/source/child-shell depth budget, capped at 16; recursive signal delivery returns a bounded error +- **`trap`**: handles shell events and self-sent virtual signals. Nested handlers share the function/source/child-shell depth budget (hard ceiling 16) plus a dedicated trap-nesting cap of 3 that trips first on small stacks; recursive signal delivery past either limit is skipped with a diagnostic and the first trip fails the exec with a resource-limit error - **Real process spawning**: all subprocess commands stay within the virtual interpreter (`TM-ESC-015`) These decisions are documented in [`knowledge/operations/limitations.md`](../knowledge/operations/limitations.md).