Repository navigation
test(security): lock in bounded rejection of bare-negation conditional print - #2669
Merged
Merged
Conversation
…l print
TM-DOS-044: f() { [[ ! ]]; }; declare -f f must be a clean parse-time
syntax error (shell stays alive), deep ! nesting rejected by depth cap,
valid [[ ! -n x ]] still defines and prints.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
bashkit | c70de14 | Commit Preview URL Branch Preview URL |
Oct 10 2026, 11:16 PM |
## What changed `date -d` reads weekday names: `monday`, `this fri`, `next tuesday`, `last sunday` (full names or three-letter abbreviations, any case). They resolve the way GNU resolves them, at midnight in the sandbox `TZ`: a bare or `this` day may be today, `next` skips today, and `last` is the latest one before today. `today` is accepted as `now`. `touch -d` and `find -newermt` share the parser. Any other unknown word after `next`/`last` (`next blursday`) is now `invalid date` with exit 1. ## Why `next monday` read `monday` as an unknown unit with a zero offset and printed the current time with exit 0, so scripts got a wrong date and no error. ## Before / After ``` $ date -d 'next monday' '+%a %F %T' # run on Friday 2026-10-09 Before: Fri 2026-10-09 05:58:34 (the current time) After: Mon 2026-10-12 00:00:00 $ date -d 'next blursday'; echo $? Before: Fri Oct 9 05:58:34 UTC 2026, exit 0 After: date: invalid date 'next blursday', exit 1 ``` Both After lines match GNU date 9.12. Compared with GNU on 20 weekday and relative inputs in UTC, America/Los_Angeles and Australia/Sydney: all identical except `next mins`, which GNU reads as one minute and this reports as an invalid date (it used to print the current time). ## Risk - Low - A script that relied on `next <unknown word>` returning the current time now gets an error, as with GNU. ## Checklist - [x] Tests added or updated: unit tests and spec cases in `date.test.sh` - [x] Backward compatibility considered ## Validation - Reproduced the weekday/unknown-unit failures on current main before applying the contribution. - Fixed a review finding: extreme virtual-clock timestamps could panic during local weekday conversion. The conversion now checks the timezone offset and returns an out-of-range error; regression covers both chrono extrema. - Added shared `date`/`touch`/`find` coverage, strict newer-than comparisons, sandbox-calendar day boundaries, midnight DST gaps, and the no-tzdata build. - 66 date unit tests, 19 timezone/shared-parser tests (including 42 GNU-date input/zone comparisons), and 9 no-tzdata tests passed. - 158 repository-script tests, capability parity, OKF, source doc links, workflow/toolchain parity, changelog checks, and cargo vet passed. - Local `just pre-pr`: formatting and workspace Clippy passed; 3,736 library tests passed. The integration binary aborts on macOS in `blackbox_security_tests::finding_nested_cmd_subst_stack_overflow::depth_50_is_bounded`. The exact test also aborts on clean latest main `bd60cdc09e19e7bdc6e7ffd8247b5663fa7bb9fc`; this is an existing baseline failure, not evidence against this date change. - Final-head all-target/all-feature Clippy passed with warnings denied. Bash spec harness: 3,954 passed, 0 failed, 28 intentional skips. - CLI smoke proof (2026-10-10), with a shared sandbox timezone: ```text export TZ=America/Chicago date -d 'next monday' '+%a %F %T %z %s' Mon 2026-10-12 00:00:00 -0500 1791781200 touch -d 'next monday' /tmp/stamp; date -r /tmp/stamp '+%a %F %T %z %s' Mon 2026-10-12 00:00:00 -0500 1791781200 find /tmp/stamp -newermt 'next monday - 1 second' /tmp/stamp find /tmp/stamp -newermt 'next monday' [no output: equal timestamps do not match] date -d 'next blursday'; echo rejected=$? date: invalid date 'next blursday' rejected=1 ``` - Both benchmark harnesses completed: Criterion parallel execution (including 1,000 shared-filesystem sessions; short 0.1s warmup/0.5s target measurements) and `just bench` (100% output agreement against Bash). Benchmark data is validation evidence, not a performance-regression claim. - All **42 GitHub checks passed** against final head `6541f04af4d2b898bba666be004205518d91ae72`. [CI run](https://github.com/everruns/bashkit/actions/runs/38091572643). ## Security No new dependencies, host filesystem reads, network access, unsafe code, or permissions. Weekdays use the existing sandbox-only timezone and virtual clock. Unknown relative units fail before `touch` creates a file; calendar arithmetic and timezone conversion are checked. Required knowledge and public compatibility documentation updated. Co-authored-by: xmakro <xmakro@users.noreply.github.com> Co-authored-by: Mykhailo Chalyi <mike@chaliy.name>
## 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)
…f, type) Exact finding shape: repeated parenthesized conditionals in an unexecuted function. 200x nesting defines and prints; 300x is a clean depth rejection on both printer paths; 80x resolves via type.
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.
Summary
Regression lock-in for the function-printing stack-overflow finding
(
f() { [[ ! ]]; };+declare -f f, TM-DOS-044).Finding
Reproduction on current
origin/main(bd60cdc0) shows the PoC isalready rejected at parse time — no code change required:
f() { [[ ! ]]; }→syntax error in conditional expression, exit 2,shell stays alive (matches real bash, which also rejects
[[ ! ]])f() { [[ ( ]]; },[[ ! ! ]]→ same clean rejection!/ 300x paren nesting →nested too deeplyviaCOND_MAX_DEPTH(256) incrates/bashkit/src/parser/mod.rs[[ ! -n x ]]still defines, evaluates, and prints viadeclare -fRoot cause of the original overflow (unbounded
cond_termrecursion on abare
!with no operand during function printing) is cut off one layerearlier: the parser never admits the malformed conditional into the AST,
so the printer is only ever reached with a bounded, valid conditional.
Change
crates/bashkit/tests/integration/blackbox_security_tests.rs(+93): new
finding_cond_bare_not_print_overflowmodule, 4 tests —bare
!, bare(, deep-!rejection, valid-form still prints.Before / After
Before (audit snapshot): SIGSEGV printing the function.
After (this branch, binary at
/tmp/bashkit-target-fp/debug/bashkitequivalent): clean syntax error, exit 2, trailing commands still run.
Validation
blackbox_security_testspasscargo fmt --checkclean;cargo clippy -p bashkit --test integration -- -D warningscleanNote: unrelated red baseline (pre-existing, verified on clean main via
git stash)threat_model_tests::nesting_depth_security::misconfig_huge_ast_depth_still_safeoverflows the test thread stack in this environment (macOS) and aborts
the integration binary; fails identically without this change.
proptest_differential::malformed_command_substitution_aborts_like_bashexpects newer-bash behavior; host bash here is 3.2 (
echo a$(|)bprints
ab, exit 0). Environmental.Both are out of scope for this single finding; flagging for a separate
maintenance pass. Merging should wait for a green-baseline decision.
Produced by yolop