From 19172d435cf6d0cbbe5dd7652122fab9d9768b3c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 01:27:19 -0500 Subject: [PATCH 1/2] fix(jq): abort error-preview traversal at budget instead of walking hostile values dump_trunc's Capped sink swallowed over-budget writes with Ok, so Val's Display (?-laden format_val!/write_seq!) kept expanding shared-structure values (e.g. 2^40-element math operands) just to discard the output. write_str now reports fmt::Error once the 15-byte budget is spent, which aborts the traversal, and the ... marker is unconditional on over-budget previews. Small-value output is byte-identical. --- .../bashkit/src/builtins/jq/jaq_json/jqmsg.rs | 98 ++++++++++++++----- crates/bashkit/src/builtins/jq/tests.rs | 15 +++ knowledge/runtimes/jaq-json-vendor.md | 2 +- 3 files changed, 89 insertions(+), 26 deletions(-) diff --git a/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs b/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs index b1e853489..6a0995dd1 100644 --- a/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs +++ b/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs @@ -24,48 +24,57 @@ fn type_name(v: &Val) -> &'static str { /// value never materializes its full dump as an intermediate string. /// (Truncating after `v.to_string()` would still transiently allocate the /// whole rendering outside the memory meter and the 64 KiB diagnostic cap.) -fn dump_trunc(v: &Val) -> String { - /// `fmt::Write` sink that keeps at most `cap` bytes; excess is dropped. - struct Capped { - buf: String, - cap: usize, - over: bool, - } - impl std::fmt::Write for Capped { - fn write_str(&mut self, s: &str) -> std::fmt::Result { - if self.buf.len() >= self.cap { - self.over = true; - return Ok(()); - } - let room = self.cap - self.buf.len(); - let mut end = s.len().min(room); - while !s.is_char_boundary(end) { - end -= 1; - } - self.buf.push_str(&s[..end]); - if end < s.len() { - self.over = true; - } - Ok(()) +/// `fmt::Write` sink that keeps at most `cap` bytes. Once the budget is +/// spent it reports `fmt::Error` so the `?`-laden display machinery +/// (`format_val!`/`write_seq!` in `write.rs`) aborts the traversal instead +/// of walking the rest of a hostile value just to discard it. +struct Capped { + buf: String, + cap: usize, + over: bool, +} +impl std::fmt::Write for Capped { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + if self.buf.len() >= self.cap { + self.over = true; + return Err(std::fmt::Error); } + let room = self.cap - self.buf.len(); + let mut end = s.len().min(room); + while !s.is_char_boundary(end) { + end -= 1; + } + self.buf.push_str(&s[..end]); + if end < s.len() { + self.over = true; + return Err(std::fmt::Error); + } + Ok(()) } +} +fn dump_trunc(v: &Val) -> String { use std::fmt::Write as _; // One byte past the 14-byte cut: enough to tell "fits" from "truncate", // so small-value output is byte-identical to the old truncation. let mut out = Capped { buf: String::new(), cap: 15, over: false }; - let _ = write!(out, "{v}"); + if write!(out, "{v}").is_err() { + out.over = true; + } let mut s = out.buf; if !out.over && s.len() <= 14 { return s; } + // Over budget: the preview dropped content (an aborted traversal may not + // even have filled the buffer, e.g. a multibyte char straddling the cut + // aborts before the closing quote), so the `...` marker is unconditional. if s.len() > 14 { let mut end = 11; while !s.is_char_boundary(end) { end -= 1; } s.truncate(end); - s.push_str("..."); } + s.push_str("..."); s } @@ -151,4 +160,43 @@ mod tests { let err = index(l, r).to_string(); assert!(err.len() < 4096, "index error materialized {} bytes", err.len()); } + + #[test] + fn math_error_stays_small_for_huge_operands() { + let err = math(huge_array_key(), ops::Math::Add, huge_array_key(), false).to_string(); + assert!(err.len() < 4096, "math error materialized {} bytes", err.len()); + } + + #[test] + fn preview_exact_fit_is_not_truncated() { + // A 14-byte rendering fits the 15-byte cap: returned verbatim. + assert_eq!(dump_trunc(&Val::utf8_str("123456789012")), "\"123456789012\""); + } + + #[test] + fn preview_just_over_truncates_with_ellipsis() { + // A 15-byte rendering fills the cap: kept prefix plus `...`. + assert_eq!(dump_trunc(&Val::utf8_str("1234567890123")), "\"1234567890..."); + } + + #[test] + fn preview_never_splits_a_multibyte_char() { + // Multi-byte content straddling the cut: valid UTF-8, always marked. + assert_eq!(dump_trunc(&Val::utf8_str("é".repeat(20))), "\"ééééé..."); + assert_eq!(dump_trunc(&Val::utf8_str("😀".repeat(10))), "\"😀😀😀..."); + } + + #[test] + fn preview_recovers_after_truncation() { + let _ = math(huge_array_key(), ops::Math::Add, huge_array_key(), false).to_string(); + assert_eq!(dump_trunc(&Val::utf8_str("ok")), "\"ok\""); + } + + #[test] + fn preview_writer_aborts_once_over_budget() { + use std::fmt::Write as _; + let mut out = Capped { buf: String::new(), cap: 15, over: false }; + let res = write!(out, "{}", "y".repeat(200_000)); + assert!(res.is_err(), "preview writer must abort instead of walking the whole value"); + } } diff --git a/crates/bashkit/src/builtins/jq/tests.rs b/crates/bashkit/src/builtins/jq/tests.rs index fc1fab2bd..d40b35305 100644 --- a/crates/bashkit/src/builtins/jq/tests.rs +++ b/crates/bashkit/src/builtins/jq/tests.rs @@ -2042,6 +2042,21 @@ async fn math_errors_use_jq_wording() { ); } +#[tokio::test] +async fn math_error_on_huge_operands_stays_bounded_and_session_recovers() { + let out = run_jq_with_args(&["-c", "try ([range(0;200000)] + 1) catch ."], "null") + .await + .unwrap(); + assert!( + out.len() < 1024, + "math error on huge operand leaked {} bytes", + out.len() + ); + assert!(out.contains("..."), "huge operand preview should truncate"); + let recovered = run_jq_with_args(&["-c", "1 + 2"], "null").await.unwrap(); + assert_eq!(recovered, "3\n"); +} + #[tokio::test] async fn division_by_zero_is_a_runtime_error() { let result = run_jq_result_with_args(&["-c", "1 / .x"], "{\"x\": 2} {\"x\": 0} {\"x\": 4}") diff --git a/knowledge/runtimes/jaq-json-vendor.md b/knowledge/runtimes/jaq-json-vendor.md index 3ddb89d3d..b07fb56b3 100644 --- a/knowledge/runtimes/jaq-json-vendor.md +++ b/knowledge/runtimes/jaq-json-vendor.md @@ -116,7 +116,7 @@ same rewrite applies to every release: timeout (`jq: execution timed out`). - Output conversion is capped by `max_stdout_bytes`, so a value built from shared parts (`reduce range(40) as $i (1; [., .])`) cannot expand when - rendered. Runtime error messages render at most 64 KiB of a value. Short error previews (`jqmsg::dump_trunc`, object-key and cannot-index messages) render through a capped sink, so even the intermediate preview never materializes a huge value's full dump. + rendered. Runtime error messages render at most 64 KiB of a value. Short error previews (`jqmsg::dump_trunc`, object-key and cannot-index messages) render through a capped sink that aborts the traversal once the budget is spent, so even the intermediate preview never materializes a huge value's full dump (and never walks the rest of it just to discard the output). Builds with `panic = "abort"` cannot unwind. There the fallible checks still apply, over-limit infallible constructions are cut short with the meter From cd84d5deed90917a3376d8bc2b67f8513aca3b6d Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 02:03:10 -0500 Subject: [PATCH 2/2] fix(jq): exact-filter bounded regression, same-session recovery, preview oracle Address review on the error-preview budget fix: - cli_oneshot bounded-child regression runs the exact finding filter (reduce range(0;40) shared tree) with deadline poll + kill; asserts raw exit 5, sub-KB stderr, truncation marker; a regression fails by timeout/kill without OOMing the harness. - True same-session recovery: one shell hits a truncated huge-operand error, then the same session continues; fresh-context helpers relabeled honestly. - dump_trunc oracle contract documented (verbatim vs 11-byte prefix + unconditional ... marker, UTF-8 boundary rule). --- crates/bashkit-cli/tests/cli_oneshot.rs | 71 +++++++++++++++++++ .../bashkit/src/builtins/jq/jaq_json/jqmsg.rs | 20 +++++- crates/bashkit/src/builtins/jq/tests.rs | 4 +- 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/crates/bashkit-cli/tests/cli_oneshot.rs b/crates/bashkit-cli/tests/cli_oneshot.rs index 1353193ac..4912be14a 100644 --- a/crates/bashkit-cli/tests/cli_oneshot.rs +++ b/crates/bashkit-cli/tests/cli_oneshot.rs @@ -1283,6 +1283,77 @@ fn jq_non_tail_recursion_is_bounded_on_a_two_mib_stack() { assert!(stderr(&out).contains("recursion limit (64) exceeded")); } +#[test] +fn jq_math_error_on_shared_tree_stays_bounded() { + // Exact filter from the security finding: `reduce` over `range(0; 40)` + // builds a shared structure whose naive error dump would expand ~2^40 + // elements. The preview budget must bound both allocation and traversal. + // The child is wall-bounded (deadline poll + kill) and output is + // collected only after exit, so a regression fails here by timeout/kill + // without ever OOMing this harness. Raw jq exits 5 on runtime errors; + // the harness itself exits 0. + let mut child = cli() + .args([ + "--no-stdin", + "--timeout", + "10", + "-c", + "jq -n 'reduce range(0; 40) as $i (1; [., .]) | . + 1'", + ]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn bashkit"); + let deadline = Instant::now() + Duration::from_secs(30); + while child.try_wait().expect("poll bashkit").is_none() && Instant::now() < deadline { + thread::sleep(Duration::from_millis(20)); + } + if child.try_wait().expect("final poll bashkit").is_none() { + child.kill().expect("kill hung bashkit"); + let _ = child.wait(); + panic!("jq shared-tree math error hung (error preview escaped its budget)"); + } + let out = child.wait_with_output().expect("collect bashkit output"); + assert_eq!(code(&out), 5, "stderr: {}", stderr(&out)); + assert!(stdout(&out).is_empty(), "stdout: {}", stdout(&out)); + let err = stderr(&out); + assert!( + err.len() < 1024, + "math error on shared tree leaked {} bytes", + err.len() + ); + assert!(err.contains("cannot be added"), "stderr: {err}"); + assert!( + err.contains("..."), + "shared-tree preview should truncate: {err}" + ); +} + +#[test] +fn shell_recovers_after_huge_operand_math_error() { + // True same-session recovery: one shell hits a truncated huge-operand + // math error, then the SAME session runs on normally. + let out = run(&[ + "-c", + "jq -n 'try ([range(0;200000)] + 1) catch .'; echo RECOVERED", + ]); + assert_eq!(code(&out), 0, "stderr: {}", stderr(&out)); + let body = stdout(&out); + assert!( + body.len() < 1024, + "in-session math error leaked {} bytes", + body.len() + ); + assert!( + body.contains("..."), + "huge operand preview should truncate: {body}" + ); + assert!( + body.ends_with("RECOVERED\n"), + "session did not continue: {body}" + ); +} + // --------------------------------------------------------------------------- // Streaming // --------------------------------------------------------------------------- diff --git a/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs b/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs index 6a0995dd1..d5a2cf1bb 100644 --- a/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs +++ b/crates/bashkit/src/builtins/jq/jaq_json/jqmsg.rs @@ -52,6 +52,21 @@ impl std::fmt::Write for Capped { Ok(()) } } +/// Preview oracle for math/index error messages. +/// +/// The value renders through [`Capped`] (15-byte budget). Exact contract: +/// - rendering fits without tripping the budget -> returned verbatim +/// (e.g. `"ok"` stays `"ok"`, a 14-byte rendering is untouched); +/// - budget spent -> keep at most the first 11 bytes floored to a UTF-8 +/// boundary and always append `...` (e.g. a 15-byte rendering becomes +/// its 11-byte prefix plus `...`; a multibyte char straddling the cut +/// yields a boundary-floored prefix plus `...`, never a split char and +/// never a silent drop); +/// - the sink reports `fmt::Error` once the budget is spent, so the +/// `?`-laden display machinery aborts the traversal instead of walking +/// the rest of a hostile value just to discard it. +/// Small-value output is byte-identical to plain truncation; only the CPU +/// cost and the truncation marker on short aborted buffers differ. fn dump_trunc(v: &Val) -> String { use std::fmt::Write as _; // One byte past the 14-byte cut: enough to tell "fits" from "truncate", @@ -187,7 +202,10 @@ mod tests { } #[test] - fn preview_recovers_after_truncation() { + fn preview_stateless_across_calls() { + // `dump_trunc` keeps no global state: a truncated huge preview must + // not affect later previews (fresh-invocation recovery; same-shell + // recovery is covered by `shell_recovers_after_huge_operand_math_error`). let _ = math(huge_array_key(), ops::Math::Add, huge_array_key(), false).to_string(); assert_eq!(dump_trunc(&Val::utf8_str("ok")), "\"ok\""); } diff --git a/crates/bashkit/src/builtins/jq/tests.rs b/crates/bashkit/src/builtins/jq/tests.rs index d40b35305..1330f93f6 100644 --- a/crates/bashkit/src/builtins/jq/tests.rs +++ b/crates/bashkit/src/builtins/jq/tests.rs @@ -2043,7 +2043,7 @@ async fn math_errors_use_jq_wording() { } #[tokio::test] -async fn math_error_on_huge_operands_stays_bounded_and_session_recovers() { +async fn math_error_on_huge_operands_stays_bounded() { let out = run_jq_with_args(&["-c", "try ([range(0;200000)] + 1) catch ."], "null") .await .unwrap(); @@ -2053,6 +2053,8 @@ async fn math_error_on_huge_operands_stays_bounded_and_session_recovers() { out.len() ); assert!(out.contains("..."), "huge operand preview should truncate"); + // Fresh-context follow-up (each helper builds a new shell); true + // same-session recovery is covered by `shell_recovers_after_huge_operand_math_error`. let recovered = run_jq_with_args(&["-c", "1 + 2"], "null").await.unwrap(); assert_eq!(recovered, "3\n"); }