Repository navigation
fix(interpreter): bound re-entrant signal-trap delivery depth - #2679
Conversation
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
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 9967065 | Commit Preview URL Branch Preview URL |
Oct 11 2026, 07:40 AM |
Required review before shipping PR #2679Reviewed head 7bbf711. Reuse this PR, branch, session and reserved checkout. Do not stop at green checks or merge before resolving all issue comments and formal review threads.
Required CI is not green: Lint failed on the reviewed head; Linux Test/Coverage and other checks were still pending. Pending builds are not stalls, and watcher |
## What changed
Tilde expansion (`~user`, `x=~`, `x=~:~`, `${u-~}`) now charges the
shared execution budget. Previously `tilde_expand_literal` built results
with an unbudgeted `String`, so a script stacking tildes could grow
output without ever tripping budget limits.
## Why
Execution-budget E5 requires every interpreter allocation site to charge
budget. Unbudgeted tilde expansion is a resource-exhaustion hole
(TM-DOS-134): each `:` operand re-resolves the home directory for free.
## Before / After
New regression tests fail on the old code (5 of 7 fail without the fix;
the 2 parity tests pass both ways) and pass with it:
- `cargo test -p bashkit --test integration execution_budget`: 32/32
pass
- CLI smoke: `echo ~; x=~; echo "$x"; echo ~root; echo ~/sub; v=${u-~};
echo "$v"` expands correctly (sandbox homes by design)
- Differential check: same script under real bash expands identically in
structure
## Risk
- Low. Fail-closed accounting only; expansion results unchanged (covered
by a bash-parity test). No API changes outside the interpreter.
## Checklist
- [x] Tests added or updated (7 tilde budget tests: exhaustion,
boundary, leases, parity)
- [x] Backward compatibility considered (internal code; no compat
needed)
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
…sion 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.
…t rewriting published commits # Conflicts: # crates/bashkit-cli/tests/cli_oneshot.rs # crates/bashkit/src/interpreter/mod.rs # crates/bashkit/tests/integration/signal_trap_depth_tests.rs # docs/security.md
|
Review addressed on this branch/PR (no duplicates, no history rewrite — joined via merge, squash on land): diagnosed Lint as clippy::unused_unit on the deliver tail and removed it; CLI probes rewritten through a fail-closed launcher (2 MiB stack + CPU/output/wall bounds during execution, concurrent capped readers, bounded reap, timeout/oversize/incomplete/abort state, limits readback, durable logs on failure); admission boundary pinned by exact-count tests (three direct handlers via cap 3, two at max_function_depth 2) and documented in both limits across knowledge/threat-model/public docs; stale propagate comment corrected; agent footer removed from PR body. Rebased on latest main; gates rerun on final head. |
|
Required follow-up review on head99670655833d6f23047a05a782f38b272621432f: the new ProbeRun improves CPU/stack readback and concurrent draining, but it does not yet satisfy the fail-closed lifecycle requested in6106321773. Do not merge on green alone.
|
…2681) ## Corrective follow-up to #2679 (merged as b563bbf) Addresses the unresolved lifecycle review on the merged `run_bounded_probe`: it 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. No product-code changes; test harness only. ## Before / After Before: `child.wait()` after the 20 s poll, oversize/error visible only at thread end, cap checked after full-chunk copy, exact-cap EOF flagged oversized, `Err(_)` read as EOF, two fresh 5 s waits past the deadline. After: one absolute deadline (documented 20 s execution + 5 s cleanup grace, never claimed as strict 20 s), bounded `try_wait` reaping, live oversize/read-error reporting, `Interrupted` retry, cap-before-copy with a bounded excess sentinel (exact-cap EOF admitted, sentinel byte discarded, never more than the cap retained), unfinished cleanup/readers reported truthfully instead of hanging. ## Tests - Reader-core self-tests: exact-cap EOF admitted, one-byte excess flagged live with exactly cap retained, genuine errors reported (not EOF) with partial bytes kept, `Interrupted` retried. - Launcher self-tests with short explicit bounds: forced overflow (oversized, cap retained, reaped), timeout/kill/reap (timed out, SIGKILL signaled, cleanup complete), grandchild-held pipes (incomplete reported truthfully, direct reap clean), fast exit (raw exit distinct from harness outcome). - Trap probes assert exact stdout bytes (three ENTRY lines per shape), exact limits readback (`stack=2048 cpu=10`), exit 1, diagnostic, and all failure flags clean. - Hostile entry counts run in bounded subprocesses; finite exactly-3 nesting success plus shared depth 0/1/2 boundaries covered in-process under explicit tiny budgets with termination arguments.
1 participant
Summary
Self-delivered signals (
kill→deliver_self_signal→run_trap_commandinline) had no depth charge: direct, mutual, and mixed function/source recursion grew the native stack until overflow instead of failing cleanly. This bounds trap re-entry with a dedicated nesting cap plus the shared function/source budget, and surfaces the first trip as a resource-limit error at theexecboundary.Threat
TM-DOS-020: hostile scripts (
trap 'kill -USR1 $$' USR1, mutual USR1/USR2,trap 'f' USR1with self-killingf,trap 'source f' USR1with self-killingf) crashed the interpreter with native stack overflow. Mixed source/signal recursion still overflowed a 2 MiB stack with only the shared budget charged, because per-cycle async frames outrun the depth-16 trip in debug builds.Before / After (2 MiB stack, debug CLI)
Before: all four shapes abort (
SIGABRT, exit 134,thread has overflowed its stack).After: each terminates with exit 1,
maximum trap nesting depth exceededon stderr, andresource limit exceededsurfaced:Finite nesting order, EXIT traps, ignored signals (
trap '' USR1), unhandled-signal 128+N, and shell recovery after a trip are unchanged.Design notes
run_trap_commandreturnsOption, 4 call sites): unparsable/failing handlers never disturb the interrupted command. Only resource exhaustion (depth/fuel) fails the exec, like command fuel.max_function_depthstill stops mixes early.Err) are flagged via a sticky error surfaced atexec_with_options.Test plan
signal_trap_depth: all four recursion shapes at depths 2/3 assert resource-limit errors without command-limit confusion, plus shell recovery, ignored-signal depth-freedom, command-limit reachability, and parse-error swallowing.cli_oneshot: small-stack subprocess probes (direct/mutual/function/source, 2 MiB, timeouts, output caps); an abort fails the test.kill-self.test.shspec: finite, exit, ignored, unhandled cases.--all-targets --all-features), feature-sliced workspace tests, doc tests, realfs, failpoints, proptest, ssh. Full integration binary matches main (one pre-existing macOS-debug differential failure and one pre-existing stack-overflow threat-model test also fail on pristine main).