From 8665d5a3dd37fb4450a48d1060af6e36ec611b20 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 17:47:47 -0500 Subject: [PATCH 1/4] test(security): lock in bounded rejection of bare-negation conditional 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. --- .../integration/blackbox_security_tests.rs | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index e382b228a..1f0c6bb1f 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -1968,3 +1968,96 @@ mod creative_abuse_passing { ); } } + +// ============================================================================= +// FINDING: STACK OVERFLOW — CONDITIONAL PRINTING (`[[ ! ]]` + `declare -f`) +// Threat: TM-DOS-044 (conditional-expression recursion). +// Issue: `f() { [[ ! ]]; }; declare -f f` overflowed the stack in +// `cond_term` recursion during function printing (SIGSEGV). The parser now +// rejects a bare `!` (and bare `(`/unbalanced nesting) up front with a +// syntax error, and caps nesting depth (COND_MAX_DEPTH), so the printer is +// only ever reached with a bounded, valid conditional. These tests lock in: +// the PoC is a clean non-fatal syntax error (no crash/hang), deep `!` +// nesting is rejected, and the valid `[[ ! ... ]]` form still prints. +// ============================================================================= + +mod finding_cond_bare_not_print_overflow { + use super::*; + + /// PoC: defining a function whose body is `[[ ! ]]` is a clean parse-time + /// rejection, never a stack overflow in function printing; shell stays alive. + #[tokio::test] + async fn bare_not_in_function_print_is_bounded() { + let mut bash = tight_bash(); + let err = format!("{:?}", bash.exec("f() { [[ ! ]]; }").await.unwrap_err()); + assert!( + err.contains("syntax error in conditional expression"), + "expected conditional syntax error, got: {}", + err + ); + let alive = bash.exec("echo alive").await.unwrap(); + assert!( + alive.stdout.contains("alive"), + "shell must survive the PoC, got stdout: {:?}", + alive.stdout + ); + } + + /// Bare `(` as the whole conditional is likewise a clean rejection. + #[tokio::test] + async fn bare_open_paren_is_bounded() { + let mut bash = tight_bash(); + let err = format!("{:?}", bash.exec("f() { [[ ( ]]; }").await.unwrap_err()); + assert!( + err.contains("syntax error in conditional expression"), + "expected conditional syntax error, got: {}", + err + ); + let alive = bash.exec("echo alive").await.unwrap(); + assert!( + alive.stdout.contains("alive"), + "shell must survive, got stdout: {:?}", + alive.stdout + ); + } + + /// Deep `!` nesting (300x) is rejected by the depth cap, not a crash. + #[tokio::test] + async fn deep_not_nesting_is_bounded() { + let mut bash = tight_bash(); + let script = format!("f() {{ [[ {} ]]; }}", "! ".repeat(300)); + let err = format!("{:?}", bash.exec(&script).await.unwrap_err()); + assert!( + err.contains("nested too deeply") + || err.contains("syntax error in conditional expression"), + "expected depth/syntax rejection, got: {}", + err + ); + let alive = bash.exec("echo alive").await.unwrap(); + assert!( + alive.stdout.contains("alive"), + "shell must survive deep nesting, got stdout: {:?}", + alive.stdout + ); + } + + /// The valid negated form still defines, evaluates, and prints. + #[tokio::test] + async fn valid_negated_conditional_still_prints() { + let mut bash = tight_bash(); + let result = bash + .exec("f() { [[ ! -n x ]]; }; declare -f f") + .await + .unwrap(); + assert!( + result.stderr.is_empty(), + "valid conditional must not error, got stderr: {:?}", + result.stderr + ); + assert!( + result.stdout.contains("[[ ! -n x ]]"), + "declare -f must print the negated conditional, got stdout: {:?}", + result.stdout + ); + } +} From 0f6a179505220648fa6a222af0a341c5dd3733dd Mon Sep 17 00:00:00 2001 From: Makro <4398091+xmakro@users.noreply.github.com> Date: Sat, 10 Oct 2026 15:55:26 -0700 Subject: [PATCH 2/4] fix(date): read weekday names in -d instead of returning now (#2663) ## 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 ` 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 Co-authored-by: Mykhailo Chalyi --- crates/bashkit/docs/compatibility.md | 2 +- crates/bashkit/src/builtins/date.rs | 124 +++++++++++++++++- .../date_timezone_differential_tests.rs | 63 +++++++++ .../date_timezone_no_tzdata_tests.rs | 14 ++ .../tests/integration/date_timezone_tests.rs | 90 +++++++++++++ .../tests/spec_cases/bash/date.test.sh | 12 ++ knowledge/operations/limitations.md | 12 ++ 7 files changed, 312 insertions(+), 5 deletions(-) diff --git a/crates/bashkit/docs/compatibility.md b/crates/bashkit/docs/compatibility.md index 333eda947..641157527 100644 --- a/crates/bashkit/docs/compatibility.md +++ b/crates/bashkit/docs/compatibility.md @@ -91,7 +91,7 @@ for sandbox security reasons. See the compliance spec for details. | `uniq` | `-c`, `-d`, `-u` | Filter duplicate lines | | `cut` | `-d DELIM`, `-f FIELDS` | Extract fields | | `tr` | `-d`, character ranges | Translate/delete chars | -| `date` | `+FORMAT`, `-u`, `-R`, `-I`, `-r`, `-d`/`--date` (relative, compound, epoch, ISO date-times with optional seconds and zone) | Display/format date with sandboxed IANA timezone support | +| `date` | `+FORMAT`, `-u`, `-R`, `-I`, `-r`, `-d`/`--date` (relative, weekday, compound, epoch, ISO date-times with optional seconds and zone) | Display/format date with sandboxed IANA timezone support | | `wait` | `[-n] [-p VAR] [-f] [ID...]` (PID or `%jobspec`) | Wait for background jobs | | `jobs` | `-l`, `-p` | List running background jobs | | `ps` | `-f`, `aux`, `-o`, `-p` | Virtual process list (shell + jobs) | diff --git a/crates/bashkit/src/builtins/date.rs b/crates/bashkit/src/builtins/date.rs index b395583c8..176cf8080 100644 --- a/crates/bashkit/src/builtins/date.rs +++ b/crates/bashkit/src/builtins/date.rs @@ -10,8 +10,12 @@ use async_trait::async_trait; use super::arg_parser::OptArg; +#[cfg(feature = "tzdata")] +use chrono::Offset; use chrono::format::{Item, StrftimeItems}; -use chrono::{DateTime, Duration, LocalResult, NaiveDate, NaiveDateTime, TimeZone, Utc}; +use chrono::{ + DateTime, Datelike, Duration, LocalResult, NaiveDate, NaiveDateTime, TimeZone, Utc, Weekday, +}; #[cfg(feature = "tzdata")] use chrono_tz::Tz; @@ -147,6 +151,19 @@ impl SandboxTimezone { Self::Iana(tz) => dt.with_timezone(&tz).format(format).to_string(), } } + + fn date_naive(self, dt: &DateTime) -> Option { + match self { + Self::Utc => Some(dt.date_naive()), + #[cfg(feature = "tzdata")] + // A host-supplied virtual clock can reach chrono's extrema; + // naive_local/date_naive panic if the zone pushes it past them. + Self::Iana(tz) => dt + .naive_utc() + .checked_add_offset(dt.with_timezone(&tz).offset().fix()) + .map(|local| local.date()), + } + } } impl Date { @@ -294,7 +311,7 @@ fn parse_base_date( // Special words match lower.as_str() { - "now" => return Ok(now), + "now" | "today" => return Ok(now), "yesterday" => return Ok(now - Duration::days(1)), "tomorrow" => return Ok(now + Duration::days(1)), _ => {} @@ -372,13 +389,41 @@ fn parse_base_date( return Ok(dt.with_timezone(&Utc)); } + // Day of week: "monday", "next fri", "last tuesday", at midnight. As in + // GNU, a bare day may be today, `next` skips today, `last` is the latest + // one before today. + let (which, day) = lower + .split_once(' ') + .map_or(("this", lower.as_str()), |(which, day)| { + (which, day.trim_start()) + }); + if let Ok(day) = day.parse::() { + let today = timezone + .date_naive(&now) + .ok_or_else(|| format!("date out of range: '{}'", s))?; + let ahead = i64::from(day.days_since(today.weekday())); + let days = match which { + "this" => ahead, + "next" if ahead == 0 => 7, + "next" => ahead, + "last" => ahead - 7, + _ => return Err(format!("date: invalid date '{}'", s)), + }; + let midnight = today + .checked_add_signed(Duration::days(days)) + .and_then(|d| d.and_hms_opt(0, 0, 0)) + .ok_or_else(|| format!("date out of range: '{}'", s))?; + return timezone.local_to_utc(midnight, s); + } + Err(format!("date: invalid date '{}'", s)) } /// Parse a date string like GNU date's -d flag. /// /// Supports simple expressions: -/// "now", "yesterday", "tomorrow", "N days ago", "+N days", +/// "now", "today", "yesterday", "tomorrow", "N days ago", "+N days", +/// "monday", "next friday", "last tuesday", /// "N weeks ago", "N months ago", "N years ago", "N hours ago", /// "@EPOCH", "YYYY-MM-DD[ HH:MM[:SS[.frac]]][ UTC|+HHMM]" /// @@ -472,7 +517,8 @@ fn unit_duration(unit: &str, n: i64) -> Option { "week" => Duration::try_weeks(n), "month" => n.checked_mul(30).and_then(Duration::try_days), // Approximate "year" => n.checked_mul(365).and_then(Duration::try_days), // Approximate - _ => Some(Duration::zero()), + // Not a unit (`next monday`): an invalid date, not a zero offset. + _ => None, } } @@ -1066,6 +1112,76 @@ mod tests { assert_eq!(date.len(), 10); } + #[cfg(feature = "tzdata")] + #[test] + fn weekday_timezone_conversion_checks_range() { + for (now, tz) in [ + (DateTime::::MAX_UTC, "Etc/GMT-14"), + (DateTime::::MIN_UTC, "Etc/GMT+12"), + ] { + let tz = SandboxTimezone::Iana(tz.parse().unwrap()); + assert!(parse_date_string("monday", now, tz).is_err()); + } + } + + #[test] + fn weekday_calendar_arithmetic_checks_range() { + let max = DateTime::::MAX_UTC; + let min = DateTime::::MIN_UTC; + assert!( + parse_date_string( + &format!("next {}", max.weekday()), + max, + SandboxTimezone::Utc + ) + .is_err() + ); + assert!( + parse_date_string( + &format!("last {}", min.weekday()), + min, + SandboxTimezone::Utc + ) + .is_err() + ); + } + + /// From Thursday 2026-10-08 12:00 UTC; expected days match GNU date. + #[test] + fn test_date_d_weekdays() { + let now = DateTime::from_timestamp(1_791_460_800, 0).unwrap(); + for (input, expected) in [ + ("thursday", "2026-10-08"), + ("next thursday", "2026-10-15"), + ("last thursday", "2026-10-01"), + ("monday", "2026-10-12"), + ("next mon", "2026-10-12"), + ("last Fri", "2026-10-02"), + ] { + let dt = parse_date_string(input, now, SandboxTimezone::Utc).unwrap(); + assert_eq!( + dt.format("%F %T").to_string(), + format!("{expected} 00:00:00"), + "{input}" + ); + } + assert_eq!( + parse_date_string("today", now, SandboxTimezone::Utc), + Ok(now) + ); + } + + /// `next ` used to read an unknown word as a zero offset and + /// print the current time. + #[tokio::test] + async fn test_date_d_unknown_word_is_invalid() { + for input in ["next blursday", "last blah"] { + let result = run_date(&["-d", input]).await; + assert_eq!(result.exit_code, 1, "{input}"); + assert!(result.stderr.contains("invalid date"), "{input}"); + } + } + #[tokio::test] async fn test_date_long_date_flag() { let result = run_date(&["--date=yesterday", "+%Y-%m-%d"]).await; diff --git a/crates/bashkit/tests/integration/date_timezone_differential_tests.rs b/crates/bashkit/tests/integration/date_timezone_differential_tests.rs index c281100f1..d96a5794d 100644 --- a/crates/bashkit/tests/integration/date_timezone_differential_tests.rs +++ b/crates/bashkit/tests/integration/date_timezone_differential_tests.rs @@ -88,3 +88,66 @@ async fn matches_gnu_date_for_timezone_sensitive_inputs() { assert_gnu_match(tz, args).await; } } + +#[tokio::test] +async fn matches_gnu_date_for_weekdays() { + let Some(program) = gnu_date() else { + eprintln!("skip: GNU date is not installed"); + return; + }; + for tz in ["UTC", "America/Chicago", "Australia/Sydney"] { + for input in [ + "monday", + "this Mon", + "next tuesday", + "last SUN", + "thursday", + "next Thursday", + "last thursday", + "this fri", + "saturday", + "next blursday", + "last blah", + ] { + // Freeze Bashkit at GNU's reference instant. If the host crosses + // local midnight during the pair, that case has no stable oracle. + let reference = Command::new(&program) + .args(["+%s %F"]) + .env("TZ", tz) + .output() + .unwrap(); + assert!(reference.status.success()); + let reference = String::from_utf8(reference.stdout).unwrap(); + let (epoch, day) = reference.trim().split_once(' ').unwrap(); + let host = Command::new(&program) + .args(["-d", input, "+%s %F %T %z"]) + .env("TZ", tz) + .env("LC_ALL", "C") + .output() + .unwrap(); + let after = Command::new(&program) + .arg("+%F") + .env("TZ", tz) + .output() + .unwrap(); + if after.stdout != format!("{day}\n").as_bytes() { + eprintln!("skip: local midnight crossed during GNU weekday comparison"); + continue; + } + let mut bash = Bash::builder() + .fixed_epoch(epoch.parse().unwrap()) + .env("TZ", tz) + .build(); + let result = bash + .exec(&format!("date -d {} '+%s %F %T %z'", shell_quote(input))) + .await + .unwrap(); + assert_eq!( + result.exit_code, + host.status.code().unwrap_or(-1), + "TZ={tz} {input}" + ); + assert_eq!(result.stdout.as_bytes(), host.stdout, "TZ={tz} {input}"); + } + } +} diff --git a/crates/bashkit/tests/integration/date_timezone_no_tzdata_tests.rs b/crates/bashkit/tests/integration/date_timezone_no_tzdata_tests.rs index 8596aaf0f..c37a31747 100644 --- a/crates/bashkit/tests/integration/date_timezone_no_tzdata_tests.rs +++ b/crates/bashkit/tests/integration/date_timezone_no_tzdata_tests.rs @@ -133,3 +133,17 @@ async fn error_is_a_clean_diagnostic_not_a_panic_or_debug_dump() { assert!(stderr.len() < 200, "diagnostic too long: {stderr:?}"); assert!(!stderr.contains('{'), "looks like a Debug dump: {stderr:?}"); } + +#[tokio::test] +async fn weekdays_work_in_utc_without_tzdata() { + let result = fixed_date(Some("UTC"), "date -d monday '+%F %T'; date -d 'next monday' '+%F %T'; date -d 'last monday' '+%F %T'; date -d today +%s").await; + assert_eq!(result.exit_code, 0); + assert_eq!( + result.stdout, + "2024-01-15 00:00:00\n2024-01-22 00:00:00\n2024-01-08 00:00:00\n1705315200\n" + ); + let result = fixed_date(Some("America/Chicago"), "date -d monday +%s").await; + assert_eq!(result.exit_code, 1); + assert!(result.stdout.is_empty()); + assert!(result.stderr.contains(UNAVAILABLE)); +} diff --git a/crates/bashkit/tests/integration/date_timezone_tests.rs b/crates/bashkit/tests/integration/date_timezone_tests.rs index 8a3b46a81..c7ff63f3a 100644 --- a/crates/bashkit/tests/integration/date_timezone_tests.rs +++ b/crates/bashkit/tests/integration/date_timezone_tests.rs @@ -208,3 +208,93 @@ async fn shared_iso_date_parsing_rejects_invalid_inputs_before_file_effects() { assert_eq!(result.exit_code, 1, "invalid touch created a file: {input}"); } } + +#[tokio::test] +async fn shared_weekday_parsing_uses_sandbox_calendar_and_clock() { + // Monday in UTC and Sydney, still Sunday in Chicago. + const EPOCH: i64 = 1_705_277_400; // 2024-01-15 00:10 UTC + for (tz, input, expected) in [ + ("UTC", "monday", "2024-01-15 00:00:00 +0000"), + ("UTC", "this Mon", "2024-01-15 00:00:00 +0000"), + ("UTC", "next monday", "2024-01-22 00:00:00 +0000"), + ("UTC", "last monday", "2024-01-08 00:00:00 +0000"), + ("America/Chicago", "sunday", "2024-01-14 00:00:00 -0600"), + ("America/Chicago", "next sun", "2024-01-21 00:00:00 -0600"), + ("America/Chicago", "last SUN", "2024-01-07 00:00:00 -0600"), + ("Australia/Sydney", "MONDAY", "2024-01-15 00:00:00 +1100"), + ] { + let mut bash = Bash::builder().fixed_epoch(EPOCH).env("TZ", tz).build(); + let result = bash.exec(&format!( + "date -d '{input}' '+%F %T %z'; touch -d '{input}' /tmp/stamp; date -r /tmp/stamp '+%F %T %z'; find /tmp/stamp -newermt '{input} - 1 second'; find /tmp/stamp -newermt '{input}'" + )).await.unwrap(); + assert_eq!(result.exit_code, 0, "TZ={tz} {input}: {}", result.stderr); + assert_eq!( + result.stdout, + format!("{expected}\n{expected}\n/tmp/stamp\n") + ); + assert!(result.stderr.is_empty()); + } + let mut bash = Bash::builder() + .fixed_epoch(EPOCH) + .env("TZ", "America/Chicago") + .build(); + let result = bash + .exec("date -d today +%s; touch -d today /tmp/stamp; date -r /tmp/stamp +%s") + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, format!("{EPOCH}\n{EPOCH}\n")); +} + +#[tokio::test] +async fn shared_weekday_parsing_rejects_unknown_words_before_file_effects() { + for input in [ + "next blursday", + "last blah", + "next mins", + "this nonsense", + "next monday trailing", + "next /etc/localtime", + ] { + let mut bash = Bash::builder().fixed_epoch(WINTER_EPOCH).build(); + for script in [ + format!("date -d '{input}' +%s"), + format!("touch -d '{input}' /tmp/rejected"), + format!("find /tmp -newermt '{input}'"), + ] { + let result = bash.exec(&script).await.unwrap(); + assert_eq!(result.exit_code, 1, "{script}: {}", result.stderr); + assert!(result.stdout.is_empty(), "{script}: {}", result.stdout); + assert!(!result.stderr.is_empty()); + } + assert_eq!( + bash.exec("test -e /tmp/rejected").await.unwrap().exit_code, + 1 + ); + } +} + +#[tokio::test] +async fn weekday_midnight_observes_dst_rules() { + let mut bash = Bash::builder() + .fixed_epoch(1_710_093_600) + .env("TZ", "America/Chicago") + .build(); // 2024-03-10 18:00 UTC + let result = bash + .exec("date -d sunday '+%F %T %z'; date -d 'next sunday' '+%F %T %z'") + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!( + result.stdout, + "2024-03-10 00:00:00 -0600\n2024-03-17 00:00:00 -0500\n" + ); + // Sao Paulo skipped midnight on this date: reject instead of inventing an instant. + let mut bash = Bash::builder() + .fixed_epoch(1_541_332_800) + .env("TZ", "America/Sao_Paulo") + .build(); // 2018-11-04 12:00 UTC + let result = bash.exec("date -d sunday +%s").await.unwrap(); + assert_eq!(result.exit_code, 1); + assert!(result.stdout.is_empty()); +} diff --git a/crates/bashkit/tests/spec_cases/bash/date.test.sh b/crates/bashkit/tests/spec_cases/bash/date.test.sh index b7c7d1c36..1e8ac1e0c 100644 --- a/crates/bashkit/tests/spec_cases/bash/date.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/date.test.sh @@ -171,6 +171,18 @@ date -d 'tomorrow' +%Y-%m-%d | grep -qE '^[0-9]{4}-[0-9]{2}-[0-9]{2}$' && echo " valid ### end +### date_next_weekday +date -d 'next monday' '+%u %T' +### expect +1 00:00:00 +### end + +### date_unknown_word_is_invalid +date -d 'next blursday' 2>/dev/null || echo invalid +### expect +invalid +### end + ### date_set_time ### skip: date -s (set time) not implemented and requires privileges date -s '2024-01-01 12:00:00' 2>/dev/null || echo "skip" diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index 19c466ad7..a6ff7f1e9 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -151,6 +151,18 @@ fractional file timestamps, strict newer-than comparisons, and rejection before file effects; `date_timezone_differential_tests` compares supported forms with GNU date when available. +The same parser accepts English full weekday names and three-letter +abbreviations (case-insensitive), bare or preceded by `this`, `next`, or `last`. +They resolve at midnight on the selected sandbox calendar day: bare/`this` +include today, `next` excludes today, and `last` selects the previous occurrence. +`today` preserves the current virtual instant, as `now` does. Unknown relative +units are errors; abbreviations such as `next mins`, numeric weekday ordinals, +and weekday zone suffixes remain unsupported. Midnight gaps are rejected and +folds choose the earlier instant, using the existing naive-date policy; virtual +clock conversion and weekday arithmetic reject dates outside chrono's range. +`shared_weekday_parsing_*`, `weekday_midnight_observes_dst_rules`, +`weekday_*_checks_range`, and the GNU-date differential cover these boundaries. + ### CPython runtime (`cpython` feature) Boundaries of the WebAssembly CPython guest; see From 8f808d841be0563ee3b796e3a974afd61faefa0c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sat, 10 Oct 2026 18:13:36 -0500 Subject: [PATCH 3/4] fix(arithmetic): bound nested dollar expansion (#2668) ## 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) --- crates/bashkit/src/interpreter/arithmetic.rs | 256 +++++++++++++----- crates/bashkit/src/interpreter/mod.rs | 34 ++- .../integration/arithmetic_resource_tests.rs | 143 ++++++++++ crates/bashkit/tests/integration/main.rs | 1 + docs/security.md | 5 + knowledge/foundations/architecture.md | 12 + knowledge/security/threat-model.md | 6 +- 7 files changed, 372 insertions(+), 85 deletions(-) create mode 100644 crates/bashkit/tests/integration/arithmetic_resource_tests.rs diff --git a/crates/bashkit/src/interpreter/arithmetic.rs b/crates/bashkit/src/interpreter/arithmetic.rs index dc8838019..649d8129d 100644 --- a/crates/bashkit/src/interpreter/arithmetic.rs +++ b/crates/bashkit/src/interpreter/arithmetic.rs @@ -14,7 +14,9 @@ //! `arith_error` and the command boundary aborts the line. //! - Integers wrap at 64 bits; shift counts are masked to 0..63 (x86 / bash). //! - THREAT[TM-DOS-026]: recursion depth (`MAX_ARITHMETIC_DEPTH`) and fuel -//! (`MAX_ARITHMETIC_EXPANSION_FUEL`) bound nested and self-referential input. +//! (`MAX_ARITHMETIC_EXPANSION_FUEL`) are shared by textual dollar expansion, +//! subscript evaluation, and recursive variable evaluation. Check before +//! expansion; read-only subscript evaluators borrow the same budget. use super::*; @@ -211,14 +213,40 @@ struct LValue { type ArithResult = std::result::Result; -/// Expression evaluator state shared across recursive variable evaluation. -pub(super) struct ArithEval<'a> { +/// One budget for both textual expansion and recursive expression evaluation. +struct ArithmeticBudget { + depth: usize, + fuel: usize, +} + +impl ArithmeticBudget { + fn new() -> Self { + Self { + depth: 0, + fuel: Interpreter::MAX_ARITHMETIC_EXPANSION_FUEL, + } + } + + fn charge(&mut self, src: &str) -> ArithResult<()> { + if src.len() > Interpreter::MAX_ARITHMETIC_EXPANSION_BYTES { + return Err("expression too long".to_string()); + } + let cost = src.len().max(1); + self.fuel = self + .fuel + .checked_sub(cost) + .ok_or_else(|| "expression recursion level exceeded".to_string())?; + Ok(()) + } +} + +/// Writes are evaluator-local; read-only subscript evaluations share only the budget. +struct ArithEval<'a, 'b> { interp: &'a Interpreter, overlay: HashMap<(String, Option), String>, - pub(super) writes: Vec, + writes: Vec, noeval: u32, - depth: usize, - fuel: usize, + budget: &'b mut ArithmeticBudget, } struct ArithParser<'s> { @@ -258,39 +286,39 @@ impl ArithParser<'_> { } } -impl<'a> ArithEval<'a> { - pub(super) fn new(interp: &'a Interpreter) -> Self { +impl<'a, 'b> ArithEval<'a, 'b> { + fn new(interp: &'a Interpreter, budget: &'b mut ArithmeticBudget) -> Self { Self { interp, overlay: HashMap::new(), writes: Vec::new(), noeval: 0, - depth: 0, - fuel: Interpreter::MAX_ARITHMETIC_EXPANSION_FUEL, + budget, } } fn enter(&mut self) -> ArithResult<()> { - self.depth += 1; - if self.depth >= Interpreter::MAX_ARITHMETIC_DEPTH { + if self.budget.depth + 1 >= Interpreter::MAX_ARITHMETIC_DEPTH { return Err("expression recursion level exceeded".to_string()); } + self.budget.depth += 1; Ok(()) } fn leave(&mut self) { - self.depth -= 1; + self.budget.depth -= 1; } /// Evaluate a full expression (already `$`-expanded). Empty -> 0. - pub(super) fn eval_str(&mut self, src: &str) -> ArithResult { - if src.len() > Interpreter::MAX_ARITHMETIC_EXPANSION_BYTES { - return Err("expression too long".to_string()); - } - if self.fuel < src.len().max(1) { - return Err("expression recursion level exceeded".to_string()); - } - self.fuel -= src.len().max(1); + fn eval_str(&mut self, src: &str) -> ArithResult { + self.enter()?; + let result = self.eval_str_inner(src); + self.leave(); + result + } + + fn eval_str_inner(&mut self, src: &str) -> ArithResult { + self.budget.charge(src)?; if src.trim().is_empty() { return Ok(0); } @@ -299,16 +327,28 @@ impl<'a> ArithEval<'a> { format!("{msg} (error token is \"{}\")", diag_echo(rest)) })?; let mut p = ArithParser { src, toks, pos: 0 }; - self.enter()?; - let r = self.comma(&mut p); - self.leave(); - let v = r?; + let v = self.comma(&mut p)?; if !matches!(p.peek(), Tok::Eof) { return Err(p.err("syntax error in expression")); } Ok(v) } + /// Guard and charge the original text before expanding any recursive subscript. + fn eval_with_dollars(&mut self, src: &str) -> ArithResult { + if !src.contains('$') && !src.contains('"') { + return self.eval_str(src); + } + self.enter()?; + let result = (|| { + self.budget.charge(src)?; + let expanded = self.expand_arith_dollars(src)?; + self.eval_str(&expanded) + })(); + self.leave(); + result + } + fn comma(&mut self, p: &mut ArithParser) -> ArithResult { let mut v = self.assign(p)?; while p.is_op(",") { @@ -829,12 +869,20 @@ impl Interpreter { /// Evaluate `expr`, returning the value or an error message, plus the /// assignments made (in order, including those before an error). pub(super) fn arith_eval(&self, expr: &str) -> (ArithResult, Vec) { - let expanded = self.expand_arith_dollars(expr); - let mut ev = ArithEval::new(self); - let r = ev.eval_str(&expanded); + let mut budget = ArithmeticBudget::new(); + let mut ev = ArithEval::new(self, &mut budget); + let r = ev.eval_with_dollars(expr); (r, ev.writes) } + /// Conditional operands have already undergone shell expansion. + pub(super) fn arith_eval_unexpanded(&self, expr: &str) -> (ArithResult, Vec) { + let mut budget = ArithmeticBudget::new(); + let mut ev = ArithEval::new(self, &mut budget); + let result = ev.eval_str(expr); + (result, ev.writes) + } + /// Apply writes produced by [`arith_eval`](Self::arith_eval). pub(super) fn apply_arith_writes(&mut self, writes: Vec) { for (name, key, value) in writes { @@ -934,12 +982,75 @@ impl Interpreter { self.arith_error.lock().ok().and_then(|mut s| s.take()) } + /// Read-only parameter/redirect lookup uses the same guarded expansion path. + pub(super) fn expand_name_or_array_element(&self, name: &str) -> String { + let mut budget = ArithmeticBudget::new(); + let mut ev = ArithEval::new(self, &mut budget); + match ev.expand_name_or_array_element(name) { + Ok(value) => value, + Err(msg) => { + self.record_arith_error(msg); + String::new() + } + } + } +} + +impl ArithEval<'_, '_> { + fn expand_variable(&mut self, name: &str) -> ArithResult { + self.enter()?; + let result = { + let resolved = self.interp.resolve_nameref(name); + if parse_embedded_array_ref(resolved).is_some() { + self.expand_name_or_array_element(resolved) + } else { + Ok(self.interp.expand_variable(resolved)) + } + }; + self.leave(); + result + } + + fn expand_variable_or_literal(&mut self, sub: &str) -> ArithResult { + let trimmed = sub.trim(); + if !trimmed.contains(['"', '\'', '\\']) + && trimmed.matches('$').count() <= 1 + && let Some(name) = trimmed.strip_prefix('$') + { + return self.expand_variable(name.trim_start_matches('{').trim_end_matches('}')); + } + if !sub.contains(['$', '"', '\'', '\\']) { + return Ok(sub.to_string()); + } + Interpreter::expand_key_text_using(sub, |name, braced| { + if braced { + if let Some(array) = name + .strip_suffix("[@]") + .or_else(|| name.strip_suffix("[*]")) + { + let separator = if name.ends_with("[*]") { + self.interp.get_ifs_separator() + } else { + " ".to_string() + }; + return Ok(self + .interp + .array_values(self.interp.resolve_nameref(array)) + .join(&separator)); + } + self.expand_brace_expr_in_arithmetic(name) + } else { + self.expand_variable(name) + } + }) + } + /// Textual `$` expansion inside an arithmetic expression (`$x`, `${..}`, /// `$1`, `$#`, ...). Bare identifiers are left for the evaluator. /// Double quotes are dropped (`"$x"` is `$x`). - pub(super) fn expand_arith_dollars(&self, expr: &str) -> String { + fn expand_arith_dollars(&mut self, expr: &str) -> ArithResult { if !expr.contains('$') && !expr.contains('"') { - return expr.to_string(); + return Ok(expr.to_string()); } let expr = expr.replace('"', ""); let mut result = String::new(); @@ -970,10 +1081,11 @@ impl Interpreter { } // THREAT[TM-DOS-130]: nesting recurses; bash-like scripts need few levels. if inner.matches("$((").count() > 32 { - self.record_arith_error("expression recursion level exceeded".to_string()); - result.push('0'); + return Err("expression recursion level exceeded".to_string()); } else { - result.push_str(&self.evaluate_arithmetic(&inner).to_string()); + let value = + ArithEval::new(self.interp, self.budget).eval_with_dollars(&inner)?; + result.push_str(&value.to_string()); } } else if chars.peek() == Some(&'{') { chars.next(); @@ -990,12 +1102,12 @@ impl Interpreter { } brace_content.push(c); } - result.push_str(&self.expand_brace_expr_in_arithmetic(&brace_content)); + result.push_str(&self.expand_brace_expr_in_arithmetic(&brace_content)?); } else if let Some(&c) = chars.peek() && matches!(c, '#' | '?' | '$' | '!' | '@' | '*' | '-' | '0'..='9') { chars.next(); - result.push_str(&self.expand_variable(&c.to_string())); + result.push_str(&self.expand_variable(&c.to_string())?); } else { let mut name = String::new(); while let Some(&c) = chars.peek() { @@ -1009,42 +1121,42 @@ impl Interpreter { if name.is_empty() { result.push('$'); } else { - result.push_str(&self.expand_variable(&name)); + result.push_str(&self.expand_variable(&name)?); } } } - result + Ok(result) } /// Expand a `${...}` expression encountered inside arithmetic context. /// Handles: `${#arr[@]}`, `${#arr[*]}`, `${#var}`, `${arr[idx]}`, `${var}`. - pub(super) fn expand_brace_expr_in_arithmetic(&self, inner: &str) -> String { + fn expand_brace_expr_in_arithmetic(&mut self, inner: &str) -> ArithResult { // ${#arr[@]} or ${#arr[*]} — array length if let Some(rest) = inner.strip_prefix('#') { if let Some(bracket) = rest.find('[') { if !rest.ends_with(']') { - return "0".to_string(); + return Ok("0".to_string()); } let end = rest.len() - 1; if bracket + 1 > end { - return "0".to_string(); + return Ok("0".to_string()); } let arr_name = &rest[..bracket]; let idx = &rest[bracket + 1..end]; if idx == "@" || idx == "*" { - if let Some(arr) = self.scoped.arrays.get(arr_name) { - return arr.len().to_string(); + if let Some(arr) = self.interp.scoped.arrays.get(arr_name) { + return Ok(arr.len().to_string()); } - if let Some(arr) = self.scoped.assoc_arrays.get(arr_name) { - return arr.len().to_string(); + if let Some(arr) = self.interp.scoped.assoc_arrays.get(arr_name) { + return Ok(arr.len().to_string()); } - return "0".to_string(); + return Ok("0".to_string()); } - let val = self.expand_name_or_array_element(&rest[..=end]); - return self.shell_length(&val).to_string(); + let val = self.expand_name_or_array_element(&rest[..=end])?; + return Ok(self.interp.shell_length(&val).to_string()); } - let val = self.expand_variable(rest); - return self.shell_length(&val).to_string(); + let val = self.expand_variable(rest)?; + return Ok(self.interp.shell_length(&val).to_string()); } if let Some(bracket) = inner.find('[') @@ -1067,37 +1179,37 @@ impl Interpreter { /// Expand a parameter expansion with operators inside arithmetic context. /// Handles common cases like ${var%%-*}, ${var##prefix}, etc. - pub(super) fn expand_param_op_in_arithmetic(&self, inner: &str) -> String { + fn expand_param_op_in_arithmetic(&mut self, inner: &str) -> ArithResult { for (pos, ch) in inner.char_indices() { match ch { '%' => { let name = &inner[..pos]; - let value = self.expand_name_or_array_element(name); + let value = self.expand_name_or_array_element(name)?; if inner[pos..].starts_with("%%") { let pattern = &inner[pos + 2..]; - return self.remove_pattern(&value, pattern, false, true); + return Ok(self.interp.remove_pattern(&value, pattern, false, true)); } let pattern = &inner[pos + 1..]; - return self.remove_pattern(&value, pattern, false, false); + return Ok(self.interp.remove_pattern(&value, pattern, false, false)); } '#' if pos > 0 => { let name = &inner[..pos]; - let value = self.expand_name_or_array_element(name); + let value = self.expand_name_or_array_element(name)?; if inner[pos..].starts_with("##") { let pattern = &inner[pos + 2..]; - return self.remove_pattern(&value, pattern, true, true); + return Ok(self.interp.remove_pattern(&value, pattern, true, true)); } let pattern = &inner[pos + 1..]; - return self.remove_pattern(&value, pattern, true, false); + return Ok(self.interp.remove_pattern(&value, pattern, true, false)); } ':' if inner[pos..].starts_with(":-") => { let name = &inner[..pos]; let default = &inner[pos + 2..]; - let value = self.expand_name_or_array_element(name); + let value = self.expand_name_or_array_element(name)?; if value.is_empty() { - return default.to_string(); + return Ok(default.to_string()); } - return value; + return Ok(value); } _ => {} } @@ -1109,28 +1221,34 @@ impl Interpreter { /// Used by parameter expansion inside arithmetic so `${arr[$key]:-N}` and /// friends can read associative/indexed array elements — `expand_variable` /// alone only handles scalar names. Fixes issue #1776. - pub(super) fn expand_name_or_array_element(&self, name: &str) -> String { + fn expand_name_or_array_element(&mut self, name: &str) -> ArithResult { if let Some(bracket) = name.find('[') && name.ends_with(']') { let arr_name = &name[..bracket]; - let resolved = self.resolve_nameref(arr_name); + let resolved = self.interp.resolve_nameref(arr_name); let idx_str = strip_subscript_quotes(&name[bracket + 1..name.len() - 1]); - if let Some(arr) = self.scoped.assoc_arrays.get(resolved) { - let key = self.expand_variable_or_literal(idx_str); - return arr.get(&key).cloned().unwrap_or_default(); + if let Some(arr) = self.interp.scoped.assoc_arrays.get(resolved) { + let key = self.expand_variable_or_literal(idx_str)?; + return Ok(arr.get(&key).cloned().unwrap_or_default()); } - if let Some(arr) = self.scoped.arrays.get(resolved) { - let idx = self.resolve_indexed_array_subscript(resolved, idx_str); - return arr.get(&idx).cloned().unwrap_or_default(); + if let Some(arr) = self.interp.scoped.arrays.get(resolved) { + let idx = self.resolve_indexed_array_subscript(resolved, idx_str)?; + return Ok(arr.get(&idx).cloned().unwrap_or_default()); } - if self.resolve_indexed_array_subscript(resolved, idx_str) == 0 { + if self.resolve_indexed_array_subscript(resolved, idx_str)? == 0 { return self.expand_variable(resolved); } - return String::new(); + return Ok(String::new()); } self.expand_variable(name) } + fn resolve_indexed_array_subscript(&mut self, name: &str, sub: &str) -> ArithResult { + // Preserve read-only dollar-subscript behavior: discard child writes, + // but never refresh depth or fuel on a recursive entry. + let raw = ArithEval::new(self.interp, self.budget).eval_with_dollars(sub)?; + Ok(self.interp.normalize_indexed_array_subscript(name, raw)) + } } /// Drop one layer of matching quotes around a subscript: `c["x y"]` -> key `x y`. diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 0ec20f358..87a32e96d 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -6046,9 +6046,7 @@ impl Interpreter { fn cond_int_cmp(&mut self, left: &str, right: &str, cmp: fn(i64, i64) -> bool) -> bool { let mut values = [0i64; 2]; for (slot, operand) in values.iter_mut().zip([left, right]) { - let mut ev = arithmetic::ArithEval::new(self); - let r = ev.eval_str(operand); - let writes = std::mem::take(&mut ev.writes); + let (r, writes) = self.arith_eval_unexpanded(operand); self.apply_arith_writes(writes); match r { Ok(v) => *slot = v, @@ -13079,8 +13077,10 @@ impl Interpreter { /// [`Self::expand_key_text`]; `plain` reads only plain scalar values /// (no nameref, array or special-parameter resolution). fn expand_key_text_with(&self, s: &str, plain: bool) -> String { - let lookup = |name: &str| -> String { - if plain { + let lookup = |name: &str, braced: bool| { + Ok::<_, std::convert::Infallible>(if !plain && !braced { + self.expand_variable(name) + } else if plain { self.scoped.variables.get(name).cloned().unwrap_or_default() } else if let Some(arr) = name .strip_suffix("[@]") @@ -13096,8 +13096,16 @@ impl Interpreter { self.array_values(self.resolve_nameref(arr)).join(&sep) } else { self.resolve_param_expansion_name(name).1 - } + }) }; + Self::expand_key_text_using(s, lookup).unwrap() + } + + /// One quote/key scanner; callers own lookup semantics and recursion budgets. + fn expand_key_text_using( + s: &str, + mut lookup: impl FnMut(&str, bool) -> std::result::Result, + ) -> std::result::Result { let mut out = String::new(); let mut chars = s.chars().peekable(); while let Some(c) = chars.next() { @@ -13121,7 +13129,7 @@ impl Interpreter { } name.push(n); } - out.push_str(&lookup(&name)); + out.push_str(&lookup(&name, true)?); } '$' if chars .peek() @@ -13135,16 +13143,12 @@ impl Interpreter { name.push(n); chars.next(); } - out.push_str(&if plain { - lookup(&name) - } else { - self.expand_variable(&name) - }); + out.push_str(&lookup(&name, false)?); } _ => out.push(c), } } - out + Ok(out) } /// Fully expand an associative array key using standard word expansion. @@ -13540,6 +13544,10 @@ impl Interpreter { fn resolve_indexed_array_subscript(&self, arr_name: &str, key: &str) -> usize { let raw_idx = self.evaluate_arithmetic(key); + self.normalize_indexed_array_subscript(arr_name, raw_idx) + } + + fn normalize_indexed_array_subscript(&self, arr_name: &str, raw_idx: i64) -> usize { if raw_idx < 0 { let len = self .scoped diff --git a/crates/bashkit/tests/integration/arithmetic_resource_tests.rs b/crates/bashkit/tests/integration/arithmetic_resource_tests.rs new file mode 100644 index 000000000..4f108dc85 --- /dev/null +++ b/crates/bashkit/tests/integration/arithmetic_resource_tests.rs @@ -0,0 +1,143 @@ +// THREAT[TM-DOS-026]: textual expansion and subscript evaluation share limits. +use bashkit::Bash; + +fn nested_array_expansion(levels: usize, length: bool) -> String { + let mut expr = "0".to_string(); + for _ in 0..levels { + expr = if length { + format!("${{#a[{expr}]}}") + } else { + format!("${{a[{expr}]}}") + }; + } + expr +} + +#[tokio::test] +async fn arithmetic_dollar_subscripts_stop_at_recursion_limit() { + // Bounded input exercises the guard without probing a host crash threshold. + for length in [false, true] { + let mut bash = Bash::new(); + let expr = nested_array_expansion(51, length); + let result = bash.exec(&format!("a=(0); let '{expr}'")).await.unwrap(); + assert_ne!(result.exit_code, 0); + assert!( + result + .stderr + .contains("expression recursion level exceeded"), + "{}", + result.stderr + ); + let recovery = bash.exec("echo recovered").await.unwrap(); + assert_eq!(recovery.stdout, "recovered\n"); + assert_eq!(recovery.exit_code, 0); + assert!(recovery.stderr.is_empty()); + } +} + +#[tokio::test] +async fn arithmetic_dollar_siblings_share_expansion_fuel() { + let mut bash = Bash::new(); + let term = nested_array_expansion(10, false); + // Source and individual terms fit. Aggregate recursive work exceeds the fuel. + let expr = format!("{}+1", vec![term; 30].join("+")); + assert!(expr.len() < 8192); + let result = bash.exec(&format!("a=(0); let '{expr}'")).await.unwrap(); + assert_eq!(result.exit_code, 1); + assert!( + result + .stderr + .contains("expression recursion level exceeded") + ); +} + +#[tokio::test] +async fn arithmetic_dollar_subscripts_match_bash_within_limits() { + let expr = nested_array_expansion(4, false); + // NOTE: subscript stays portable on purpose: macOS ships bash 3.2 with no + // negative-index support while Linux CI has bash 5.x, so `${a[-1]}` would + // make this differential assertion environment-dependent. `i-0` still + // exercises arithmetic inside the subscript on every bash. + let script = format!( + "a=(0 7); n=123; i=1; echo $(({expr}+${{a[i]}})); \ + echo $((${{#a[1]}}+${{n%3}})); echo $((${{a[i-0]}}))" + ); + let expected = std::process::Command::new("bash") + .args(["--noprofile", "--norc", "-c", &script]) + .output() + .unwrap(); + assert!(expected.status.success()); + let mut bash = Bash::new(); + let result = bash.exec(&script).await.unwrap(); + assert_eq!(result.exit_code, 0); + assert!(result.stderr.is_empty(), "{}", result.stderr); + assert_eq!(result.stdout.as_bytes(), expected.stdout); +} + +#[tokio::test] +async fn arithmetic_quoted_let_keeps_shallow_dollar_expansion() { + let mut bash = Bash::new(); + let expr = nested_array_expansion(4, false); + let result = bash + .exec(&format!("a=(0 7); let 'x={expr}+${{a[1]}}'; echo \"$x\"")) + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, "7\n"); + assert!(result.stderr.is_empty()); +} + +#[tokio::test] +async fn arithmetic_nameref_and_assoc_keys_cannot_restart_the_budget() { + let expr = nested_array_expansion(51, false); + for usage in [ + "$r", + "${r}", + "${#r}", + "${m[$r]}", + "${m[k$r]}", + "${m[k${r}]}", + ] { + let mut bash = Bash::new(); + let result = bash + .exec(&format!( + "a=(0); declare -A m; declare -n r='a[{expr}]'; let '{usage}'" + )) + .await + .unwrap(); + assert_eq!(result.exit_code, 1, "{usage}"); + assert!( + result + .stderr + .contains("expression recursion level exceeded"), + "{usage}: {}", + result.stderr + ); + } +} + +#[tokio::test] +async fn nested_arithmetic_dollars_share_fuel_with_subscripts() { + let term = format!("$(({}))", nested_array_expansion(10, false)); + let expr = format!("{}+1", vec![term; 30].join("+")); + let mut bash = Bash::new(); + let result = bash.exec(&format!("a=(0); let '{expr}'")).await.unwrap(); + assert_eq!(result.exit_code, 1); + assert!( + result + .stderr + .contains("expression recursion level exceeded") + ); +} + +#[tokio::test] +async fn dollar_subscript_writes_remain_read_only() { + let mut bash = Bash::new(); + let result = bash + .exec("a=(0 7); i=0; let 'x=${a[i=1]}'; echo \"$i $x\"") + .await + .unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout, "0 7\n"); + assert!(result.stderr.is_empty()); +} diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index 8d649db77..3bc4432a5 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -17,6 +17,7 @@ pub mod agent_skills_publication_tests; pub mod allexport_tests; pub mod archive_bzip2_tests; pub mod arithmetic_fuzz_scaffold_tests; +pub mod arithmetic_resource_tests; pub mod array_budget_security_tests; pub mod awk_fuzz_scaffold_tests; pub mod awk_newline_tests; diff --git a/docs/security.md b/docs/security.md index d8f1e66fd..91dc142b8 100644 --- a/docs/security.md +++ b/docs/security.md @@ -115,6 +115,11 @@ behaviour on malformed input. behaviour parity where expected, and confirm intentional divergences where security requires it. +Arithmetic limits cover textual dollar expansion inside expressions. +Nested array subscripts share the enclosing expression's recursion and work +limits, including quoted arguments to `let`. Exceeding either limit returns +an arithmetic error; subsequent executions can reuse the session. + ## Panic safety All builtin commands are wrapped with `catch_unwind`. If a builtin panics, the diff --git a/knowledge/foundations/architecture.md b/knowledge/foundations/architecture.md index c2e8d822a..b6d2bb4c2 100644 --- a/knowledge/foundations/architecture.md +++ b/knowledge/foundations/architecture.md @@ -92,6 +92,18 @@ process-local scheduling, not a serializable continuation; snapshots still capture state only between executions. See [Builtin Commands](builtins.md) and [Snapshot History](snapshot-history.md). +### Arithmetic expansion budget + +Arithmetic's synchronous evaluator owns one depth/fuel budget per top-level +expression. Dollar expansion checks and charges original text before +recursing into array subscripts or nested `$((...))`. Read-only child +evaluators borrow that budget while discarding writes; siblings cannot +restart fuel. Nameref and associative-key lookups stay in the same budget. +Recursive bare-variable evaluation and syntax parsing share the depth +counter. Expansion errors propagate to the arithmetic boundary; later host +executions receive fresh state. Regression and Bash differential coverage: +`crates/bashkit/tests/integration/arithmetic_resource_tests.rs` (TM-DOS-026). + ### Shared execution budget Each `exec_with_options` creates exactly one `ExecutionBudget` before hooks or diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index a5896ba04..db87172e1 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -240,10 +240,10 @@ runaway scripts without permanently breaking the session. | TM-DOS-020 | Function recursion | `f() { f; }; f` | Effective depth limit is at most 16 even if an embedder requests more; the previous default 100 and hardened 64 could overflow a 2 MiB host stack before the counter fired (#2459). The CLI subprocess test runs direct and mutual recursion on that stack size. | **MITIGATED** | | TM-DOS-021 | Command sub nesting | `$($($($())))` | Child parsers inherit remaining depth budget + fuel from parent | **MITIGATED** | | TM-DOS-022 | Parser recursion | Deeply nested `(((())))` | `max_ast_depth` limit (100) + `HARD_MAX_AST_DEPTH` cap (100) | **MITIGATED** | -| TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))` | `MAX_ARITHMETIC_DEPTH` limit (50), shared expansion fuel, cycle detection, expanded-size cap | **MITIGATED** | +| TM-DOS-026 | Arithmetic recursion and expansion amplification | `$((((...))))`, `a=a+a; $((a))`, `i=arr[i]; $((arr[i]))`, nested `${a[${a[...]}]}` passed to `let` | `MAX_ARITHMETIC_DEPTH` limit (50) checked before dollar expansion and tokenization; shared fuel across textual expansion, nested read-only subscripts, nameref/key lookup, and recursive variable evaluation; expanded-size cap | **MITIGATED** | | TM-DOS-064 | Heredoc suffix re-injection CPU amplification | Many `: < Date: Sat, 10 Oct 2026 18:15:53 -0500 Subject: [PATCH 4/4] test(security): cover repeated-paren conditional print PoC (declare -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. --- .../integration/blackbox_security_tests.rs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/crates/bashkit/tests/integration/blackbox_security_tests.rs b/crates/bashkit/tests/integration/blackbox_security_tests.rs index 1f0c6bb1f..4a316137a 100644 --- a/crates/bashkit/tests/integration/blackbox_security_tests.rs +++ b/crates/bashkit/tests/integration/blackbox_security_tests.rs @@ -2060,4 +2060,83 @@ mod finding_cond_bare_not_print_overflow { result.stdout ); } + + /// Exact PoC shape: repeated parenthesized conditionals in an unexecuted + /// function, then `declare -f`. Deep-but-valid nesting (200x) must define + /// and print without overflowing; the shell stays alive and the function + /// is never executed. + #[tokio::test] + async fn repeated_paren_nesting_valid_max_prints() { + let mut bash = tight_bash(); + let script = format!( + "f() {{ [[ {}-n x{} ]]; }}; declare -f f", + "( ".repeat(200), + " )".repeat(200) + ); + let result = bash.exec(&script).await.unwrap(); + assert!( + result.stderr.is_empty(), + "valid nested parens must not error, got stderr: {:?}", + result.stderr + ); + assert!( + result.stdout.contains("declare -f") || result.stdout.contains("f ()"), + "declare -f must print the function, got stdout: {:?}", + result.stdout + ); + let alive = bash.exec("echo alive").await.unwrap(); + assert!(alive.stdout.contains("alive")); + } + + /// Exact PoC shape via `type`, and excessive nesting (300x) rejected + /// cleanly on both the `declare -f` and `type` paths — no crash, no hang. + #[tokio::test] + async fn repeated_paren_nesting_excessive_is_bounded() { + for printer in ["declare -f f", "type f"] { + let mut bash = tight_bash(); + let script = format!( + "f() {{ [[ {}-n x{} ]]; }}; {}", + "( ".repeat(300), + " )".repeat(300), + printer + ); + let err = format!("{:?}", bash.exec(&script).await.unwrap_err()); + assert!( + err.contains("nested too deeply") + || err.contains("syntax error in conditional expression"), + "{}: expected depth/syntax rejection, got: {}", + printer, + err + ); + let alive = bash.exec("echo alive").await.unwrap(); + assert!( + alive.stdout.contains("alive"), + "{}: shell must survive, got stdout: {:?}", + printer, + alive.stdout + ); + } + } + + /// Valid nested parens resolve through `type` as well. + #[tokio::test] + async fn repeated_paren_nesting_valid_type_prints() { + let mut bash = tight_bash(); + let script = format!( + "f() {{ [[ {}-n x{} ]]; }}; type f", + "( ".repeat(80), + " )".repeat(80) + ); + let result = bash.exec(&script).await.unwrap(); + assert!( + result.stderr.is_empty(), + "valid nested parens must not error, got stderr: {:?}", + result.stderr + ); + assert!( + result.stdout.contains("-n x"), + "type must print the conditional body, got stdout: {:?}", + result.stdout + ); + } }