Repository navigation
test(indirect): enforce real bounds in the hostile-cycle harness - #2676
Conversation
…sion
An indirect array target such as r='a[0]:-${!r}]' passed validation whole, so desugar re-parsed ${a[0]:-${!r}]} and the unset-element default re-entered indirect expansion without bound (stack overflow, SIGABRT).
Truncate the resolved bracket target at the first ] after [ before validation. Real bash resolves a[0] and ignores the rest (verified oracles), so trailing operands — including nested ${!...} — never reach the re-parse. Plain names, a[$i], a[@], assoc subscripts pass through unchanged; $(/backquote/process-substitution guards unchanged.
Co-Authored-By: yolop <yolop@everruns.com>
…oundary
PR 2673 truncated indirect array targets at the first ']', which broke valid nested subscripts (a[b[0]] errored; both real bash versions print two). The boundary now ends at the close of the first complete [...] reference, tracking nested brackets and skipping quoted/escaped text, so trailing operators still never reach the desugar re-parse (self/mutual ${!...} cycles terminate with bash-parity output) while nested and quoted-key references resolve whole.
Tests: hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash; assoc direct/indirect agreement. Renumbers the prior TM-DOS-134 rows to TM-DOS-135 (tilde budget work owns 134).
Co-Authored-By: yolop <yolop@everruns.com>
…plete
Modern bash (5.x) rejects indirect array targets with trailing operator/nesting text (invalid variable name); older 3.2 ignored the tail. Refusal (instead of stripping) keeps nested ${!...} from the desugar re-parse while valid nested subscripts (a[b[0]]) resolve whole, matching bash 5.x in every probed shape.
Hostile self/mutual cycles run in a bounded child CLI under a wall-clock watchdog with output caps and recorded producer status; finite nested indexed indirects assert byte equality with real bash on both bash generations for complete references; assoc direct/indirect agreement. TM-DOS-135 (tilde owns 134).
Co-Authored-By: yolop <yolop@everruns.com>
The bounded child runner now drains each pipe on its own reader thread with an 8 KiB sample cap, kills the child past the wall limit or on oversize, and launches through best-effort ulimit CPU/stack caps with exec keeping the CLI as producer. Oversize and timeout are harness failures, never silent cuts. Adds helper self-tests: a hung child must report timeout, an overproducer must report oversize. Hostile self/mutual cycles assert refusal with the real producer exit.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 32dd777 | Commit Preview URL Branch Preview URL |
Oct 11 2026, 04:17 AM |
|
Review of head ceba71b: The concurrent readers and visible oversize flag resolve the old silent
Read all actual comment bodies completely, not counts or head snippets. Retain |
The hostile-cycle harness launcher now refuses without CPU/stack caps (exit 99) instead of suppressing ulimit failures, and reports the effective limits for the harness to assert. One shared wall deadline covers child exit, pipe drain, and reaping: pipe EOF alone never ends the run, retained pipes are reported incomplete within bounds, and oversize still fails visibly. Self-tests cover launcher refusal, early pipe close while live, retained pipes, hung children, and overproducers.
|
Resolving each item in the blocking verification note (#2676 (comment)):
Draft stays until CI is green; no merge before that. |
|
Review of head2df6999233ed1486f847712a477b0da09e6667f7: Fail-closed ulimit setup/readback and early-EOF/inherited-pipe coverage are substantial improvements. One remaining timeout-status race needs resolution before merging: the loop kills at the wall deadline, then polls try_wait, then sets timed_out = status.is_none(). If SIGKILL is reaped by that poll, a genuinely timed-out live child is reported timed_out=false. Both hung-child self-tests can therefore pass/fail based on scheduling, rather than the deadline event. Record the deadline decision from the pre-kill observed liveness and preserve that timeout outcome while subsequently reaping the real status. Keep the exited-child/retained-pipe case distinct. Add repeated harmless short-deadline coverage or a deterministic helper invariant; do not loosen the timeout assertions. Also update the editable PR body: it still describes CPU/stack caps as best-effort, unlike the new fail-closed implementation. Correct the review-response permalink (6104616156 belongs to2674, not2676). Existing draft/required CI and actual post-merge proof remain required; do not create another follow-up or rewrite merged history. |
Record the wall-deadline verdict from pre-kill observed liveness instead of recomputing it after polling try_wait, so reaping the SIGKILL victim cannot flip a genuine timeout. Covers the verdict with repeated short-deadline runs.
|
Resolving the remaining race review (#2676 (comment); correction: my previous reply wrongly linked the 2674 comment 6104616156 — this is the actual 2676 thread):
Draft stays until this round of CI is green; no merge before that. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
## What changed Generalizes `run_cli_bounded` with a `run_cli_argv_bounded(script, extra_args, limit)` variant and deletes the hand-rolled argv reader the TM-DOS-136 regression carried. One file, net -46 lines. ## Why The #2676 harness rewrite changed `BoundedRun`'s shape (oversized/incomplete tracking, fail-closed CPU/stack launcher). The TM-DOS-136 multiline-token x long-ARG0 regression still constructed the old shape, so the integration test binary did not compile on main after the #2675 squash merge — main is red (`Test` + `Lint` fail on ec540f4). This routes the vector through the new harness, keeping the fail-closed caps and real producer exit codes. ## Before / After Before: `cargo test -p bashkit --test integration --no-run` fails with E0063 on main. After: compiles; `threat_model` 214 passed (skipping only the pre-existing macOS stack-overflow abort in `misconfig_huge_ast_depth_still_safe`, identical on unmodified code); `error::` 14 passed; fmt + clippy clean. ## Risk - Test-only change. No production code touched. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal test harness)
1 participant
Corrective follow-up to #2674, resolving the remaining review (#2674 (comment)).
What changed (tests only; no interpreter change)
The hostile-cycle child runner now enforces real bounds instead of collecting everything and truncating after the fact:
oversized— a harness failure, never a silent cut;ulimit -t/-sCPU/stack caps (|| exit 99beforeexec, so a refusal can never run unbounded) withexeckeeping the CLI as the real producer; effective values are read back and asserted per run (Rust std exposes no portable per-child CPU/stack/memory caps; Darwin address-space limits are documented as unenforced, not claimed).Helper proof (fails visibly on regression)
bounded_helper_kills_hung_child:sleep 30under a 3 s limit must report timeout;bounded_helper_flags_oversize_output: a ~100 KB overproducer must report oversize with a capped sample.invalid variable name, real producer exit 1, empty stdout) plus non-oversize.Risk
bashfor the launcher shell (the suite already requiresbashfor real-bash differentials)Checklist
Follow-up to #2674; resolves the remaining review comment linked above.