From 7bbf7115b91d567228146c83274581a49ca62d50 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:31:18 -0500 Subject: [PATCH 1/7] 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/7] 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 b84bd19372dc0be02a9f886233b89a5c92e596ac Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 01:31:18 -0500 Subject: [PATCH 3/7] 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 4/7] 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 5/7] 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). From a286683aced4bc3ef7cc1cba19b2bb71645929e2 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 03:41:40 -0500 Subject: [PATCH 6/7] fix(tests): fail-closed bounded probe launcher with honest lifecycle The merged run_bounded_probe waited unboundedly after its deadline, never reported oversize/read errors while the child ran, extended whole chunks before checking the cap, misclassified exactly-cap EOF, treated read errors as EOF, and started fresh waits past the deadline. - One absolute deadline (documented execution plus cleanup grace); bounded try_wait reaping; truthful cleanup_failed/incomplete flags - Live oversize/read-error reporting; Interrupted retry; cap-before-copy with a bounded excess sentinel; exact-cap EOF admitted - Bounded self-tests: exact/excess/error/Interrupted reader cores plus overflow, timeout/kill/reap, incomplete-pipe, and fast-exit children - Trap probes assert exact stdout bytes, exact limits readback, and all failure flags; hostile entry counts run in bounded subprocesses - Finite exactly-3 nesting success plus shared depth 0/1/2 boundaries --- crates/bashkit-cli/tests/cli_oneshot.rs | 512 +++++++++++++++--- .../integration/signal_trap_depth_tests.rs | 48 +- 2 files changed, 477 insertions(+), 83 deletions(-) diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index 324da9446..b866496a9 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -150,104 +150,263 @@ fn function_recursion_is_bounded_on_a_two_mib_stack() { } // 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. +// not plain `.output()`. One absolute deadline covers execution plus a +// documented cleanup grace (20 s execution + 5 s grace for trap probes; the +// suite never claims a strict 20 s bound): reader threads drain both pipes +// concurrently with byte caps, oversize and read errors are reported live +// while the child runs, the poll loop kills on oversize/timeout, and reaping +// plus reader completion are bounded by the remaining grace. Unfinished +// cleanup or readers yield truthful failure flags instead of hanging. 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, + read_error: bool, incomplete_readers: bool, + cleanup_failed: 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, - ]) +struct ProbeLimits { + exec_secs: u64, + grace_secs: u64, + stream_cap: usize, +} + +#[cfg(unix)] +const TRAP_PROBE_LIMITS: ProbeLimits = ProbeLimits { + exec_secs: 20, + grace_secs: 5, + stream_cap: 64 * 1024, +}; + +#[cfg(unix)] +enum ReaderLive { + Oversize, + ReadError, +} + +#[cfg(unix)] +enum ReaderMsg { + Live(ReaderLive), + Finished { + stream_stdout: bool, + buf: Vec, + oversized: bool, + read_error: bool, + }, +} + +/// Drain a pipe keeping at most `cap` bytes. Returns the retained bytes, +/// whether more input existed, and any genuine read error. `Interrupted` is +/// retried; any other error stops the reader and is reported instead of +/// being mistaken for EOF. Exactly-at-cap EOF is admitted, not oversized: a +/// single-byte sentinel decides, and the sentinel byte itself is discarded +/// so no more than `cap` bytes are ever retained. `live` reports oversize +/// and read errors the moment they are proven, while the child may still run. +#[cfg(unix)] +fn drain_capped( + pipe: &mut dyn Read, + cap: usize, + mut live: impl FnMut(ReaderLive), +) -> (Vec, bool, bool) { + let mut buf = Vec::new(); + let mut chunk = [0u8; 8192]; + loop { + if buf.len() >= cap { + let mut sentinel = [0u8; 1]; + match pipe.read(&mut sentinel) { + Ok(0) => return (buf, false, false), + Ok(_) => { + live(ReaderLive::Oversize); + return (buf, true, false); + } + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(_) => { + live(ReaderLive::ReadError); + return (buf, false, true); + } + } + } + // Cap-before-copy: never ask for more than the remaining allowance. + let allowance = (cap - buf.len()).min(chunk.len()); + match pipe.read(&mut chunk[..allowance]) { + Ok(0) => return (buf, false, false), + Ok(n) => buf.extend_from_slice(&chunk[..n]), + Err(e) if e.kind() == std::io::ErrorKind::Interrupted => continue, + Err(_) => { + live(ReaderLive::ReadError); + return (buf, false, true); + } + } + } +} + +#[cfg(unix)] +fn spawn_pipe_reader( + pipe: Box, + is_stdout: bool, + cap: usize, + tx: mpsc::Sender, +) { + thread::spawn(move || { + let mut pipe = pipe; + let (buf, oversized, read_error) = drain_capped(&mut *pipe, cap, |live| { + let _ = tx.send(ReaderMsg::Live(live)); + }); + let _ = tx.send(ReaderMsg::Finished { + stream_stdout: is_stdout, + buf, + oversized, + read_error, + }); + }); +} + +#[cfg(unix)] +fn run_bounded_child(argv: &[&str], limits: &ProbeLimits) -> ProbeRun { + let start = Instant::now(); + let exec_deadline = start + Duration::from_secs(limits.exec_secs); + let absolute_deadline = exec_deadline + Duration::from_secs(limits.grace_secs); + let mut cmd = Command::new(argv[0]); + cmd.args(&argv[1..]); + let mut child = cmd .env_remove("RUST_BACKTRACE") .stdout(Stdio::piped()) .stderr(Stdio::piped()) .spawn() - .expect("spawn signal trap probe"); + .expect("spawn bounded probe child"); 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 { + let (tx, rx) = mpsc::channel::(); + spawn_pipe_reader(stdout_pipe, true, limits.stream_cap, tx.clone()); + spawn_pipe_reader(stderr_pipe, false, limits.stream_cap, tx.clone()); + drop(tx); + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut timed_out = false; + let mut oversized = false; + let mut read_error = false; + let mut exit_status = None; + let mut killed = false; + let mut finals = 0; + let kill = |child: &mut std::process::Child, killed: &mut bool| { + if !*killed { + *killed = true; + let _ = child.kill(); + } + }; + // Execution phase: reap promptly on exit, kill on live oversize, error, + // or exec-deadline expiry. The timeout decision is recorded before the + // kill so cleanup outcome cannot rewrite it. + loop { + while let Ok(msg) = rx.try_recv() { + match msg { + ReaderMsg::Live(ReaderLive::Oversize) => { oversized = true; - break; + kill(&mut child, &mut killed); + } + ReaderMsg::Live(ReaderLive::ReadError) => { + read_error = true; + kill(&mut child, &mut killed); } - match pipe.read(&mut chunk) { - Ok(0) => break, - Ok(n) => buf.extend_from_slice(&chunk[..n]), - Err(_) => break, + ReaderMsg::Finished { + stream_stdout, + buf, + oversized: over, + read_error: err, + } => { + if stream_stdout { + stdout = buf; + } else { + stderr = buf; + } + oversized |= over; + read_error |= err; + finals += 1; } } - let _ = tx.send((is_stdout, buf, oversized)); - }); + } + match child.try_wait() { + Ok(Some(status)) => { + exit_status = Some(status); + break; + } + Ok(None) => {} + Err(_) => break, + } + if Instant::now() >= exec_deadline { + timed_out = true; + kill(&mut child, &mut killed); + break; + } + thread::sleep(Duration::from_millis(20)); } - 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(); + // Cleanup phase: bounded reap, then bounded reader completion. No fresh + // waits start past the absolute deadline; whatever is unfinished at that + // point is reported truthfully instead of hanging the suite. + let mut cleanup_failed = false; + while exit_status.is_none() { + match child.try_wait() { + Ok(Some(status)) => { + exit_status = Some(status); + break; + } + Ok(None) => { + if Instant::now() >= absolute_deadline { + cleanup_failed = true; + let _ = child.kill(); + break; + } + thread::sleep(Duration::from_millis(20)); + } + Err(_) => { + cleanup_failed = true; 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; + while finals < 2 { + let remaining = absolute_deadline.saturating_duration_since(Instant::now()); + match rx.recv_timeout(remaining) { + Ok(ReaderMsg::Finished { + stream_stdout, + buf, + oversized: over, + read_error: err, + }) => { + if stream_stdout { + stdout = buf; + } else { + stderr = buf; + } oversized |= over; + read_error |= err; + finals += 1; } - Ok((false, buf, over)) => { - stderr = buf; - oversized |= over; + Ok(ReaderMsg::Live(ReaderLive::Oversize)) => oversized = true, + Ok(ReaderMsg::Live(ReaderLive::ReadError)) => read_error = true, + // Timeout: readers did not finish inside the absolute deadline. + // Disconnected: a reader died without its final message. Both + // are reported truthfully instead of hanging the suite. + Err(_) => { + incomplete_readers = true; + break; } - Err(_) => incomplete_readers = true, } } use std::os::unix::process::ExitStatusExt; + let (exit_code, signaled) = match exit_status { + Some(status) => (status.code(), status.signal().is_some()), + None => (None, false), + }; let stderr_text = String::from_utf8_lossy(&stderr).into_owned(); let limits_note = stderr_text .lines() @@ -255,17 +414,34 @@ fn run_bounded_probe(script: &str) -> ProbeRun { .unwrap_or("") .to_owned(); ProbeRun { - exit_code: status.code(), - signaled: status.signal().is_some(), + exit_code, + signaled, timed_out, oversized, + read_error, incomplete_readers, + cleanup_failed, limits_note, stdout, stderr, } } +#[cfg(unix)] +fn run_bounded_probe(script: &str) -> ProbeRun { + run_bounded_child( + &[ + "sh", + "-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, + ], + &TRAP_PROBE_LIMITS, + ) +} + #[cfg(unix)] fn dump_probe_logs(case: &str, run: &ProbeRun) -> (String, String) { let safe: String = case @@ -296,28 +472,35 @@ fn dump_probe_logs(case: &str, run: &ProbeRun) -> (String, String) { #[test] fn signal_trap_recursion_is_bounded_on_a_two_mib_stack() { for (case, script) in [ - ("direct", "trap 'kill -USR1 $$' USR1; kill -USR1 $$"), + ( + "direct", + "trap 'echo ENTRY; kill -USR1 $$' USR1; kill -USR1 $$", + ), ( "mutual", - "trap 'kill -USR2 $$' USR1; trap 'kill -USR1 $$' USR2; kill -USR1 $$", + "trap 'echo ENTRY; kill -USR2 $$' USR1; trap 'echo ENTRY; kill -USR1 $$' USR2; kill -USR1 $$", ), ( "function", - "f() { kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", + "f() { echo ENTRY; kill -USR1 $$; }; trap 'f' USR1; kill -USR1 $$", ), ( "source", - "echo 'kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", + "echo 'echo ENTRY; kill -USR1 $$' > /handler; trap 'source /handler' USR1; kill -USR1 $$", ), ] { let run = run_bounded_probe(script); + // The dedicated cap admits exactly three handler executions, then + // trips the fourth entry: stdout is exactly three ENTRY lines. if run.timed_out || run.oversized + || run.read_error || run.incomplete_readers + || run.cleanup_failed || run.signaled || run.exit_code != Some(1) - || !run.limits_note.contains("stack=2048") - || !run.limits_note.contains("cpu=10") + || run.limits_note != "harness-limits: stack=2048 cpu=10" + || run.stdout != b"ENTRY\nENTRY\nENTRY\n" || !String::from_utf8_lossy(&run.stderr).contains("depth exceeded") { let (out_path, err_path) = dump_probe_logs(case, &run); @@ -336,6 +519,193 @@ fn signal_trap_recursion_is_bounded_on_a_two_mib_stack() { } } +// Bounded-launcher self-tests: force every lifecycle path deterministically. +// Reader-core tests use in-memory readers (exact cap, excess, genuine +// errors, Interrupted); launcher tests use tiny real children with short +// explicit bounds (overflow, timeout/kill/reap, incomplete pipes, fast exit). +#[cfg(unix)] +struct FailAfter { + data: Vec, + fail_at: usize, + pos: usize, +} + +#[cfg(unix)] +impl Read for FailAfter { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.pos >= self.fail_at { + return Err(std::io::Error::other("boom")); + } + let n = (self.fail_at - self.pos) + .min(buf.len()) + .min(self.data.len() - self.pos); + buf[..n].copy_from_slice(&self.data[self.pos..self.pos + n]); + self.pos += n; + Ok(n) + } +} + +#[cfg(unix)] +struct Flaky { + interrupts: usize, + data: Vec, + pos: usize, +} + +#[cfg(unix)] +impl Read for Flaky { + fn read(&mut self, buf: &mut [u8]) -> std::io::Result { + if self.interrupts > 0 { + self.interrupts -= 1; + return Err(std::io::Error::new( + std::io::ErrorKind::Interrupted, + "retry", + )); + } + if self.pos >= self.data.len() { + return Ok(0); + } + let n = buf.len().min(self.data.len() - self.pos); + buf[..n].copy_from_slice(&self.data[self.pos..self.pos + n]); + self.pos += n; + Ok(n) + } +} + +#[cfg(unix)] +#[test] +fn capped_reader_admits_exact_cap_eof() { + let mut cursor = std::io::Cursor::new(vec![b'x'; 1024]); + let live: Vec = Vec::new(); + let (buf, oversized, read_error) = drain_capped(&mut cursor, 1024, |_| {}); + assert_eq!(buf.len(), 1024); + assert!(!oversized); + assert!(!read_error); + assert!(live.is_empty()); +} + +#[cfg(unix)] +#[test] +fn capped_reader_flags_one_byte_excess_live() { + let mut cursor = std::io::Cursor::new(vec![b'x'; 1025]); + let mut live_oversize = false; + let (buf, oversized, read_error) = drain_capped(&mut cursor, 1024, |ev| { + if matches!(ev, ReaderLive::Oversize) { + live_oversize = true; + } + }); + // Exactly the cap is retained; the sentinel byte proves excess. + assert_eq!(buf.len(), 1024); + assert!(oversized); + assert!(!read_error); + assert!(live_oversize); +} + +#[cfg(unix)] +#[test] +fn capped_reader_reports_genuine_error_not_eof() { + let mut pipe = FailAfter { + data: vec![b'y'; 100], + fail_at: 10, + pos: 0, + }; + let mut live_error = false; + let (buf, oversized, read_error) = drain_capped(&mut pipe, 1024, |ev| { + if matches!(ev, ReaderLive::ReadError) { + live_error = true; + } + }); + assert_eq!(buf.len(), 10); + assert!(!oversized); + assert!(read_error); + assert!(live_error); +} + +#[cfg(unix)] +#[test] +fn capped_reader_retries_interrupted() { + let mut pipe = Flaky { + interrupts: 2, + data: vec![b'z'; 50], + pos: 0, + }; + let (buf, oversized, read_error) = drain_capped(&mut pipe, 1024, |_| {}); + assert_eq!(buf.len(), 50); + assert!(!oversized); + assert!(!read_error); +} + +#[cfg(unix)] +#[test] +fn launcher_flags_output_overflow_with_cap_retained() { + let limits = ProbeLimits { + exec_secs: 10, + grace_secs: 5, + stream_cap: 4096, + }; + let run = run_bounded_child(&["sh", "-c", "head -c 200000 /dev/zero"], &limits); + assert!(run.oversized, "overflow must be flagged"); + assert_eq!(run.stdout.len(), 4096, "retain exactly the cap"); + assert!(!run.timed_out); + assert!(!run.read_error); + assert!(!run.cleanup_failed); + assert!(!run.incomplete_readers); +} + +#[cfg(unix)] +#[test] +fn launcher_times_out_kills_and_reaps() { + let limits = ProbeLimits { + exec_secs: 1, + grace_secs: 3, + stream_cap: 4096, + }; + let run = run_bounded_child(&["sh", "-c", "sleep 30"], &limits); + assert!(run.timed_out, "wall deadline must fire"); + assert!(run.signaled, "SIGKILL must land for the reap proof"); + assert_eq!(run.exit_code, None, "signal death has no exit code"); + assert!(!run.oversized); + assert!(!run.cleanup_failed, "kill plus reap must complete"); + assert!(!run.incomplete_readers, "pipes close after reap"); +} + +#[cfg(unix)] +#[test] +fn launcher_reports_incomplete_pipes_truthfully() { + // A grandchild inherits stdout and outlives the grace period, so the + // direct child reaps cleanly but readers cannot finish. + let limits = ProbeLimits { + exec_secs: 1, + grace_secs: 2, + stream_cap: 4096, + }; + let run = run_bounded_child(&["sh", "-c", "(sleep 8 &)"], &limits); + assert!(!run.timed_out, "direct child exits immediately"); + assert!(!run.cleanup_failed, "direct reap succeeds"); + assert_eq!(run.exit_code, Some(0)); + assert!( + run.incomplete_readers, + "open grandchild pipes must be reported, not hung on" + ); +} + +#[cfg(unix)] +#[test] +fn launcher_reaps_fast_child_with_real_exit() { + let limits = ProbeLimits { + exec_secs: 10, + grace_secs: 5, + stream_cap: 4096, + }; + let run = run_bounded_child(&["sh", "-c", "exit 3"], &limits); + assert_eq!(run.exit_code, Some(3), "raw child exit is distinct"); + assert!(!run.signaled); + assert!(!run.timed_out); + assert!(!run.oversized); + assert!(!run.read_error); + assert!(!run.cleanup_failed); + assert!(!run.incomplete_readers); +} #[cfg(unix)] #[test] fn function_nesting_below_limit_works_on_a_two_mib_stack() { diff --git a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs index 67a3ac5cb..1c5547733 100644 --- a/crates/bashkit/tests/integration/signal_trap_depth_tests.rs +++ b/crates/bashkit/tests/integration/signal_trap_depth_tests.rs @@ -86,33 +86,57 @@ async fn signal_handler_parse_error_is_swallowed() { } #[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. +async fn finite_three_level_nesting_succeeds_with_recovery() { + // Terminates by construction (acyclic signal chain USR1 -> USR2 -> USR3): + // three nested handler entries are admitted, with exact output. let mut bash = Bash::builder().build(); + let result = bash + .exec("trap 'echo one; kill -USR2 $$' USR1; trap 'echo two; kill -TERM $$' USR2; trap 'echo three' TERM; kill -USR1 $$; echo done") + .await + .unwrap(); + assert_eq!(result.stdout, "one\ntwo\nthree\ndone\n"); + assert_eq!(result.exit_code, 0); + let next = bash.exec("echo recovered").await.unwrap(); + assert_eq!(next.stdout, "recovered\n"); +} + +#[tokio::test] +async fn shared_depth_zero_admits_no_handlers() { + // Bounded by construction: the first trap entry already exceeds a zero + // budget, so no handler body runs and the trip surfaces. + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_function_depth(0)) + .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); + assert!(err.contains("depth exceeded"), "{err}"); + let log = bash + .exec("cat /log 2>/dev/null || echo none") + .await + .unwrap(); + assert!( + log.stdout.contains("none"), + "no handler may run: {}", + 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. +async fn shared_depth_one_admits_exactly_one_handler() { + // Bounded by construction: each cycle consumes depth; the second entry + // trips, so exactly one handler body runs. let mut bash = Bash::builder() - .limits(ExecutionLimits::new().max_function_depth(2)) + .limits(ExecutionLimits::new().max_function_depth(1)) .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}"); + assert!(err.contains("depth exceeded"), "{err}"); let log = bash.exec("cat /log").await.unwrap(); - assert_eq!(log.stdout.lines().count(), 2, "{}", log.stdout); + assert_eq!(log.stdout.lines().count(), 1, "{}", log.stdout); } From 2f2561fe956cc043cf192b53808ae1ff59fc2835 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 03:59:08 -0500 Subject: [PATCH 7/7] fix(tests): force exec in timeout self-test for cross-platform reap --- crates/bashkit-cli/tests/cli_oneshot.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index b866496a9..1353193ac 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -660,7 +660,11 @@ fn launcher_times_out_kills_and_reaps() { grace_secs: 3, stream_cap: 4096, }; - let run = run_bounded_child(&["sh", "-c", "sleep 30"], &limits); + // `exec` forces no-fork on every platform: the kill reaps the only + // process and the pipes close, so this exercises timeout plus reap. + // (A forked grandchild would hold the pipes open; that orphan case is + // covered deliberately by launcher_reports_incomplete_pipes.) + let run = run_bounded_child(&["sh", "-c", "exec sleep 30"], &limits); assert!(run.timed_out, "wall deadline must fire"); assert!(run.signaled, "SIGKILL must land for the reap proof"); assert_eq!(run.exit_code, None, "signal death has no exit code");