Repository navigation
fix(arithmetic): bound nested dollar expansion - #2668
Merged
Merged
Conversation
Negative indices need bash 4.3+; macOS ships bash 3.2 while Linux CI
has bash 5.x, so ${a[-1]} makes the assertion environment-dependent.
${a[i-0]} still exercises arithmetic inside the subscript everywhere.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | 7e87c12 | Commit Preview URL Branch Preview URL |
Oct 10 2026, 10:44 PM |
chaliy
added a commit
that referenced
this pull request
Oct 10, 2026
## What changed Nested arithmetic dollar expansion (`$((...))` inside `$((...))`) no longer escapes the shared evaluation budget. Each nested expansion now charges fuel and borrows the parent budget instead of starting fresh, so hostile nesting aborts with a bounded error instead of exhausting the stack. ## Why Finding arter-y7s8: textual expansion ran before the shared depth/fuel checks, and every recursive `$((...))` got a brand-new budget, so the 8-deep / 1024-byte limits never engaged across nesting levels. ## Before / After Sensor (`echo \$((1+\` x 5000 + `1\` + `))\` x 5000`): - Before: never returns (stack exhaustion, unbounded diagnostics). - After: exits 1 in ~2.7s, 327-byte stderr ending in `expression recursion level exceeded`. Tests: 7 regression tests in `arithmetic_resource_tests.rs` (recursion aborts, valid nesting keeps working, bash parity within limits); 40/40 arithmetic integration tests pass. `cargo fmt`, `clippy --all-targets`, `cargo vet`, capability/OKF/doc-link/workflow/changelog checks all green. Note (macOS-local only, unrelated paths, Linux CI is the gate): `blackbox depth_50 cmd-subst` test needs a bigger thread stack locally (passes with `RUST_MIN_STACK=16777216`); `scripts/tests` python suite needs Python 3.11+ (local python is 3.9). ## Risk - Low: only the arithmetic evaluation path; valid expressions (nesting <= 8, subscripts, namerefs) verified unchanged against real bash. - What can break: scripts relying on >8-deep arithmetic nesting now get an error instead of a hang. ## Checklist - [x] Tests added or updated - [x] Backward compatibility considered (internal code, no compat needed) Produced by [yolop](https://everruns.com/yolop)
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
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.
What changed
Nested arithmetic dollar expansion (
$((...))inside$((...))) no longer escapes the shared evaluation budget. Each nested expansion now charges fuel and borrows the parent budget instead of starting fresh, so hostile nesting aborts with a bounded error instead of exhausting the stack.Why
Finding arter-y7s8: textual expansion ran before the shared depth/fuel checks, and every recursive
$((...))got a brand-new budget, so the 8-deep / 1024-byte limits never engaged across nesting levels.Before / After
Sensor (
echo \$((1+\x 5000 +1\+))\x 5000`):expression recursion level exceeded.Tests: 7 regression tests in
arithmetic_resource_tests.rs(recursion aborts, valid nesting keeps working, bash parity within limits); 40/40 arithmetic integration tests pass.cargo fmt,clippy --all-targets,cargo vet, capability/OKF/doc-link/workflow/changelog checks all green.Note (macOS-local only, unrelated paths, Linux CI is the gate):
blackbox depth_50 cmd-substtest needs a bigger thread stack locally (passes withRUST_MIN_STACK=16777216);scripts/testspython suite needs Python 3.11+ (local python is 3.9).Risk
Checklist
Produced by yolop