Repository navigation
fix(tests): fail-closed bounded probe launcher with honest lifecycle - #2681
Merged
Merged
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
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
…for corrective follow-up
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
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 2f2561f | Commit Preview URL Branch Preview URL |
Oct 11 2026, 08:59 AM |
Contributor
Author
|
Actual Linux instrumented coverage for current2681 head a286683 failed in run38125732123/job114429595911: launcher_times_out_kills_and_reaps panics at cli_oneshot.rs669, "pipes close after reap" (64pass/1fail). This is a real required-gate failure, not a green watcher result. The timeout fixture is sh -c "sleep 30". Killing/reaping only the direct shell need not close inherited output pipes: its sleep child may survive and hold them. Review the intended direct-child versus process-tree contract. Use a deliberately direct exec fixture for direct reap proof, retain the independent grandchild-held-pipe negative test and its honest incomplete verdict, or implement properly bounded process-group cleanup if that is the claimed contract. Do not remove/weaken failure flags, accept unknown/incomplete capture as success, raise test stacks or waive Linux coverage. Keep execution and cleanup deadlines explicit and rerun both lifecycle tests plus unchanged full required CI on the changed actual head. This does not settle all source-level review claims or exact merged product proof. Wait for actual required conclusions/full reviews, squash merge corrective2681 only then, rebuild exact merge and write durable actual producer/assertion file evidence. Healthy watcher task_01a12a206ef177219d12561c0e77c9b7 remains owned; monitor neither cancels nor duplicates it. |
2 tasks done
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
## What changed Test-only, one file: threat_model_tests.rs (+211/-33). Reworks the shared run_command_bounded lifecycle per the review on #2680: - Retention truncated BEFORE extending: the sample buffer never passes the 8192 cap (was: extend-then-truncate kept a chunk over). - Interrupted reads retried; real reader errors end capture with partial bytes kept and a new read_error flag — never relabeled as clean EOF. - Exact-cap-then-EOF is not oversize; only real excess sets the flag. - Reaping bounded inside a finite 5s grace; no unconditional wait() past the cleanup deadline (targeted allow documents why the lint suggestion would reintroduce the hang). - timed_out retained as recorded, never recomputed; drain 2s + reap 5s graces documented against the execution deadline. - TM-DOS-136 regression gains !read_error and the corrected 8192-byte cap text. - Bounded selftests: exact-cap, cap+1, scripted Interrupted/error pipes, timeout kill+reap (~2s), overflow flag. Does not touch #2681 distinct run_bounded_probe helper. ## Before / After Before: reader errors read as clean EOF; retention could exceed the cap by a chunk; kill-path wait() unbounded; no lifecycle selftests. After: threat_model 219 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness)
2 tasks done
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
…#2683) ## What changed Test-only, one file: threat_model_tests.rs. Closes the lifecycle review on #2682 without touching #2681 distinct run_bounded_probe helper: - Bounded join gate: readers join only after all channel receipts (a receipt proves its thread finished every blocking call; sending is the last act before return). Missing receipts skip the join and fail incomplete; panicking readers fail visibly via join error. std offers no timed join; the gate plus elapsed-total assertions are the documented termination proof. - Reap extracted into a poll seam (reap_with_grace) with deterministic verdict unit tests: immediate, delayed, never-exits (bounded 50ms), and poll-error cases. - Timeout/overflow selftests run under the shared fail-closed launcher (new run_raw_bounded) so the effective stack=4096/cpu=10 bounds are applied and read back, with elapsed-total bounds (2s+2s+5s budget). - Existing bounded_helper_flags_retained_pipes reused and strengthened: elapsed proof (returns without waiting for the 30s sleeper), read_error distinction, outer-deadline documentation. ## Before / After Before: unconditional join past the grace; reap untestable without OS fakery; selftests bypassed the launcher; no elapsed proofs. After: threat_model 220 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness)
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
…tests Per completion review on #2683: join readers only while is_finished inside the shared completion grace (a receipt is not the termination contract); deterministic paused-after-send proof with release/cleanup watchdog; poll/read/reap faults injected through the real parent lifecycle with distinct fail-closed verdicts (no panicking expect on child poll); timeout/overflow selftests under the fail-closed launcher with cap readback and elapsed bounds; existing retained-pipe fixture reused and strengthened. Untouched: #2681 distinct probe helper.
2 tasks done
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
…tests (#2684) ## What changed Test-only, one file: threat_model_tests.rs. Closes the completion review on #2683 without touching #2681 distinct run_bounded_probe helper: - Bounded join completion gate: readers join only while is_finished inside the shared completion grace (JOIN_GRACE 5s; full budget is execution limit + drain 2s + reap 5s + completion 5s). A channel receipt is not the termination contract since scheduling can pause the sender after send. Unfinished handles stay detached and fail incomplete; panicking readers fail visibly. No reproduced production hang is claimed. - Deterministic paused-after-send proof with explicit release/cleanup path and watchdog: a sender blocked on release reads unfinished by construction, releases cleanly, and joins inside the bound. - Poll/read/reap faults injected through the real parent lifecycle (LifecycleFaults seam): poll failure fails closed with a verdict distinct from timeout (incomplete, reap-recovered exit, no timeout); injected read failure propagates read_error with partial capture; unavailable reap yields exitless incomplete. The panicking child.try_wait expect is gone. - Timeout/overflow selftests run under the shared fail-closed launcher with stack/cpu readback and elapsed-total bounds. - Existing bounded_helper_flags_retained_pipes reused and strengthened (elapsed proof, read_error distinction, outer-deadline documentation). ## Before / After Before: unconditional join after receipts; reap untestable without OS fakery; child poll panicked; selftests bypassed the launcher. After: threat_model 222 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness)
2 tasks done
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
## What changed Test-only, one file: threat_model_tests.rs. Closes the completion review on #2684 without touching #2681 distinct run_bounded_probe helper: - Common cleanup termination: every loop exit (observed completion, deadline, oversized, poll failure) funnels through one reachable kill guard; verdicts recorded pre-kill are never recomputed. Previously the poll-fault break skipped its kill guard. - Bounded join completion gate via a shared join_finished primitive: readers join only while is_finished inside JOIN_GRACE (a receipt is not the termination contract since scheduling can pause the sender after send). Unfinished handles stay detached and fail incomplete; panics fail visibly. No reproduced production hang is claimed. - Deterministic paused-after-send proof drives the actual gate primitive with bounded readiness (recv_timeout), explicit release, and a finite join-cleanup watchdog. - Poll/read/reap faults injected through the real parent lifecycle with distinct fail-closed verdicts: the panicking child.try_wait expect is gone; unavailable reap keeps real bounded OS cleanup while the verdict reports None (no invented success). - Timeout/overflow/retained-pipe selftests under the shared fail-closed launcher with stack/cpu readback and elapsed-total bounds; the existing retained-pipe fixture is reused and strengthened, never duplicated. ## Before / After Before: poll-fault break skipped termination; unconditional join after receipts; reap untestable without OS fakery; child poll panicked; selftests bypassed the launcher. After: threat_model 222 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness)
2 tasks done
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
Per completion review on #2685: fail_poll fires only after a readiness file proves the victim truly live (direct-exec sleeper, no orphan); BoundedRun gains a reaped receipt separating actual OS cleanup from exit/observation verdicts; retained-pipe fixture runs under the fail-closed launcher with a file side-channel receipt proving the effective bounds despite incomplete pipe capture. Reuses the existing retained-pipe fixture; untouched: #2681 distinct probe helper.
chaliy
added a commit
that referenced
this pull request
Oct 11, 2026
…receipts (#2686) ## What changed Test-only, one file: threat_model_tests.rs. Closes the completion review on #2685 without touching #2681 distinct run_bounded_probe helper: - join_finished returns the unfinished handle, so the paused-after-send proof releases and joins THAT SAME thread through the actual gate primitive (bounded readiness, release, finite join cleanup, elapsed watchdog). No twin thread, no detached-while-asserting-cleanup. - Poll-fault fixture uses a direct-exec sleeper: the shell replaces itself, so killing closes the pipes at once with no orphan and no 30s descendant. - reap_never keeps the real bounded OS cleanup while only the observation is discarded for the verdict: no zombies by construction, no invented exit/reap success. - Retained-pipe fixture runs under the fail-closed launcher; because incomplete pipe capture precludes the stderr readback by design, the effective bounds are proven through a per-process receipt file the fixture shell writes before parking the sleeper (removed afterwards). Existing fixture reused and strengthened, never duplicated. - Prior review debt acknowledged: #2684 merged on formal reviews alone while the completion comment was unresolved; this followup resolves it before any merge. ## Before / After Before: twin-thread gate proof; forked sleeper orphan in the poll fixture; reap skipped under fault; no launcher proof on the retained fixture. After: threat_model 222 passed (only pre-existing macOS nesting abort skipped, identical on unmodified code); error 14, partial_parse 11 green; fmt plus clippy -D warnings clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness) ## Update: readiness gating, reaped receipt, launcher receipts Follow-up commits on the same branch add: - fail_poll fires only after a readiness file proves the victim truly live under bounds (direct-exec sleeper, no orphan); readiness itself is asserted alongside the reaped receipt, cap readback, and elapsed bound. - BoundedRun gains a reaped receipt: OS termination accounted for, orthogonal to exit_code and to injected observation faults (which discard the observation, never the cleanup). reap_never asserts real cleanup ran alongside its discarded verdict. - Retained-pipe fixture runs under the fail-closed launcher; because incomplete pipe capture precludes the stderr readback by design, the effective stack/cpu bounds are proven through a per-process receipt file the fixture shell writes before parking the sleeper (removed afterwards).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_waitreaping, live oversize/read-error reporting,Interruptedretry, 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
Interruptedretried.stack=2048 cpu=10), exit 1, diagnostic, and all failure flags clean.