From 3077e33f964c60a1f1856926de15ea5dba6c434e Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Sun, 11 Oct 2026 22:52:29 -0500 Subject: [PATCH 01/16] fix(interpreter): charge input budget for numbered input redirects Each N $f`, `> out-*`, /// `> a-{1,2}`), or one with a quoted expansion. Plain words skip the /// field pass. +/// Numbered-fd table with replace semantics (input-redirect budget fix): re-redirecting +/// an fd drops the superseded buffer instead of accumulating clones. +fn replace_own_fd(own_fds: &mut Vec<(i32, crate::StreamData)>, fd: i32, data: crate::StreamData) { + if let Some(slot) = own_fds.iter_mut().find(|(existing, _)| *existing == fd) { + *slot = (fd, data); + } else { + own_fds.push((fd, data)); + } +} + fn redirect_target_may_split(redirect: &Redirect) -> bool { if !matches!( redirect.kind, @@ -229,6 +239,21 @@ impl Interpreter { self.diag(format!("{path}: {reason}\n")) } + /// Charging helper for numbered input redirects (input-redirect budget fix). Every + /// retained buffer counts against the aggregate input budget and holds + /// a live-bytes lease while the redirect list is processed. Charges run + /// before the clone/allocation they cover; re-redirecting the same fd + /// replaces the superseded entry instead of piling clones. + fn charge_input_retention( + &self, + leases: &mut Vec, + len: usize, + ) -> crate::error::Result<()> { + self.execution_budget.consume_input(len)?; + leases.push(self.execution_budget.lease_bytes(len)?); + Ok(()) + } + /// Process input redirections (< file, <<< string). `demand` is how /// much the command reads, for stdin taken from a coproc's pipe. pub(super) async fn process_input_redirections( @@ -241,6 +266,9 @@ impl Interpreter { let mut stdin = existing_stdin; // Readable fds this command's own redirects open (`4< = Vec::new(); + // Live-bytes leases for buffers retained by numbered + // input redirects, held until the redirect list is fully processed. + let mut input_leases: Vec = Vec::new(); for redirect in redirects { // Fd N >= 1 for an input-side redirect (`3f`). @@ -271,7 +299,10 @@ impl Interpreter { } }; match high_fd { - Some(fd) => own_fds.push((fd, content)), + Some(fd) => { + self.charge_input_retention(&mut input_leases, content.len())?; + replace_own_fd(&mut own_fds, fd, content); + } None => stdin = Some(content), } } @@ -300,7 +331,8 @@ impl Interpreter { Ok(content) => { let content: crate::StreamData = content.into(); if let Some(fd) = high_fd { - own_fds.push((fd, content.clone())); + self.charge_input_retention(&mut input_leases, content.len())?; + replace_own_fd(&mut own_fds, fd, content.clone()); } // WTF: on a compound `3 own_fds.push((fd, content)), + Some(fd) => { + self.charge_input_retention(&mut input_leases, content.len())?; + replace_own_fd(&mut own_fds, fd, content); + } None => stdin = Some(content), } } @@ -340,7 +375,10 @@ impl Interpreter { let content: crate::StreamData = self.expand_word(&redirect.target).await?.into(); match high_fd { - Some(fd) => own_fds.push((fd, content)), + Some(fd) => { + self.charge_input_retention(&mut input_leases, content.len())?; + replace_own_fd(&mut own_fds, fd, content); + } None => stdin = Some(content), } } @@ -360,11 +398,13 @@ impl Interpreter { let Ok(src) = target.parse::() else { continue; }; - // `<&4` after this command's own `4< own_fds.push((fd, content)), + Some(fd) => replace_own_fd(&mut own_fds, fd, content), None => stdin = Some(content), } continue; diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs new file mode 100644 index 000000000..c082f1319 --- /dev/null +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -0,0 +1,75 @@ +//! Repeated numbered input redirects must charge the input budget: +//! +//! Each `N Date: Sun, 11 Oct 2026 23:44:46 -0500 Subject: [PATCH 02/16] fix(interpreter): bind input-retention leases to fd entries, count both buffers StreamData retains bytes plus decoded text; charge retained_len so the live/aggregate admission covers the full backing store. Store each live-bytes lease with its fd entry so replacing a superseded descriptor releases the old lease while aggregate consumption stays cumulative. Tests: exact 63/64 and 71/72 boundaries, live release on replace (2x transient), cumulative-across-replace, all reachable retaining branches, 1024x /dev/zero shape, session recovery, dup-chain parity, 1MiB heredoc bound. --- crates/bashkit/src/interpreter/redirection.rs | 64 ++++--- crates/bashkit/src/stream.rs | 6 + .../input_redirect_budget_tests.rs | 173 +++++++++++++++++- 3 files changed, 212 insertions(+), 31 deletions(-) diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 42aa6a6b9..30dbb4bbe 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -25,16 +25,6 @@ use crate::error::io_error_reason; /// expansion could turn into other than one word (`> $f`, `> out-*`, /// `> a-{1,2}`), or one with a quoted expansion. Plain words skip the /// field pass. -/// Numbered-fd table with replace semantics (input-redirect budget fix): re-redirecting -/// an fd drops the superseded buffer instead of accumulating clones. -fn replace_own_fd(own_fds: &mut Vec<(i32, crate::StreamData)>, fd: i32, data: crate::StreamData) { - if let Some(slot) = own_fds.iter_mut().find(|(existing, _)| *existing == fd) { - *slot = (fd, data); - } else { - own_fds.push((fd, data)); - } -} - fn redirect_target_may_split(redirect: &Redirect) -> bool { if !matches!( redirect.kind, @@ -66,6 +56,24 @@ fn redirect_target_may_split(redirect: &Redirect) -> bool { }) } +/// Numbered-fd table with replace semantics: re-redirecting an fd drops the +/// superseded buffer — and its live-bytes lease — instead of piling clones. +/// Aggregate input consumption stays cumulative. +type OwnFdEntry = (i32, crate::StreamData, crate::limits::ExecutionBudgetLease); + +fn replace_own_fd( + own_fds: &mut Vec, + fd: i32, + data: crate::StreamData, + lease: crate::limits::ExecutionBudgetLease, +) { + if let Some(slot) = own_fds.iter_mut().find(|(existing, _, _)| *existing == fd) { + *slot = (fd, data, lease); + } else { + own_fds.push((fd, data, lease)); + } +} + /// `N<= 3: a readable fd for the command's body. fn high_fd_here(redirect: &Redirect) -> bool { matches!( @@ -246,12 +254,10 @@ impl Interpreter { /// replaces the superseded entry instead of piling clones. fn charge_input_retention( &self, - leases: &mut Vec, len: usize, - ) -> crate::error::Result<()> { + ) -> crate::error::Result { self.execution_budget.consume_input(len)?; - leases.push(self.execution_budget.lease_bytes(len)?); - Ok(()) + Ok(self.execution_budget.lease_bytes(len)?) } /// Process input redirections (< file, <<< string). `demand` is how @@ -265,10 +271,7 @@ impl Interpreter { ) -> Result> { let mut stdin = existing_stdin; // Readable fds this command's own redirects open (`4< = Vec::new(); - // Live-bytes leases for buffers retained by numbered - // input redirects, held until the redirect list is fully processed. - let mut input_leases: Vec = Vec::new(); + let mut own_fds: Vec = Vec::new(); for redirect in redirects { // Fd N >= 1 for an input-side redirect (`3f`). @@ -300,8 +303,8 @@ impl Interpreter { }; match high_fd { Some(fd) => { - self.charge_input_retention(&mut input_leases, content.len())?; - replace_own_fd(&mut own_fds, fd, content); + let lease = self.charge_input_retention(content.retained_len())?; + replace_own_fd(&mut own_fds, fd, content, lease); } None => stdin = Some(content), } @@ -331,8 +334,9 @@ impl Interpreter { Ok(content) => { let content: crate::StreamData = content.into(); if let Some(fd) = high_fd { - self.charge_input_retention(&mut input_leases, content.len())?; - replace_own_fd(&mut own_fds, fd, content.clone()); + let lease = + self.charge_input_retention(content.retained_len())?; + replace_own_fd(&mut own_fds, fd, content.clone(), lease); } // WTF: on a compound `3 { - self.charge_input_retention(&mut input_leases, content.len())?; - replace_own_fd(&mut own_fds, fd, content); + let lease = self.charge_input_retention(content.retained_len())?; + replace_own_fd(&mut own_fds, fd, content, lease); } None => stdin = Some(content), } @@ -376,8 +380,8 @@ impl Interpreter { self.expand_word(&redirect.target).await?.into(); match high_fd { Some(fd) => { - self.charge_input_retention(&mut input_leases, content.len())?; - replace_own_fd(&mut own_fds, fd, content); + let lease = self.charge_input_retention(content.retained_len())?; + replace_own_fd(&mut own_fds, fd, content, lease); } None => stdin = Some(content), } @@ -400,11 +404,13 @@ impl Interpreter { }; // `<&4` after this command's own `4< replace_own_fd(&mut own_fds, fd, content), + Some(fd) => replace_own_fd(&mut own_fds, fd, content, lease), None => stdin = Some(content), } continue; diff --git a/crates/bashkit/src/stream.rs b/crates/bashkit/src/stream.rs index e22fb3799..c33a92fb3 100644 --- a/crates/bashkit/src/stream.rs +++ b/crates/bashkit/src/stream.rs @@ -58,6 +58,12 @@ impl StreamData { pub fn len(&self) -> usize { self.bytes.len() } + + /// Live bytes retained by this buffer: the raw bytes plus the decoded + /// text kept alongside them. + pub(crate) fn retained_len(&self) -> usize { + self.bytes.len().saturating_add(self.text.len()) + } pub fn is_empty(&self) -> bool { self.bytes.is_empty() } diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index c082f1319..2391e7842 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -12,7 +12,7 @@ use std::path::Path; #[tokio::test] /// Repeated numbered input redirects exceed the aggregate input budget. async fn numbered_input_redirects_charge_aggregate_input_budget() { - let payload = vec![b'x'; 64]; + let payload = vec![b'x'; 32]; let mut bash = Bash::builder() .limits(ExecutionLimits::new().max_aggregate_input_bytes(100)) .build(); @@ -53,7 +53,7 @@ async fn superseded_numbered_input_fd_is_replaced() { #[tokio::test] /// Retained numbered-fd buffers hold live-bytes leases while processed. async fn numbered_input_redirects_hold_live_bytes_budget() { - let payload = vec![b'x'; 64]; + let payload = vec![b'x'; 32]; let mut bash = Bash::builder() .limits(ExecutionLimits::new().max_live_intermediate_bytes(100)) .build(); @@ -73,3 +73,172 @@ async fn numbered_input_redirects_hold_live_bytes_budget() { "expected live-bytes budget exhaustion, got {result:?}" ); } + +#[tokio::test] +/// Original attack shape: ':' with 1024 same-fd /dev/zero redirects trips +/// the aggregate budget on the first retained buffer instead of piling MiBs. +async fn original_shape_1024_dev_zero_trips_fast() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_aggregate_input_bytes(65_536)) + .build(); + let script = format!(":{}", " 3/big"]; + for script in cases { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_aggregate_input_bytes(31)) + .build(); + bash.fs() + .write_file(Path::new("/big"), b"sixteen-bytesxx") + .await + .unwrap(); + let result = bash.exec(script).await; + assert!( + matches!( + result, + Err(Error::ResourceLimit(LimitExceeded::ExecutionBudget( + ExecutionBudgetExceeded::InputBytes { .. } + ))) + ), + "script {script:?} should trip the input budget, got {result:?}" + ); + } +} + +#[tokio::test] +/// The session stays usable after a budget error: unrelated commands run. +async fn session_recovers_after_budget_error() { + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_aggregate_input_bytes(100)) + .build(); + bash.fs() + .write_file(Path::new("/big"), &[b'x'; 32]) + .await + .unwrap(); + let tripped = bash.exec(": 3 Date: Mon, 12 Oct 2026 00:32:50 -0500 Subject: [PATCH 03/16] fix(interpreter): hand stdin-clone leases to caller, prove dup/overlap/child bounds The DupInput-into-stdin clone dropped its live lease while retained. The stdin slot now carries the clone lease and the setup struct hands it to callers, who hold it for stdin's lifetime; replacing stdin drops both. Tests: two-phase DupInput proof (source fits, target trips InputBytes), ReadWrite trip, stdin-clone 3x64B live overlap (192 ok / 191 LiveBytes) with recovery, bounded-child original 1024x/dev/zero denial naming aggregate input bytes with clean exit 1. --- crates/bashkit/src/interpreter/mod.rs | 19 +- crates/bashkit/src/interpreter/redirection.rs | 61 ++++- .../input_redirect_budget_tests.rs | 244 ++++++++++++++++-- 3 files changed, 282 insertions(+), 42 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 10fce58d5..d63d2f37c 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -9379,7 +9379,7 @@ impl Interpreter { } // Handle input redirections first - let stdin = match self + let stdin_setup = match self .process_input_redirections( stdin, &command.redirects, @@ -9394,6 +9394,10 @@ impl Interpreter { } Err(e) => return Err(e), }; + // Hold the stdin clone's live lease (if stdin is a budgeted + // clone minted above) for as long as `stdin` is retained below. + let _stdin_storage = stdin_setup.stdin_lease; + let stdin = stdin_setup.stdin; // `read -u FD` on a readable fd (`exec 3 return Err(e), }; + // Hold the stdin clone's live lease (if stdin is a budgeted + // clone minted above) for as long as `stdin` is retained below. + let _stdin_storage = stdin_setup.stdin_lease; + let stdin = stdin_setup.stdin; let prev_pipeline_stdin = if stdin.is_some() { let prev = self.pipeline_stdin.take(); self.pipeline_stdin = stdin; @@ -12974,8 +12982,11 @@ impl Interpreter { .await }; match read { - Ok(content) => { - if let Some(content) = content { + Ok(setup) => { + // Hold the stdin clone's live lease while the + // substitution output derived from it is live. + let _stdin_storage = setup.stdin_lease; + if let Some(content) = setup.stdin { // Account for the VFS-owned input while it and the // decoded substitution text are simultaneously live. let _content_lease = diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 30dbb4bbe..46b4ca424 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -56,6 +56,33 @@ fn redirect_target_may_split(redirect: &Redirect) -> bool { }) } +/// Stdin slot: the buffer plus the live-bytes lease when the buffer is a +/// budgeted clone minted here. Assigning the slot drops any previous +/// buffer and its clone lease together. +type StdinSlot = Option<( + crate::StreamData, + Option, +)>; + +/// Assign the stdin slot, dropping any previous buffer and its clone lease +/// together. Fresh reads pass `lease: None`; budgeted clones pass the lease +/// from `charge_input_retention` so it lives as long as stdin does. +fn set_stdin_slot( + slot: &mut StdinSlot, + value: Option, + lease: Option, +) { + *slot = value.map(|data| (data, lease)); +} + +/// Result of input-redirection processing: the stdin buffer plus the +/// live-bytes lease for it when stdin is a budgeted clone minted here. +/// Callers hold `stdin_lease` for as long as they retain `stdin`. +pub(crate) struct StdinSetup { + pub stdin: Option, + pub stdin_lease: Option, +} + /// Numbered-fd table with replace semantics: re-redirecting an fd drops the /// superseded buffer — and its live-bytes lease — instead of piling clones. /// Aggregate input consumption stays cumulative. @@ -268,8 +295,11 @@ impl Interpreter { redirects: &[Redirect], demand: StdinDemand, high_fd_stdin: HighFdStdin, - ) -> Result> { - let mut stdin = existing_stdin; + ) -> crate::error::Result { + // Stdin slot: a budgeted clone's live lease travels with the buffer + // so replacing stdin drops both together; the surviving lease is + // handed to the caller with stdin. + let mut stdin: StdinSlot = existing_stdin.map(|data| (data, None)); // Readable fds this command's own redirects open (`4< = Vec::new(); @@ -306,7 +336,7 @@ impl Interpreter { let lease = self.charge_input_retention(content.retained_len())?; replace_own_fd(&mut own_fds, fd, content, lease); } - None => stdin = Some(content), + None => set_stdin_slot(&mut stdin, Some(content), None), } } RedirectKind::Input => { @@ -324,7 +354,7 @@ impl Interpreter { if self.fd_alias(&path) == Some(0) { // `< /dev/stdin` re-reads the current stdin: no-op. } else if is_dev_null(&path) { - stdin = Some(crate::StreamData::new()); // EOF + set_stdin_slot(&mut stdin, Some(crate::StreamData::new()), None); // EOF } else if !self.shell_features.has_file_redirects() { return Err(crate::error::Error::Execution( self.diag(format!("{}: filesystem redirection disabled", target_path)), @@ -350,7 +380,7 @@ impl Interpreter { (Some(_), HighFdStdin::Never) => false, }; if feeds { - stdin = Some(content); + set_stdin_slot(&mut stdin, Some(content), None); } } Err(e) => { @@ -370,7 +400,7 @@ impl Interpreter { let lease = self.charge_input_retention(content.retained_len())?; replace_own_fd(&mut own_fds, fd, content, lease); } - None => stdin = Some(content), + None => set_stdin_slot(&mut stdin, Some(content), None), } } RedirectKind::HereDoc | RedirectKind::HereDocStrip => { @@ -383,7 +413,7 @@ impl Interpreter { let lease = self.charge_input_retention(content.retained_len())?; replace_own_fd(&mut own_fds, fd, content, lease); } - None => stdin = Some(content), + None => set_stdin_slot(&mut stdin, Some(content), None), } } RedirectKind::DupInput => { @@ -411,7 +441,7 @@ impl Interpreter { let content = content.clone(); match high_fd { Some(fd) => replace_own_fd(&mut own_fds, fd, content, lease), - None => stdin = Some(content), + None => set_stdin_slot(&mut stdin, Some(content), Some(lease)), } continue; } @@ -437,11 +467,15 @@ impl Interpreter { text.push('\n'); } } - stdin = Some(text.into()); + set_stdin_slot(&mut stdin, Some(text.into()), None); } Some(coproc::InputFd::Pipe(end)) => { let pipe = Arc::clone(&end.0); - stdin = Some(Box::pin(coproc::read_coproc_input(pipe, demand)).await); + set_stdin_slot( + &mut stdin, + Some(Box::pin(coproc::read_coproc_input(pipe, demand)).await), + None, + ); } // `<&5` with fd 5 never opened (or closed): bash // reports it and does not run the command. @@ -462,7 +496,12 @@ impl Interpreter { } } - Ok(stdin) + let mut stdin_lease = None; + let stdin = stdin.map(|(data, lease)| { + stdin_lease = lease; + data + }); + Ok(StdinSetup { stdin, stdin_lease }) } /// Apply output redirections to command output diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 2391e7842..1c1abd555 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -124,7 +124,9 @@ async fn both_backing_buffers_admitted() { #[tokio::test] /// Replacing a superseded same-fd entry releases its live lease: eight -/// same-fd repeats hold only one 64B lease under a 64B live cap. +/// same-fd repeats never hold more than two 64B buffers transiently (one +/// retained plus one incoming during replacement), so a 128B live cap +/// suffices where append-only leases would grow without bound. async fn superseded_entry_releases_live_lease() { let mut bash = Bash::builder() .limits(ExecutionLimits::new().max_live_intermediate_bytes(128_u64)) @@ -165,32 +167,71 @@ async fn cumulative_aggregate_survives_replace() { } #[tokio::test] -/// Every reachable cloning/retaining numbered-input branch charges: DupInput -/// clone and ReadWrite each trip a 31B aggregate cap (16B payload retains -/// 32B). (`3<<<` drops its fd in the parser and always feeds stdin, and -/// numbered heredoc bodies are parse-metered with no per-exec retention, so -/// those arms keep their charges as defense-in-depth.) -async fn all_numbered_input_branches_charge() { - let cases = [": 3/big"]; - for script in cases { - let mut bash = Bash::builder() - .limits(ExecutionLimits::new().max_aggregate_input_bytes(31)) - .build(); - bash.fs() - .write_file(Path::new("/big"), b"sixteen-bytesxx") - .await - .unwrap(); - let result = bash.exec(script).await; - assert!( - matches!( - result, - Err(Error::ResourceLimit(LimitExceeded::ExecutionBudget( - ExecutionBudgetExceeded::InputBytes { .. } - ))) - ), - "script {script:?} should trip the input budget, got {result:?}" - ); - } +/// The DupInput target duplication itself is charged: with a 16B payload +/// (32B retained), a 47B aggregate cap admits the 15B script plus the source +/// fd (47B total) but the `4<&3` clone (79B) trips `InputBytes`. Controls +/// prove the script text and source fd fit on their own. +async fn dup_target_duplication_charges() { + // Control 1: source fd alone fits (7B script + 32B retained = 39B). + let mut bash = Bash::builder() + .limits(ExecutionLimits::new().max_aggregate_input_bytes(39)) + .build(); + bash.fs() + .write_file(Path::new("/big"), b"sixteen-bytesxx") + .await + .unwrap(); + assert!(bash.exec(": 3/big").await; + assert!( + matches!( + result, + Err(Error::ResourceLimit(LimitExceeded::ExecutionBudget( + ExecutionBudgetExceeded::InputBytes { .. } + ))) + ), + "ReadWrite retention should trip InputBytes, got {result:?}" + ); } #[tokio::test] @@ -242,3 +283,152 @@ async fn large_numbered_heredoc_stays_bounded() { let result = bash.exec(&script).await.unwrap(); assert_eq!(result.exit_code, 0); } + +#[tokio::test] +/// Stdin-clone live overlap: `cat 3 deadline { + child.kill().expect("kill overrun child"); + let _ = child.wait(); + use std::os::unix::process::ExitStatusExt; + return ( + std::process::ExitStatus::from_raw(9), + std::process::Output { + status: std::process::ExitStatus::from_raw(9), + stdout: Vec::new(), + stderr: b"wall-clock overrun".to_vec(), + }, + start.elapsed(), + true, + ); + } + std::thread::sleep(std::time::Duration::from_millis(50)); + } + }) + .await + .expect("child harness join"); + let (status, output, elapsed, overran) = outcome; + assert!( + !overran, + "bounded child overran the wall clock; killed and reaped" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert_eq!( + status.code(), + Some(1), + "producer must fail cleanly (signal death = OOM, 0 = unbounded success): {status:?}" + ); + assert!( + stderr.contains("aggregate input bytes"), + "denial must name the aggregate input budget phase, got: {stderr:?}" + ); + assert!(output.stdout.len() < 4096, "producer output capped"); + assert!( + elapsed < std::time::Duration::from_secs(60), + "producer took {elapsed:?}" + ); +} From 6f46a4a4b3fc4482e0272d1a12231d6db7e8dd82 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 01:05:59 -0500 Subject: [PATCH 04/16] fix(interpreter): isolate coverage artifacts from bounded-child file cap Under cargo-llvm-cov the spawned CLI inherits LLVM_PROFILE_FILE and flushes a multi-MiB profraw at exit, tripping the child's 1MiB -f cap (SIGXFSZ, wait status 153). Point the child's profile at a controlled temp sink (coverage stays enabled), raise -f to an explicit 128MiB artifact allowance with a size receipt plus cleanup, and stop equating signal death with OOM in assertions. --- .../input_redirect_budget_tests.rs | 54 ++++++++++++++++--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 1c1abd555..15f7a3182 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -360,6 +360,15 @@ async fn stdin_clone_trip_recovers() { #[tokio::test] async fn original_shape_bounded_child_default_limits() { let script = format!(":{}", " 3 deadline { child.kill().expect("kill overrun child"); let _ = child.wait(); - use std::os::unix::process::ExitStatusExt; - return ( - std::process::ExitStatus::from_raw(9), + break ( + { + use std::os::unix::process::ExitStatusExt; + std::process::ExitStatus::from_raw(9) + }, std::process::Output { - status: std::process::ExitStatus::from_raw(9), + status: { + use std::os::unix::process::ExitStatusExt; + std::process::ExitStatus::from_raw(9) + }, stdout: Vec::new(), stderr: b"wall-clock overrun".to_vec(), }, @@ -407,7 +431,19 @@ async fn original_shape_bounded_child_default_limits() { ); } std::thread::sleep(std::time::Duration::from_millis(50)); + }; + // Artifact receipt (when coverage writes one) plus finite cleanup. + const ARTIFACT_BOUND: u64 = 128 * 1024 * 1024; + if let Ok(meta) = std::fs::metadata(&profile) { + assert!( + meta.len() < ARTIFACT_BOUND, + "coverage artifact receipt: {} bytes exceeds {}", + meta.len(), + ARTIFACT_BOUND + ); } + let _ = std::fs::remove_dir_all(sink_dir); + (status, output, elapsed, overran) }) .await .expect("child harness join"); @@ -420,7 +456,9 @@ async fn original_shape_bounded_child_default_limits() { assert_eq!( status.code(), Some(1), - "producer must fail cleanly (signal death = OOM, 0 = unbounded success): {status:?}" + // Signal death here means OOM kill, an artifact-cap trip (SIGXFSZ), + // or a crash, never success; producer diagnostics included. + "producer must fail cleanly, got {status:?} with stderr {stderr:?}", ); assert!( stderr.contains("aggregate input bytes"), From 35d690e48583d423ed3c31a45f0bba22f5a5a6ce Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 06:06:24 -0500 Subject: [PATCH 05/16] test(interpreter): opt-in bounded runner extension for input-redirect job Reuse reviewed run_command_bounded_with lifecycle with a job-specific launcher (shared CPU/stack caps plus Linux-gated address-space cap and file-size backstop, all fail-closed, extended byte-unit readback), an owned LLVM artifact sink with explicit bound plus receipt, work-dir inventory, and TempWorkdir ownership with asserted cleanup. Tiny synthetic fixtures cover launcher text, receipt parsing, artifact classes, guard lifecycle, and normal child completion. --- .../input_redirect_budget_tests.rs | 507 ++++++++++++++---- .../tests/integration/threat_model_tests.rs | 28 +- 2 files changed, 408 insertions(+), 127 deletions(-) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 15f7a3182..d6f8aef19 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -345,128 +345,409 @@ async fn stdin_clone_trip_recovers() { assert!(result.stdout.text().unwrap().contains("recovered")); } -/// Original shape under default limits in an owned bounded child: `:` plus -/// 1024 same-fd `/dev/zero` redirects is denied quickly by the aggregate -/// input budget (each 1MiB read retains ~2MiB against the 100MB default cap, -/// tripping after ~50 redirects) instead of piling ~2GiB of retained clones. -/// The harness asserts the exact denial: clean exit code 1 naming `aggregate -/// input bytes` (production retention admission, far above the ~7KiB script -/// text), which distinguishes budget denial from OOM/signal death (no exit -/// code) and from pre-fix success (exit 0). The 1536MiB RLIMIT_AS -/// additionally caps any regression to append-only retention. The child is -/// always reaped, killed first on wall-clock expiry. Unix-only: needs bash + -/// ulimit for enforcement. -#[cfg(unix)] -#[tokio::test] -async fn original_shape_bounded_child_default_limits() { - let script = format!(":{}", " 3 deadline { - child.kill().expect("kill overrun child"); - let _ = child.wait(); - break ( - { - use std::os::unix::process::ExitStatusExt; - std::process::ExitStatus::from_raw(9) - }, - std::process::Output { - status: { - use std::os::unix::process::ExitStatusExt; - std::process::ExitStatus::from_raw(9) - }, - stdout: Vec::new(), - stderr: b"wall-clock overrun".to_vec(), - }, - start.elapsed(), - true, - ); - } - std::thread::sleep(std::time::Duration::from_millis(50)); - }; - // Artifact receipt (when coverage writes one) plus finite cleanup. - const ARTIFACT_BOUND: u64 = 128 * 1024 * 1024; - if let Ok(meta) = std::fs::metadata(&profile) { - assert!( - meta.len() < ARTIFACT_BOUND, - "coverage artifact receipt: {} bytes exceeds {}", - meta.len(), - ARTIFACT_BOUND - ); +// ---------- opt-in bounded runner extension for the input-redirect job ---------- +// Reuses the reviewed lifecycle (`run_command_bounded_with`) with a +// job-specific launcher. Shared defaults for other products are untouched. + +use crate::threat_model_tests::resource_exhaustion::{ + BoundedRun, LifecycleFaults, bashkit_cli_path, run_command_bounded_with, +}; +use std::process::{Command, Stdio}; +use std::time::Duration; + +/// Address-space cap in KiB for the bash `ulimit -v` knob (Linux only). +const IR_AS_KIB: u64 = 1_572_864; // 1.5GiB +/// File-size backstop in POSIX 512B blocks for the bash `ulimit -f` knob. +const IR_FILE_BLOCKS: u64 = 131_072; // 131072 * 512B = 64MiB nominal +/// Explicit allowance for one short instrumented run's coverage artifact. +const IR_ARTIFACT_BOUND: u64 = 64 * 1024 * 1024; // 64MiB + +/// Opt-in launcher for the input-redirect job: shared CPU/stack caps plus an +/// address-space cap and a file-size backstop, every `ulimit` fail-closed +/// (`|| exit 99`), with an extended readback receipt on stderr. +/// +/// The AS cap is Linux-only (`ulimit -v` is refused elsewhere); other +/// platforms record `mem=unsupported` truthfully instead of pretending. +/// Raw values use the shell's own units (POSIX 512B blocks for `-f`; bash +/// KiB for `-s`/`-v`; seconds for `-t`); `mem_bytes`/`file_bytes` come from +/// /proc byte-exact on Linux and are `unsupported` elsewhere, so unit +/// surprises fail closed at parse/assert time instead of converting +/// silently. +fn input_shape_launcher() -> String { + format!( + "ulimit -s 4096 || exit 99; \ + ulimit -t 10 || exit 99; \ + ulimit -f {fileblk} || exit 99; \ + mem_kb=unsupported; mem_bytes=unsupported; file_bytes=unsupported; \ + if [ \"$(uname)\" = Linux ] && [ -r /proc/self/limits ]; then \ + ulimit -v {askib} || exit 99; \ + mem_kb=$(ulimit -Sv); \ + mem_bytes=$(awk '/Max address space/ {{print $4}}' /proc/self/limits); \ + file_bytes=$(awk '/Max file size/ {{print $4}}' /proc/self/limits); \ + fi; \ + echo \"ir-limits: stack=$(ulimit -s) cpu=$(ulimit -t) \ + mem_kb=$mem_kb mem_bytes=$mem_bytes \ + fileblk=$(ulimit -Sf) file_bytes=$file_bytes\" >&2; \ + exec \"$@\"", + fileblk = IR_FILE_BLOCKS, + askib = IR_AS_KIB, + ) +} + +/// Parsed `ir-limits:` receipt. Raw fields are the shell's own units; +/// `*_bytes` are kernel byte counts (Linux) or `None` (explicitly +/// unsupported elsewhere). +#[derive(Debug)] +struct IrLimitsNote { + stack_raw: String, + cpu_raw: String, + mem_kb_raw: String, + mem_bytes: Option, + fileblk_raw: String, + file_bytes: Option, +} + +fn parse_ir_limits_note(line: &str) -> Result { + let body = line + .strip_prefix("ir-limits:") + .ok_or_else(|| format!("missing ir-limits prefix: {line:?}"))?; + let get = |key: &str| -> Result { + body.split_whitespace() + .find_map(|field| field.strip_prefix(&format!("{key}="))) + .map(str::to_string) + .ok_or_else(|| format!("missing field {key} in {line:?}")) + }; + let parse_bytes = |key: &str, raw: &str| -> Result, String> { + if raw == "unsupported" { + return Ok(None); } - let _ = std::fs::remove_dir_all(sink_dir); - (status, output, elapsed, overran) + raw.parse::() + .map(Some) + .map_err(|_| format!("bad byte count {key}={raw:?} in {line:?}")) + }; + let mem_bytes_raw = get("mem_bytes")?; + let file_bytes_raw = get("file_bytes")?; + Ok(IrLimitsNote { + stack_raw: get("stack")?, + cpu_raw: get("cpu")?, + mem_kb_raw: get("mem_kb")?, + mem_bytes: parse_bytes("mem_bytes", &mem_bytes_raw)?, + fileblk_raw: get("fileblk")?, + file_bytes: parse_bytes("file_bytes", &file_bytes_raw)?, }) - .await - .expect("child harness join"); - let (status, output, elapsed, overran) = outcome; +} + +/// Classified artifact-sink outcome: absence is normal off-coverage, a +/// present artifact must fit the explicit bound, anything else fails loudly +/// (never confused with absence). +#[derive(Debug)] +enum ArtifactCheck { + Absent, + Within(u64), + Over(u64), +} + +fn check_artifact_bytes(profile: &std::path::Path, bound: u64) -> std::io::Result { + match std::fs::metadata(profile) { + Ok(meta) => Ok(if meta.len() < bound { + ArtifactCheck::Within(meta.len()) + } else { + ArtifactCheck::Over(meta.len()) + }), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(ArtifactCheck::Absent), + Err(e) => Err(e), + } +} + +/// Owned temp working dir: created and guarded before use, removed exactly +/// once via `cleanup`; the Drop backstop only covers unwind paths. +struct TempWorkdir { + path: Option, +} + +impl TempWorkdir { + fn create(tag: &str) -> std::io::Result { + let path = std::env::temp_dir().join(format!("bashkit-{tag}-{}", std::process::id())); + std::fs::create_dir_all(&path)?; + if std::fs::read_dir(&path)?.next().is_some() { + return Err(std::io::Error::new( + std::io::ErrorKind::AlreadyExists, + format!("stale work dir (pid reuse?): {}", path.display()), + )); + } + Ok(Self { path: Some(path) }) + } + + fn path(&self) -> &std::path::Path { + self.path.as_ref().expect("workdir already cleaned") + } + + fn cleanup(mut self) -> std::io::Result<()> { + let path = self.path.take().expect("workdir already cleaned"); + std::fs::remove_dir_all(path) + } +} + +impl Drop for TempWorkdir { + fn drop(&mut self) { + if let Some(path) = self.path.take() { + let _ = std::fs::remove_dir_all(path); + } + } +} + +/// Outcome of one opt-in bounded input-shape run: the reviewed verdict plus +/// job-specific receipts. Setup faults panic with context (fail-closed); +/// runtime observations are flags for the caller to assert on. +struct InputShapeRun { + exit_code: Option, + stdout: String, + stderr: String, + timed_out: bool, + oversized: bool, + incomplete_readers: bool, + read_error: bool, + reaped: bool, + limits: IrLimitsNote, + artifact: ArtifactCheck, + leftovers: Vec, +} + +fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { + let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); + let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); + let profile = sink.path().join("child.profraw"); + let cli = bashkit_cli_path(); + let mut cmd = Command::new("bash"); + cmd.arg("-c") + .arg(input_shape_launcher()) + .arg("ir-child") + .arg(&cli) + .arg("-c") + .arg(script) + .env("LLVM_PROFILE_FILE", &profile) + .current_dir(work.path()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let run: BoundedRun = run_command_bounded_with( + cmd, + Duration::from_secs(wall_secs), + LifecycleFaults::default(), + ); + // Receipt first: the launcher note proves the caps ran; without it the + // run is untrusted regardless of exit code. + let (note, rest) = run + .stderr + .split_once('\n') + .expect("launcher receipt missing: empty stderr"); + let limits = parse_ir_limits_note(note).unwrap_or_else(|e| panic!("bad launcher receipt: {e}")); + let stderr = rest.to_string(); + let artifact = check_artifact_bytes(&profile, IR_ARTIFACT_BOUND) + .unwrap_or_else(|e| panic!("cannot stat artifact sink: {e:?}")); + // Inventory before cleanup: any producer file fails closed by naming it; + // entry errors are loud, never filtered away. + let mut leftovers = Vec::new(); + for entry in std::fs::read_dir(work.path()) + .unwrap_or_else(|e| panic!("cannot inventory work dir: {e:?}")) + { + match entry { + Ok(e) => leftovers.push(e.file_name().to_string_lossy().into_owned()), + Err(e) => panic!("cannot inventory work dir entry: {e:?}"), + } + } + sink.cleanup().expect("artifact sink cleanup"); + work.cleanup().expect("work dir cleanup"); + InputShapeRun { + exit_code: run.exit_code, + stdout: run.stdout, + stderr, + timed_out: run.timed_out, + oversized: run.oversized, + incomplete_readers: run.incomplete_readers, + read_error: run.read_error, + reaped: run.reaped, + limits, + artifact, + leftovers, + } +} + +/// Effective-limits assertions shared by bounded input-shape runs: exact raw +/// round-trips everywhere; kernel byte proofs on Linux, explicit unsupported +/// markers elsewhere (fail-closed on any unit surprise). +fn assert_effective_limits(limits: &IrLimitsNote) { + assert_eq!(limits.stack_raw, "4096", "stack cap round-trip"); + assert_eq!(limits.cpu_raw, "10", "cpu cap round-trip"); + assert_eq!( + limits.fileblk_raw, + IR_FILE_BLOCKS.to_string(), + "file cap round-trip" + ); + #[cfg(target_os = "linux")] + { + assert_eq!( + limits.mem_kb_raw, + IR_AS_KIB.to_string(), + "AS cap round-trip" + ); + assert_eq!(limits.mem_bytes, Some(IR_AS_KIB * 1024), "AS bytes proof"); + assert_eq!( + limits.file_bytes, + Some(IR_FILE_BLOCKS * 512), + "file bytes proof" + ); + } + #[cfg(not(target_os = "linux"))] + { + assert_eq!(limits.mem_kb_raw, "unsupported", "no AS enforcement here"); + assert_eq!(limits.mem_bytes, None, "no AS byteproof here"); + assert_eq!(limits.file_bytes, None, "no file byteproof here"); + } +} + +/// Original shape under default limits in the opt-in bounded runner: `:` +/// plus 1024 same-fd `/dev/zero` redirects is denied quickly by the +/// aggregate input budget (each 1MiB read retains ~2MiB against the 100MB +/// default cap, tripping after ~50 redirects) instead of piling ~2GiB of +/// retained clones. Asserts the exact denial: clean exit code 1 naming +/// `aggregate input bytes` (production retention admission, far above the +/// ~7KiB script text), distinguishing budget denial from OOM/signal death +/// (no exit code) and from pre-fix success (exit 0). +/// NOT EXECUTED in this environment (hostile-fixture ban); kept as the +/// blocked-verification target with full harness assertions intact. +#[cfg(unix)] +#[test] +fn original_shape_bounded_child_default_limits() { + let r = run_input_shape_bounded(&format!(":{}", " 3 std::path::PathBuf { + pub(crate) fn bashkit_cli_path() -> std::path::PathBuf { let exe = std::env::current_exe().expect("current test executable"); match cli_in_ancestors(&exe) { Ok(cli) => cli, @@ -99,30 +99,30 @@ mod resource_exhaustion { } } - struct BoundedRun { - timed_out: bool, - oversized: bool, + pub(crate) struct BoundedRun { + pub(crate) timed_out: bool, + pub(crate) oversized: bool, /// True when the deadline passed with readers unfinished: a foreign /// descendant retains the pipes. Detection, not reaping: without /// platform process-group APIs (no new dependencies) a foreign /// descendant cannot be reaped portably, so the run fails visibly. - incomplete_readers: bool, + pub(crate) incomplete_readers: bool, /// A pipe reader hit a real I/O error (anything but `Interrupted`, /// which is retried). Capture is then partial by definition, so the /// run fails closed instead of relabeling the error as a clean EOF. - read_error: bool, + pub(crate) read_error: bool, /// OS termination accounted for: the loop observed an exit or /// the bounded reap recovered one. Orthogonal to `exit_code` /// (signaled victims have no code) and to injected observation /// faults (which discard the observation, never the cleanup). - reaped: bool, + pub(crate) reaped: bool, /// First stderr line from the launcher (`harness-limits: ...`), /// proving the CPU/stack caps were applied to this run. Empty when /// the launcher itself refused (fail-closed exit 99). - limits_note: String, - exit_code: Option, - stdout: String, - stderr: String, + pub(crate) limits_note: String, + pub(crate) exit_code: Option, + pub(crate) stdout: String, + pub(crate) stderr: String, } /// Fail-closed launcher limits: 10 CPU seconds, 4 MiB main-thread @@ -981,7 +981,7 @@ mod resource_exhaustion { // drives the real lifecycle path with exactly one hostile condition. // All flags stay off outside these tests. #[derive(Clone, Default)] - struct LifecycleFaults { + pub(crate) struct LifecycleFaults { /// Child poll fails: the parent must fail closed, never panic. fail_poll: bool, /// Stdout pipe reads fail: the parent verdict must carry @@ -1011,7 +1011,7 @@ mod resource_exhaustion { /// Standard runner: core lifecycle plus the bounded join gate. Reader /// threads that never reported stay detached by contract. - fn run_command_bounded_with( + pub(crate) fn run_command_bounded_with( cmd: Command, limit: Duration, faults: LifecycleFaults, From c4547ce6e74c8e8c654006a200e62d6f6b7dfab9 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 07:13:21 -0500 Subject: [PATCH 06/16] test(interpreter): owner-guarded bounded runner with measured units Establish OwnedChild immediately after spawn so every fallible pipe, thread, and poll operation runs under defined unwind cleanup. Use exclusive directory creation that rejects collisions without deleting preexisting contents. Bound the ps sampler, retain and complete readers without forgetting handles, and attest the measured 1024-byte shell file-size unit (131072 blocks = 128MiB actual) with factor-tolerant byteproof checks. --- .../input_redirect_budget_tests.rs | 399 ++++++++++++++++-- .../tests/integration/threat_model_tests.rs | 4 +- 2 files changed, 363 insertions(+), 40 deletions(-) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index d6f8aef19..739b45d56 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -346,19 +346,28 @@ async fn stdin_clone_trip_recovers() { } // ---------- opt-in bounded runner extension for the input-redirect job ---------- -// Reuses the reviewed lifecycle (`run_command_bounded_with`) with a +// Reuses reviewed primitives (`drain_capped`, `reap_with_grace`) with a // job-specific launcher. Shared defaults for other products are untouched. use crate::threat_model_tests::resource_exhaustion::{ - BoundedRun, LifecycleFaults, bashkit_cli_path, run_command_bounded_with, + bashkit_cli_path, drain_capped, reap_with_grace, }; use std::process::{Command, Stdio}; use std::time::Duration; /// Address-space cap in KiB for the bash `ulimit -v` knob (Linux only). const IR_AS_KIB: u64 = 1_572_864; // 1.5GiB -/// File-size backstop in POSIX 512B blocks for the bash `ulimit -f` knob. -const IR_FILE_BLOCKS: u64 = 131_072; // 131072 * 512B = 64MiB nominal +/// File-size backstop for the bash `ulimit -f` knob, in the shell's blocks. +/// MEASURED (not assumed): both /bin/bash and /opt/homebrew/bin/bash report +/// 1024-byte increments (`ulimit -f 1` reads back 1024 bytes via +/// resource.getrlimit), so 131072 blocks = 128MiB actual on measured builds. +/// The assertion below accepts exactly the historically attested unit set +/// {512B POSIX, 1024B measured bash} and fails closed on anything else. +const IR_FILE_BLOCKS: u64 = 131_072; +/// Active RSS cutoff in KiB (`ps -o rss=` reports KiB on Linux and macOS): +/// catches runaway growth far below RAM/disk exhaustion while legit runs +/// peak at O(MB). Policed by polling, not assumed from any cap. +const IR_RSS_CAP_KIB: u64 = 1_048_576; // 1GiB /// Explicit allowance for one short instrumented run's coverage artifact. const IR_ARTIFACT_BOUND: u64 = 64 * 1024 * 1024; // 64MiB @@ -459,8 +468,12 @@ fn check_artifact_bytes(profile: &std::path::Path, bound: u64) -> std::io::Resul } } -/// Owned temp working dir: created and guarded before use, removed exactly -/// once via `cleanup`; the Drop backstop only covers unwind paths. +/// Owned temp working dir. Ownership is established FIRST (the guard object +/// exists before any fallible use); every operation runs through the guard. +/// `cleanup` disarms only after removal succeeds, so a failed cleanup keeps +/// the guard armed and fails loudly instead of leaking silently. `Drop` is a +/// backstop for unwind paths only and documents its silent best-effort. +#[derive(Debug)] struct TempWorkdir { path: Option, } @@ -468,27 +481,34 @@ struct TempWorkdir { impl TempWorkdir { fn create(tag: &str) -> std::io::Result { let path = std::env::temp_dir().join(format!("bashkit-{tag}-{}", std::process::id())); - std::fs::create_dir_all(&path)?; - if std::fs::read_dir(&path)?.next().is_some() { - return Err(std::io::Error::new( + // Exclusive creation: existence is NOT proof this invocation created + // the directory. A collision rejects WITHOUT deleting so preexisting + // contents (possibly another owner's) are always preserved. + match std::fs::create_dir(&path) { + Ok(()) => Ok(Self { path: Some(path) }), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => Err(std::io::Error::new( std::io::ErrorKind::AlreadyExists, - format!("stale work dir (pid reuse?): {}", path.display()), - )); + format!("work dir collision, refusing to adopt: {}", path.display()), + )), + Err(e) => Err(e), } - Ok(Self { path: Some(path) }) } - fn path(&self) -> &std::path::Path { self.path.as_ref().expect("workdir already cleaned") } fn cleanup(mut self) -> std::io::Result<()> { - let path = self.path.take().expect("workdir already cleaned"); - std::fs::remove_dir_all(path) + let path = self.path.as_ref().expect("workdir already cleaned"); + std::fs::remove_dir_all(path)?; + self.path = None; + Ok(()) } } impl Drop for TempWorkdir { + // Backstop only: the explicit `cleanup` path above is the truthful + // receipt. A Drop-time failure has nowhere to report, hence best-effort; + // normal paths never reach here armed. fn drop(&mut self) { if let Some(path) = self.path.take() { let _ = std::fs::remove_dir_all(path); @@ -496,6 +516,29 @@ impl Drop for TempWorkdir { } } +/// Parse `ps -o rss=` output (KiB on Linux and macOS) to a byte-exact... +/// KiB count. Anything else fails loudly: a policing loop must never parse +/// garbage as a small number. +fn parse_ps_rss(out: &str) -> Result { + out.trim() + .parse::() + .map_err(|_| format!("unparseable ps rss output: {out:?}")) +} + +/// Attested file-size unit factor: the byteproof must equal blocks times one +/// of the historically attested shell units (512B POSIX, 1024B measured +/// bash). Anything else fails closed instead of converting silently. +fn file_size_unit_factor(blocks: u64, bytes: u64) -> Result { + for unit in [512u64, 1024u64] { + if blocks.saturating_mul(unit) == bytes { + return Ok(unit); + } + } + Err(format!( + "file byteproof {bytes} matches neither 512B nor 1024B units for {blocks} blocks" + )) +} + /// Outcome of one opt-in bounded input-shape run: the reviewed verdict plus /// job-specific receipts. Setup faults panic with context (fail-closed); /// runtime observations are flags for the caller to assert on. @@ -511,6 +554,77 @@ struct InputShapeRun { limits: IrLimitsNote, artifact: ArtifactCheck, leftovers: Vec, + file_unit: Option, + rss_pid: u32, + rss_samples: u32, + peak_rss_kb: u64, + rss_exceeded: bool, + exited_fast: bool, +} + +/// Sample resident-set size (KiB) of `pid` via `ps` (`rss` is KiB on +/// Linux and macOS). Any failure is loud: blind policing is worse than none, +/// and the caller kills first on doubt. +fn sample_child_rss(pid: u32) -> Result { + // The sampler itself is bounded (5s): `ps` must never block past the + // wall deadline. Kill and reap on expiry, loud error either way. + let mut ps = std::process::Command::new("ps") + .arg("-o") + .arg("rss=") + .arg("-p") + .arg(pid.to_string()) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .map_err(|e| format!("spawn ps for rss of {pid}: {e:?}"))?; + let ps_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + match ps.try_wait() { + Ok(Some(_)) => break, + Ok(None) => { + if std::time::Instant::now() >= ps_deadline { + ps.kill() + .map_err(|e| format!("kill stuck ps for rss of {pid}: {e:?}"))?; + let _ = ps.wait(); + return Err(format!("ps rss of {pid} timed out")); + } + std::thread::sleep(std::time::Duration::from_millis(50)); + } + Err(e) => return Err(format!("poll ps for rss of {pid}: {e:?}")), + } + } + let out = ps + .wait_with_output() + .map_err(|e| format!("reap ps for rss of {pid}: {e:?}"))?; + if !out.status.success() { + return Err(format!( + "ps rss of {pid} failed: {}", + String::from_utf8_lossy(&out.stderr).trim() + )); + } + parse_ps_rss(&String::from_utf8_lossy(&out.stdout)) +} + +/// Join a pipe-reader thread before `deadline`. On time, returns its +/// receipts with `true`. Past the deadline, one final blocking join +/// completes the original reader: pipes hit EOF once the reaped child is +/// gone and our shapes spawn no grandchildren, so this cannot hang on a live +/// pipe; it is still flagged `false` so the verdict fails closed. The +/// handle is never forgotten or dropped unfinished. +fn ir_join_capped( + handle: std::thread::JoinHandle<(Vec, bool, bool)>, + deadline: std::time::Instant, +) -> (Vec, bool, bool, bool) { + while !handle.is_finished() { + if std::time::Instant::now() >= deadline { + break; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + let finished = handle.is_finished(); + let (kept, oversized, read_error) = handle.join().expect("reader thread join"); + (kept, oversized, read_error, finished) } fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { @@ -530,19 +644,114 @@ fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShap .stdin(Stdio::null()) .stdout(Stdio::piped()) .stderr(Stdio::piped()); - let run: BoundedRun = run_command_bounded_with( - cmd, - Duration::from_secs(wall_secs), - LifecycleFaults::default(), - ); + let mut child = OwnedChild::new(cmd.spawn().expect("spawn bounded child")); + let rss_pid = child.id(); + // Pipes are taken before the first poll so the concurrent readers below + // own them outright; the child handle keeps only pid and lifecycle. + let (out_pipe, err_pipe) = child.take_pipes(); + let mut out_pipe = out_pipe.expect("take child stdout"); + let mut err_pipe = err_pipe.expect("take child stderr"); + let out_reader = std::thread::Builder::new() + .name("ir-child-stdout".into()) + .spawn(move || drain_capped(&mut out_pipe)) + .expect("spawn stdout reader"); + let err_reader = std::thread::Builder::new() + .name("ir-child-stderr".into()) + .spawn(move || drain_capped(&mut err_pipe)) + .expect("spawn stderr reader"); + let start = std::time::Instant::now(); + let deadline = start + Duration::from_secs(wall_secs); + let mut ticks = 0u64; + let mut rss_samples = 0u32; + let mut peak_rss_kb = 0u64; + let mut rss_exceeded = false; + let mut exited_fast = false; + let mut overran = false; + // try_wait reaps the zombie it reports, so the exited branch needs no + // further wait; killed branches go through the reviewed grace reap, + // never an open wait. + let mut final_status: Option = None; + let mut reaped = false; + loop { + if let Some(status) = child.try_wait().expect("poll child") { + if rss_samples == 0 { + exited_fast = true; + } + final_status = Some(status); + reaped = true; + break; + } + ticks += 1; + if ticks == 1 || ticks.is_multiple_of(10) { + match sample_child_rss(rss_pid) { + Ok(rss) => { + rss_samples += 1; + peak_rss_kb = peak_rss_kb.max(rss); + if rss > IR_RSS_CAP_KIB { + child.kill().expect("kill RSS overrun child"); + rss_exceeded = true; + break; + } + } + Err(e) => { + // The pid may have vanished between try_wait and ps; + // re-check before treating this as a policing fault. + if let Some(status) = child.try_wait().expect("re-poll child") { + if rss_samples == 0 { + exited_fast = true; + } + final_status = Some(status); + reaped = true; + break; + } + panic!("RSS policing failed on live child {rss_pid}: {e}"); + } + } + } + if std::time::Instant::now() >= deadline { + child.kill().expect("kill overrun child"); + overran = true; + break; + } + std::thread::sleep(std::time::Duration::from_millis(50)); + } + if final_status.is_none() { + let mut poll = || child.try_wait(); + match reap_with_grace(&mut poll, Duration::from_secs(5)) { + Some(status) => { + final_status = Some(status); + reaped = true; + } + None => { + reaped = false; + } + } + } + // Finite reader completion shares the absolute budget: pipes hit EOF + // once the reaped child is gone. See ir_join_capped on unfinished. + let join_deadline = start + Duration::from_secs(wall_secs + 15); + let (out_kept, out_over, out_err, out_done) = ir_join_capped(out_reader, join_deadline); + let (err_kept, err_over, err_err, err_done) = ir_join_capped(err_reader, join_deadline); + let incomplete_readers = !(out_done && err_done); // Receipt first: the launcher note proves the caps ran; without it the // run is untrusted regardless of exit code. - let (note, rest) = run - .stderr + let stderr_full = String::from_utf8_lossy(&err_kept).into_owned(); + let (note, rest) = stderr_full .split_once('\n') .expect("launcher receipt missing: empty stderr"); let limits = parse_ir_limits_note(note).unwrap_or_else(|e| panic!("bad launcher receipt: {e}")); let stderr = rest.to_string(); + #[cfg(target_os = "linux")] + let file_unit = { + let blocks: u64 = limits + .fileblk_raw + .parse() + .expect("fileblk receipt must be numeric"); + let bytes = limits.file_bytes.expect("file byteproof required on Linux"); + Some(file_size_unit_factor(blocks, bytes).unwrap_or_else(|e| panic!("file unit cap: {e}"))) + }; + #[cfg(not(target_os = "linux"))] + let file_unit = None; let artifact = check_artifact_bytes(&profile, IR_ARTIFACT_BOUND) .unwrap_or_else(|e| panic!("cannot stat artifact sink: {e:?}")); // Inventory before cleanup: any producer file fails closed by naming it; @@ -559,24 +768,37 @@ fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShap sink.cleanup().expect("artifact sink cleanup"); work.cleanup().expect("work dir cleanup"); InputShapeRun { - exit_code: run.exit_code, - stdout: run.stdout, + exit_code: final_status.and_then(|s| s.code()), + stdout: String::from_utf8_lossy(&out_kept).into_owned(), stderr, - timed_out: run.timed_out, - oversized: run.oversized, - incomplete_readers: run.incomplete_readers, - read_error: run.read_error, - reaped: run.reaped, + timed_out: overran, + oversized: out_over || err_over, + incomplete_readers, + read_error: out_err || err_err, + reaped, limits, artifact, leftovers, + file_unit, + rss_pid, + rss_samples, + peak_rss_kb, + rss_exceeded, + exited_fast, } } /// Effective-limits assertions shared by bounded input-shape runs: exact raw /// round-trips everywhere; kernel byte proofs on Linux, explicit unsupported /// markers elsewhere (fail-closed on any unit surprise). -fn assert_effective_limits(limits: &IrLimitsNote) { +/// Effective-bounds assertions shared by bounded input-shape runs: exact raw +/// round-trips everywhere; kernel byte proofs on Linux (including the +/// shell-unit factor for `-f`); explicit unsupported markers elsewhere +/// (fail-closed on any unit surprise); RSS policing receipts (a real pid, +/// no exceedance, peak under the cutoff, samples taken unless the child +/// exited before the first poll). +fn assert_effective_bounds(r: &InputShapeRun) { + let limits = &r.limits; assert_eq!(limits.stack_raw, "4096", "stack cap round-trip"); assert_eq!(limits.cpu_raw, "10", "cpu cap round-trip"); assert_eq!( @@ -592,10 +814,9 @@ fn assert_effective_limits(limits: &IrLimitsNote) { "AS cap round-trip" ); assert_eq!(limits.mem_bytes, Some(IR_AS_KIB * 1024), "AS bytes proof"); - assert_eq!( - limits.file_bytes, - Some(IR_FILE_BLOCKS * 512), - "file bytes proof" + assert!( + r.file_unit.is_some(), + "file unit factor must be attested on Linux" ); } #[cfg(not(target_os = "linux"))] @@ -603,7 +824,23 @@ fn assert_effective_limits(limits: &IrLimitsNote) { assert_eq!(limits.mem_kb_raw, "unsupported", "no AS enforcement here"); assert_eq!(limits.mem_bytes, None, "no AS byteproof here"); assert_eq!(limits.file_bytes, None, "no file byteproof here"); + assert_eq!(r.file_unit, None, "no file unit attested here"); } + assert_ne!(r.rss_pid, 0, "RSS policing must attach to a real pid"); + assert!( + !r.rss_exceeded, + "RSS cutoff tripped at {} KiB", + r.peak_rss_kb + ); + assert!( + r.peak_rss_kb < IR_RSS_CAP_KIB, + "peak RSS {} KiB over cutoff", + r.peak_rss_kb + ); + assert!( + r.rss_samples >= 1 || r.exited_fast, + "RSS policing never sampled a live child" + ); } /// Original shape under default limits in the opt-in bounded runner: `:` @@ -628,7 +865,7 @@ fn original_shape_bounded_child_default_limits() { ); assert!(!r.read_error, "bounded child pipe read failed"); assert!(r.reaped, "bounded child was not reaped"); - assert_effective_limits(&r.limits); + assert_effective_bounds(&r); assert!( matches!(r.artifact, ArtifactCheck::Absent | ArtifactCheck::Within(_)), "artifact outside explicit allowance: {:?}", @@ -672,13 +909,13 @@ fn ir_helper_launcher_fragments() { #[test] fn ir_helper_parse_receipt() { - let full = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=1572864 mem_bytes=1610612736 fileblk=131072 file_bytes=67108864").unwrap(); + let full = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=1572864 mem_bytes=1610612736 fileblk=131072 file_bytes=134217728").unwrap(); assert_eq!(full.stack_raw, "4096"); assert_eq!(full.cpu_raw, "10"); assert_eq!(full.mem_kb_raw, "1572864"); assert_eq!(full.mem_bytes, Some(1610612736)); assert_eq!(full.fileblk_raw, "131072"); - assert_eq!(full.file_bytes, Some(67108864)); + assert_eq!(full.file_bytes, Some(134217728)); let unsupported = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=unsupported mem_bytes=unsupported fileblk=131072 file_bytes=unsupported").unwrap(); assert_eq!(unsupported.mem_bytes, None); assert_eq!(unsupported.file_bytes, None); @@ -742,7 +979,7 @@ fn ir_helper_normal_child_completes() { !r.timed_out && !r.oversized && !r.incomplete_readers && !r.read_error && r.reaped, "harness lifecycle must complete cleanly" ); - assert_effective_limits(&r.limits); + assert_effective_bounds(&r); assert_eq!(r.exit_code, Some(0)); assert!(r.stdout.is_empty() && r.stderr.is_empty()); assert!(r.leftovers.is_empty()); @@ -751,3 +988,89 @@ fn ir_helper_normal_child_completes() { ArtifactCheck::Absent | ArtifactCheck::Within(_) )); } + +#[test] +fn ir_helper_file_unit_cases() { + // Measured shell units: 1024B increments on tested bash builds, 512B POSIX. + assert_eq!(file_size_unit_factor(131072, 131072 * 1024), Ok(1024)); + assert_eq!(file_size_unit_factor(131072, 131072 * 512), Ok(512)); + assert!(file_size_unit_factor(131072, 131072 * 1000).is_err()); + assert!(file_size_unit_factor(131072, 0).is_err()); +} +/// A preexisting dir must fail creation AND keep every byte: existence is +/// not proof this invocation created it, so a collision rejects without +/// deleting (possibly another owner's contents). +#[test] +fn ir_helper_workdir_collision_preserves() { + let tag = "ir-collision-probe"; + let path = std::env::temp_dir().join(format!("bashkit-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&path); + std::fs::create_dir_all(&path).expect("plant collision dir"); + std::fs::write(path.join("precious"), b"precious").expect("plant collision file"); + let err = TempWorkdir::create(tag).expect_err("collision must fail"); + assert_eq!(err.kind(), std::io::ErrorKind::AlreadyExists); + assert_eq!( + std::fs::read(path.join("precious")).expect("preexisting contents preserved"), + b"precious" + ); + std::fs::remove_dir_all(&path).expect("remove planted dir"); + assert!(!path.exists()); +} + +#[test] +fn ir_helper_ps_parse_cases() { + assert_eq!(parse_ps_rss(" 1234\n"), Ok(1234)); + assert_eq!(parse_ps_rss("0"), Ok(0)); + assert!(parse_ps_rss("").is_err()); + assert!(parse_ps_rss("abc").is_err()); + assert!(parse_ps_rss("12 34").is_err()); +} + +/// Owned child handle: established immediately after spawn, before any +/// fallible pipe/thread/poll operation. Every later expect runs with this +/// owner alive, so unwind paths always have a defined backstop. Normal +/// paths reap explicitly (see `reap_with_grace` callers) and then drop the +/// guard; Drop kills best-effort (documented backstop, never the primary +/// path) but never waits, so Drop cannot block. +struct OwnedChild { + inner: Option, +} + +impl OwnedChild { + fn new(child: std::process::Child) -> Self { + Self { inner: Some(child) } + } + + fn id(&self) -> u32 { + self.inner.as_ref().expect("child already released").id() + } + + fn take_pipes( + &mut self, + ) -> ( + Option, + Option, + ) { + let inner = self.inner.as_mut().expect("child already released"); + (inner.stdout.take(), inner.stderr.take()) + } + + fn try_wait(&mut self) -> std::io::Result> { + self.inner + .as_mut() + .expect("child already released") + .try_wait() + } + + fn kill(&mut self) -> std::io::Result<()> { + self.inner.as_mut().expect("child already released").kill() + } +} + +impl Drop for OwnedChild { + fn drop(&mut self) { + if let Some(child) = self.inner.as_mut() { + let _ = child.kill(); + } + } +} diff --git a/crates/bashkit/tests/integration/threat_model_tests.rs b/crates/bashkit/tests/integration/threat_model_tests.rs index 8f2ec24e8..765f4104a 100644 --- a/crates/bashkit/tests/integration/threat_model_tests.rs +++ b/crates/bashkit/tests/integration/threat_model_tests.rs @@ -186,7 +186,7 @@ pub(crate) mod resource_exhaustion { /// the capture with the bytes kept so far and `read_error` set, never /// relabeled as a clean EOF. Reaching exactly the cap followed by EOF /// is not oversize; only real excess is flagged. - fn drain_capped(reader: &mut R) -> (Vec, bool, bool) { + pub(crate) fn drain_capped(reader: &mut R) -> (Vec, bool, bool) { let mut buf = Vec::new(); let mut chunk = [0u8; 4096]; loop { @@ -935,7 +935,7 @@ pub(crate) mod resource_exhaustion { /// Returns the observed status, or `None` when the victim cannot be /// reaped inside the grace (or polling itself errors): the safe seam /// for the unavailable-reap verdict. - fn reap_with_grace( + pub(crate) fn reap_with_grace( poll: &mut dyn FnMut() -> std::io::Result>, grace: Duration, ) -> Option { From 791f59804b2cff2e24b010753d358d487260c0c5 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 09:42:53 -0500 Subject: [PATCH 07/16] test(interpreter): deadline-aware async child lifecycle for input-redirect job Drive concurrent capped drains via select from the first poll with no detached tasks. Bound the ps sampler (timeout, kill, explicit reap receipt) and reap the producer through cancel-safe wait with finite deadlines. Establish OwnedChild immediately after spawn; replace the forgetting join with retain-and-complete semantics. TEST-ONLY tokio process feature (dev-dependencies); production features, versions, and lockfile unchanged. --- crates/bashkit/Cargo.toml | 3 + .../input_redirect_budget_tests.rs | 412 +++++++++++++----- 2 files changed, 295 insertions(+), 120 deletions(-) diff --git a/crates/bashkit/Cargo.toml b/crates/bashkit/Cargo.toml index ba28efdf6..e257e9a98 100644 --- a/crates/bashkit/Cargo.toml +++ b/crates/bashkit/Cargo.toml @@ -297,6 +297,9 @@ all-features = true rustdoc-args = ["--cfg", "docsrs"] [dev-dependencies] +# TEST-ONLY: process feature for the opt-in bounded child-test runner. +# Production builds never see this feature; versions and lock unchanged. +tokio = { workspace = true, features = ["process"] } tokio-test = { workspace = true } pretty_assertions = { workspace = true } insta = { workspace = true } diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 739b45d56..289da904a 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -349,11 +349,10 @@ async fn stdin_clone_trip_recovers() { // Reuses reviewed primitives (`drain_capped`, `reap_with_grace`) with a // job-specific launcher. Shared defaults for other products are untouched. -use crate::threat_model_tests::resource_exhaustion::{ - bashkit_cli_path, drain_capped, reap_with_grace, -}; -use std::process::{Command, Stdio}; +use crate::threat_model_tests::resource_exhaustion::bashkit_cli_path; +use std::process::Stdio; use std::time::Duration; +use tokio::process::Command; /// Address-space cap in KiB for the bash `ulimit -v` knob (Linux only). const IR_AS_KIB: u64 = 1_572_864; // 1.5GiB @@ -370,6 +369,8 @@ const IR_FILE_BLOCKS: u64 = 131_072; const IR_RSS_CAP_KIB: u64 = 1_048_576; // 1GiB /// Explicit allowance for one short instrumented run's coverage artifact. const IR_ARTIFACT_BOUND: u64 = 64 * 1024 * 1024; // 64MiB +/// Per-stream kept bytes for concurrent diagnostic drains. +const PIPE_CAP: usize = 256 * 1024; /// Opt-in launcher for the input-redirect job: shared CPU/stack caps plus an /// address-space cap and a file-size backstop, every `ulimit` fail-closed @@ -565,10 +566,45 @@ struct InputShapeRun { /// Sample resident-set size (KiB) of `pid` via `ps` (`rss` is KiB on /// Linux and macOS). Any failure is loud: blind policing is worse than none, /// and the caller kills first on doubt. -fn sample_child_rss(pid: u32) -> Result { - // The sampler itself is bounded (5s): `ps` must never block past the - // wall deadline. Kill and reap on expiry, loud error either way. - let mut ps = std::process::Command::new("ps") +/// Bounded async drain: keeps at most `cap` bytes, counts the rest without +/// storing. Cancellation-safe per `AsyncReadExt::read` (locked tokio +/// 1.53.2): dropping the future abandons only unread bytes, so a deadline +/// timeout followed by kill+reap stays finite. Never collects unbounded +/// `read_to_end`. +async fn ir_drain_capped_async(pipe: &mut R, cap: usize) -> (Vec, usize, bool) +where + R: tokio::io::AsyncRead + Unpin, +{ + use tokio::io::AsyncReadExt; + let mut kept = Vec::new(); + let mut total = 0usize; + let mut had_error = false; + let mut chunk = [0u8; 8192]; + loop { + match pipe.read(&mut chunk).await { + Ok(0) => break, + Ok(n) => { + let room = cap.saturating_sub(total).min(n); + kept.extend_from_slice(&chunk[..room]); + total += n; + } + Err(_) => { + had_error = true; + break; + } + } + } + let kept_len = kept.len(); + (kept, total.saturating_sub(kept_len), had_error) +} + +/// Bounded RSS sampler on the same async machinery (no std `output()` or +/// `wait` escape): timeout, kill with finite reap, loud errors. Preserves +/// the effective pid receipt and distinguishes sampling failure from a +/// child that simply exited fast. +async fn sample_child_rss(pid: u32) -> Result { + use std::time::Duration; + let mut ps = tokio::process::Command::new("ps") .arg("-o") .arg("rss=") .arg("-p") @@ -576,58 +612,58 @@ fn sample_child_rss(pid: u32) -> Result { .stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) + .kill_on_drop(true) .spawn() .map_err(|e| format!("spawn ps for rss of {pid}: {e:?}"))?; - let ps_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + // Take the pipe upfront so the bounded drain below owns capture; the + // one-line output then cannot block or grow past its cap. + let mut out_pipe = ps + .stdout + .take() + .ok_or_else(|| format!("ps {pid} has no stdout"))?; + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); loop { match ps.try_wait() { Ok(Some(_)) => break, Ok(None) => { - if std::time::Instant::now() >= ps_deadline { - ps.kill() + if tokio::time::Instant::now() >= deadline { + ps.start_kill() .map_err(|e| format!("kill stuck ps for rss of {pid}: {e:?}"))?; - let _ = ps.wait(); - return Err(format!("ps rss of {pid} timed out")); + // Finite reap with explicit receipt (not `let _`): either + // the kill is confirmed or the timeout is recorded. + let reaped = + tokio::time::timeout_at(deadline + Duration::from_secs(2), ps.wait()) + .await + .is_ok(); + return Err(format!("ps rss of {pid} timed out (reaped: {reaped})")); } - std::thread::sleep(std::time::Duration::from_millis(50)); + tokio::time::sleep(Duration::from_millis(50)).await; } Err(e) => return Err(format!("poll ps for rss of {pid}: {e:?}")), } } - let out = ps - .wait_with_output() - .map_err(|e| format!("reap ps for rss of {pid}: {e:?}"))?; - if !out.status.success() { - return Err(format!( - "ps rss of {pid} failed: {}", - String::from_utf8_lossy(&out.stderr).trim() - )); + // Reap explicitly (already exited: prompt), then bounded-drain the + // one-line output. Never wait_with_output without bounds. + match tokio::time::timeout_at( + tokio::time::Instant::now() + Duration::from_secs(2), + ps.wait(), + ) + .await + { + Ok(Ok(_)) => {} + Ok(Err(e)) => return Err(format!("reap ps for rss of {pid}: {e:?}")), + Err(_) => return Err(format!("reap ps for rss of {pid} timed out")), + } + let (kept, trunc, had_error) = ir_drain_capped_async(&mut out_pipe, 4096).await; + if had_error { + return Err(format!("ps rss of {pid} output unreadable")); } - parse_ps_rss(&String::from_utf8_lossy(&out.stdout)) -} - -/// Join a pipe-reader thread before `deadline`. On time, returns its -/// receipts with `true`. Past the deadline, one final blocking join -/// completes the original reader: pipes hit EOF once the reaped child is -/// gone and our shapes spawn no grandchildren, so this cannot hang on a live -/// pipe; it is still flagged `false` so the verdict fails closed. The -/// handle is never forgotten or dropped unfinished. -fn ir_join_capped( - handle: std::thread::JoinHandle<(Vec, bool, bool)>, - deadline: std::time::Instant, -) -> (Vec, bool, bool, bool) { - while !handle.is_finished() { - if std::time::Instant::now() >= deadline { - break; - } - std::thread::sleep(std::time::Duration::from_millis(20)); + if trunc > 0 { + return Err(format!("ps rss of {pid} output oversize")); } - let finished = handle.is_finished(); - let (kept, oversized, read_error) = handle.join().expect("reader thread join"); - (kept, oversized, read_error, finished) + parse_ps_rss(&String::from_utf8_lossy(&kept)) } - -fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { +async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); let profile = sink.path().join("child.profraw"); @@ -643,96 +679,139 @@ fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShap .current_dir(work.path()) .stdin(Stdio::null()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()); + .stderr(Stdio::piped()) + // Backstop only: SIGKILL on drop if the explicit kill+reap below + // never runs. Not proof of anything; receipts are. + .kill_on_drop(true); let mut child = OwnedChild::new(cmd.spawn().expect("spawn bounded child")); let rss_pid = child.id(); - // Pipes are taken before the first poll so the concurrent readers below - // own them outright; the child handle keeps only pid and lifecycle. + // Pipes are taken before the first poll so the concurrent drains below + // own them outright; the owner guard keeps pid and lifecycle. let (out_pipe, err_pipe) = child.take_pipes(); let mut out_pipe = out_pipe.expect("take child stdout"); let mut err_pipe = err_pipe.expect("take child stderr"); - let out_reader = std::thread::Builder::new() - .name("ir-child-stdout".into()) - .spawn(move || drain_capped(&mut out_pipe)) - .expect("spawn stdout reader"); - let err_reader = std::thread::Builder::new() - .name("ir-child-stderr".into()) - .spawn(move || drain_capped(&mut err_pipe)) - .expect("spawn stderr reader"); - let start = std::time::Instant::now(); + let start = tokio::time::Instant::now(); let deadline = start + Duration::from_secs(wall_secs); + let join_deadline = start + Duration::from_secs(wall_secs + 15); let mut ticks = 0u64; let mut rss_samples = 0u32; let mut peak_rss_kb = 0u64; let mut rss_exceeded = false; let mut exited_fast = false; let mut overran = false; - // try_wait reaps the zombie it reports, so the exited branch needs no - // further wait; killed branches go through the reviewed grace reap, - // never an open wait. let mut final_status: Option = None; let mut reaped = false; + let mut out_done = false; + let mut err_done = false; + let mut out_kept = Vec::new(); + let mut out_trunc = 0usize; + let mut out_err = false; + let mut err_kept = Vec::new(); + let mut err_trunc = 0usize; + let mut err_err = false; + let mut incomplete_readers = false; + // Drain futures live for the whole supervision scope and are polled + // alongside exit, RSS, and deadline from the first iteration: output can + // never block the producer. No spawned tasks; dropping the futures on + // timeout is cancel-safe, and the pipes die with the reaped child. + // Admission stays capped: each drain keeps PIPE_CAP bytes and counts the + // rest without storing. + let mut out_fut = std::pin::pin!(ir_drain_capped_async(&mut out_pipe, PIPE_CAP)); + let mut err_fut = std::pin::pin!(ir_drain_capped_async(&mut err_pipe, PIPE_CAP)); loop { - if let Some(status) = child.try_wait().expect("poll child") { - if rss_samples == 0 { - exited_fast = true; - } - final_status = Some(status); - reaped = true; + let running_done = final_status.is_some() || overran || rss_exceeded; + if running_done && out_done && err_done { break; } - ticks += 1; - if ticks == 1 || ticks.is_multiple_of(10) { - match sample_child_rss(rss_pid) { - Ok(rss) => { - rss_samples += 1; - peak_rss_kb = peak_rss_kb.max(rss); - if rss > IR_RSS_CAP_KIB { - child.kill().expect("kill RSS overrun child"); - rss_exceeded = true; - break; - } - } - Err(e) => { - // The pid may have vanished between try_wait and ps; - // re-check before treating this as a policing fault. - if let Some(status) = child.try_wait().expect("re-poll child") { + if running_done && tokio::time::Instant::now() >= join_deadline { + incomplete_readers = true; + break; + } + tokio::select! { + biased; + status = child.wait(), if !running_done => { + match status { + Ok(s) => { if rss_samples == 0 { exited_fast = true; } - final_status = Some(status); + final_status = Some(s); reaped = true; - break; } - panic!("RSS policing failed on live child {rss_pid}: {e}"); + Err(e) => panic!("wait child {rss_pid} failed: {e:?}"), } } + _ = tokio::time::sleep(Duration::from_millis(50)) => { + if running_done { + continue; + } + ticks += 1; + if ticks == 1 || ticks.is_multiple_of(10) { + match sample_child_rss(rss_pid).await { + Ok(rss) => { + rss_samples += 1; + peak_rss_kb = peak_rss_kb.max(rss); + if rss > IR_RSS_CAP_KIB { + child.start_kill().expect("kill RSS overrun child"); + rss_exceeded = true; + } + } + Err(e) => { + if let Some(status) = child.try_wait().expect("re-poll child") { + if rss_samples == 0 { + exited_fast = true; + } + final_status = Some(status); + reaped = true; + } else { + panic!("RSS policing failed on live child {rss_pid}: {e}"); + } + } + } + } + if !rss_exceeded && final_status.is_none() && tokio::time::Instant::now() >= deadline { + child.start_kill().expect("kill overrun child"); + overran = true; + } + } + out = &mut out_fut, if !out_done => { + let (kept, trunc, err) = out; + out_kept = kept; + out_trunc = trunc; + out_err = err; + out_done = true; + } + err = &mut err_fut, if !err_done => { + let (kept, trunc, err) = err; + err_kept = kept; + err_trunc = trunc; + err_err = err; + err_done = true; + } } - if std::time::Instant::now() >= deadline { - child.kill().expect("kill overrun child"); - overran = true; - break; - } - std::thread::sleep(std::time::Duration::from_millis(50)); } + // Finite reap for killed paths (exited paths already reaped via wait): + // Child::wait is cancel-safe; the timeout bounds it. start_kill was + // already sent on every non-exit path above. if final_status.is_none() { - let mut poll = || child.try_wait(); - match reap_with_grace(&mut poll, Duration::from_secs(5)) { - Some(status) => { + match tokio::time::timeout_at( + tokio::time::Instant::now() + Duration::from_secs(5), + child.wait(), + ) + .await + { + Ok(Ok(status)) => { final_status = Some(status); reaped = true; } - None => { + Ok(Err(e)) => panic!("reap child {rss_pid} failed: {e:?}"), + Err(_) => { reaped = false; } } } - // Finite reader completion shares the absolute budget: pipes hit EOF - // once the reaped child is gone. See ir_join_capped on unfinished. - let join_deadline = start + Duration::from_secs(wall_secs + 15); - let (out_kept, out_over, out_err, out_done) = ir_join_capped(out_reader, join_deadline); - let (err_kept, err_over, err_err, err_done) = ir_join_capped(err_reader, join_deadline); - let incomplete_readers = !(out_done && err_done); + let elapsed = start.elapsed(); + let _ = elapsed; // Receipt first: the launcher note proves the caps ran; without it the // run is untrusted regardless of exit code. let stderr_full = String::from_utf8_lossy(&err_kept).into_owned(); @@ -772,7 +851,7 @@ fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShap stdout: String::from_utf8_lossy(&out_kept).into_owned(), stderr, timed_out: overran, - oversized: out_over || err_over, + oversized: out_trunc > 0 || err_trunc > 0, incomplete_readers, read_error: out_err || err_err, reaped, @@ -854,9 +933,10 @@ fn assert_effective_bounds(r: &InputShapeRun) { /// NOT EXECUTED in this environment (hostile-fixture ban); kept as the /// blocked-verification target with full harness assertions intact. #[cfg(unix)] -#[test] -fn original_shape_bounded_child_default_limits() { - let r = run_input_shape_bounded(&format!(":{}", " 3 {} + Err(e) => panic!("fixture root busy, clean {path:?} manually: {e:?}"), + } std::fs::write(path.join("precious"), b"precious").expect("plant collision file"); let err = TempWorkdir::create(tag).expect_err("collision must fail"); assert_eq!(err.kind(), std::io::ErrorKind::AlreadyExists); @@ -1013,7 +1097,7 @@ fn ir_helper_workdir_collision_preserves() { std::fs::read(path.join("precious")).expect("preexisting contents preserved"), b"precious" ); - std::fs::remove_dir_all(&path).expect("remove planted dir"); + std::fs::remove_dir_all(&path).expect("remove owned planted dir"); assert!(!path.exists()); } @@ -1032,24 +1116,34 @@ fn ir_helper_ps_parse_cases() { /// paths reap explicitly (see `reap_with_grace` callers) and then drop the /// guard; Drop kills best-effort (documented backstop, never the primary /// path) but never waits, so Drop cannot block. +/// Owned child handle: established immediately after spawn, before any +/// fallible pipe/poll operation. Every later expect runs with this owner +/// alive, so unwind paths always have a defined backstop. Normal paths reap +/// explicitly with a finite deadline and then drop the guard; Drop sends +/// SIGKILL best-effort via sync `start_kill` (documented backstop, never the +/// primary path) but never waits, so Drop cannot block. struct OwnedChild { - inner: Option, + inner: Option, } impl OwnedChild { - fn new(child: std::process::Child) -> Self { + fn new(child: tokio::process::Child) -> Self { Self { inner: Some(child) } } fn id(&self) -> u32 { - self.inner.as_ref().expect("child already released").id() + self.inner + .as_ref() + .expect("child already released") + .id() + .expect("child has pid") } fn take_pipes( &mut self, ) -> ( - Option, - Option, + Option, + Option, ) { let inner = self.inner.as_mut().expect("child already released"); (inner.stdout.take(), inner.stderr.take()) @@ -1062,15 +1156,93 @@ impl OwnedChild { .try_wait() } - fn kill(&mut self) -> std::io::Result<()> { - self.inner.as_mut().expect("child already released").kill() + fn start_kill(&mut self) -> std::io::Result<()> { + self.inner + .as_mut() + .expect("child already released") + .start_kill() + } + + async fn wait(&mut self) -> std::io::Result { + self.inner + .as_mut() + .expect("child already released") + .wait() + .await } } impl Drop for OwnedChild { fn drop(&mut self) { if let Some(child) = self.inner.as_mut() { - let _ = child.kill(); + let _ = child.start_kill(); } } } + +/// In-memory async drain contract: 100B through a duplex pair with a 16B +/// cap keeps 16, counts 84 truncated, no error. Harmless synthetic fixture: +/// proves the bounded-drain primitive admits before growth without any +/// child, pipe, or resource pressure. +#[tokio::test] +async fn ir_helper_async_drain_bounded() { + let (mut writer, mut reader) = tokio::io::duplex(256); + tokio::io::AsyncWriteExt::write_all(&mut writer, &[9u8; 100]) + .await + .expect("fill duplex"); + drop(writer); + let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16).await; + assert_eq!(kept.len(), 16); + assert_eq!(trunc, 84); + assert!(!err); +} + +/// Injected I/O error contract: a reader failing on first poll reports +/// had_error instead of hanging or misattributing bytes. +struct FailOnce { + failed: bool, +} + +impl tokio::io::AsyncRead for FailOnce { + fn poll_read( + self: std::pin::Pin<&mut Self>, + _cx: &mut std::task::Context<'_>, + _buf: &mut tokio::io::ReadBuf<'_>, + ) -> std::task::Poll> { + if self.failed { + std::task::Poll::Ready(Ok(())) + } else { + self.get_mut().failed = true; + std::task::Poll::Ready(Err(std::io::Error::other("injected"))) + } + } +} + +#[tokio::test] +async fn ir_helper_async_drain_error() { + let mut reader = FailOnce { failed: false }; + let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16).await; + assert!(kept.is_empty()); + assert_eq!(trunc, 0); + assert!(err); +} + +/// Deterministic timer contract: a ready timer completes under deadline; a +/// pending future does not. Proves timeout_at behaves before trusting it +/// for kill/reap/drain bounds. No wall-clock flakiness: margins are 500x. +#[tokio::test] +async fn ir_helper_timeout_contract() { + use std::time::Duration; + let ok = tokio::time::timeout_at( + tokio::time::Instant::now() + Duration::from_secs(5), + tokio::time::sleep(Duration::from_millis(10)), + ) + .await; + assert!(ok.is_ok()); + let timed_out = tokio::time::timeout_at( + tokio::time::Instant::now() + Duration::from_millis(10), + std::future::pending::<()>(), + ) + .await; + assert!(timed_out.is_err()); +} From 3b439ad6f5737e1a7a4fab10a41646d2cf7cc41f Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 10:58:10 -0500 Subject: [PATCH 08/16] test(interpreter): concurrent sampler arm and ownership diagnostics Poll the RSS sampler alongside drains, exit, and deadline instead of awaiting it inline where it suspends everything. Pin one-shot futures once and poll by reference; taking them out destroys the only release path on the first lost race (proven by the fixed scheduling fixture). Document why per-tick recreation stays safe in production. TEST-ONLY tokio process feature; production features, versions, lockfile unchanged. --- .../input_redirect_budget_tests.rs | 183 +++++++++++++----- 1 file changed, 139 insertions(+), 44 deletions(-) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 289da904a..b848dfefa 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -615,12 +615,16 @@ async fn sample_child_rss(pid: u32) -> Result { .kill_on_drop(true) .spawn() .map_err(|e| format!("spawn ps for rss of {pid}: {e:?}"))?; - // Take the pipe upfront so the bounded drain below owns capture; the - // one-line output then cannot block or grow past its cap. + // Both pipes are taken upfront so the bounded drains below own capture; + // neither stream can block the sampler, and both are capped. let mut out_pipe = ps .stdout .take() .ok_or_else(|| format!("ps {pid} has no stdout"))?; + let mut err_pipe = ps + .stderr + .take() + .ok_or_else(|| format!("ps {pid} has no stderr"))?; let deadline = tokio::time::Instant::now() + Duration::from_secs(5); loop { match ps.try_wait() { @@ -629,8 +633,9 @@ async fn sample_child_rss(pid: u32) -> Result { if tokio::time::Instant::now() >= deadline { ps.start_kill() .map_err(|e| format!("kill stuck ps for rss of {pid}: {e:?}"))?; - // Finite reap with explicit receipt (not `let _`): either - // the kill is confirmed or the timeout is recorded. + // Finite reap with explicit receipt: timeout, kill + // failure, and success are three distinct outcomes, never + // conflated into one boolean. let reaped = tokio::time::timeout_at(deadline + Duration::from_secs(2), ps.wait()) .await @@ -642,27 +647,35 @@ async fn sample_child_rss(pid: u32) -> Result { Err(e) => return Err(format!("poll ps for rss of {pid}: {e:?}")), } } - // Reap explicitly (already exited: prompt), then bounded-drain the - // one-line output. Never wait_with_output without bounds. - match tokio::time::timeout_at( + // Reap explicitly (already exited: prompt) with all three outcomes + // distinguished, then bounded-drain both captures. + let status = match tokio::time::timeout_at( tokio::time::Instant::now() + Duration::from_secs(2), ps.wait(), ) .await { - Ok(Ok(_)) => {} + Ok(Ok(status)) => status, Ok(Err(e)) => return Err(format!("reap ps for rss of {pid}: {e:?}")), Err(_) => return Err(format!("reap ps for rss of {pid} timed out")), - } - let (kept, trunc, had_error) = ir_drain_capped_async(&mut out_pipe, 4096).await; - if had_error { + }; + let (out_kept, out_trunc, out_err) = ir_drain_capped_async(&mut out_pipe, 4096).await; + let (_err_kept, err_trunc, err_err) = ir_drain_capped_async(&mut err_pipe, 4096).await; + if out_err || err_err { return Err(format!("ps rss of {pid} output unreadable")); } - if trunc > 0 { + if out_trunc > 0 || err_trunc > 0 { return Err(format!("ps rss of {pid} output oversize")); } - parse_ps_rss(&String::from_utf8_lossy(&kept)) + if !status.success() { + return Err(format!("ps rss of {pid} failed")); + } + parse_ps_rss(&String::from_utf8_lossy(&out_kept)) } +/// One in-flight RSS sample: boxed so the select! arm can poll it +/// alongside drains, exit, and deadline without awaiting inline. +type SampleFuture = std::pin::Pin>>>; + async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); @@ -710,14 +723,12 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp let mut err_trunc = 0usize; let mut err_err = false; let mut incomplete_readers = false; - // Drain futures live for the whole supervision scope and are polled - // alongside exit, RSS, and deadline from the first iteration: output can - // never block the producer. No spawned tasks; dropping the futures on - // timeout is cancel-safe, and the pipes die with the reaped child. - // Admission stays capped: each drain keeps PIPE_CAP bytes and counts the - // rest without storing. let mut out_fut = std::pin::pin!(ir_drain_capped_async(&mut out_pipe, PIPE_CAP)); let mut err_fut = std::pin::pin!(ir_drain_capped_async(&mut err_pipe, PIPE_CAP)); + // Sampler slot: at most one RSS sample in flight. The ticker below only + // CREATES it; a dedicated select arm below POLLS it, so slow sampling + // never suspends drains, exit detection, or the wall deadline. + let mut sample_fut: Option = None; loop { let running_done = final_status.is_some() || overran || rss_exceeded; if running_done && out_done && err_done { @@ -746,28 +757,8 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp continue; } ticks += 1; - if ticks == 1 || ticks.is_multiple_of(10) { - match sample_child_rss(rss_pid).await { - Ok(rss) => { - rss_samples += 1; - peak_rss_kb = peak_rss_kb.max(rss); - if rss > IR_RSS_CAP_KIB { - child.start_kill().expect("kill RSS overrun child"); - rss_exceeded = true; - } - } - Err(e) => { - if let Some(status) = child.try_wait().expect("re-poll child") { - if rss_samples == 0 { - exited_fast = true; - } - final_status = Some(status); - reaped = true; - } else { - panic!("RSS policing failed on live child {rss_pid}: {e}"); - } - } - } + if (ticks == 1 || ticks.is_multiple_of(10)) && sample_fut.is_none() { + sample_fut = Some(Box::pin(sample_child_rss(rss_pid))); } if !rss_exceeded && final_status.is_none() && tokio::time::Instant::now() >= deadline { child.start_kill().expect("kill overrun child"); @@ -788,11 +779,42 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp err_err = err; err_done = true; } + // Take-then-poll is safe HERE (unlike one-shot-gated designs, see + // ir_helper_concurrent_schedule): each tick mints a fresh + // idempotent sampler, so losing a race only wastes one bounded ps + // run (kill_on_drop bounds it) and the ticker re-arms. The slot + // being None simply disables this arm until then. + res = async { sample_fut.take().expect("armed with sampler").await }, + if sample_fut.is_some() => + { + match res { + Ok(rss) => { + rss_samples += 1; + peak_rss_kb = peak_rss_kb.max(rss); + if rss > IR_RSS_CAP_KIB { + child.start_kill().expect("kill RSS overrun child"); + rss_exceeded = true; + } + } + Err(e) => { + if let Some(status) = child.try_wait().expect("re-poll child") { + if rss_samples == 0 { + exited_fast = true; + } + final_status = Some(status); + reaped = true; + } else { + panic!("RSS policing failed on live child {rss_pid}: {e}"); + } + } + } + } } } // Finite reap for killed paths (exited paths already reaped via wait): // Child::wait is cancel-safe; the timeout bounds it. start_kill was - // already sent on every non-exit path above. + // already sent on every non-exit path above. kill_on_drop stays armed + // as a backstop only, never proof. if final_status.is_none() { match tokio::time::timeout_at( tokio::time::Instant::now() + Duration::from_secs(5), @@ -810,8 +832,6 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp } } } - let elapsed = start.elapsed(); - let _ = elapsed; // Receipt first: the launcher note proves the caps ran; without it the // run is untrusted regardless of exit code. let stderr_full = String::from_utf8_lossy(&err_kept).into_owned(); @@ -1246,3 +1266,78 @@ async fn ir_helper_timeout_contract() { .await; assert!(timed_out.is_err()); } + +/// Scheduling contract: a release-gated sampler, two 32B duplex drains, and +/// a timer must ALL complete concurrently in one select! scope. The release +/// fires only after both drains finish, so awaiting the sampler first (as +/// the old ticker arm did) would stall at the gate until the outer bound; +/// polling everything together finishes in milliseconds. 64B total fixture +/// data, 2s outer bound, fully deterministic (no timing margins asserted, +/// only completion). +#[tokio::test] +async fn ir_helper_concurrent_schedule() { + use std::time::Duration; + let (tx, rx) = tokio::sync::oneshot::channel::(); + let (mut w1, mut r1) = tokio::io::duplex(64); + let (mut w2, mut r2) = tokio::io::duplex(64); + tokio::io::AsyncWriteExt::write_all(&mut w1, &[1u8; 32]) + .await + .expect("fill one"); + tokio::io::AsyncWriteExt::write_all(&mut w2, &[2u8; 32]) + .await + .expect("fill two"); + drop((w1, w2)); + // Pinned once and polled by &mut every round: taking it out would + // destroy the only release path on the first lost select race (that + // exact bug hung this fixture), so ownership stays in the slot. + let mut sampler = std::pin::pin!(async move { rx.await.expect("release") }); + let mut d1 = std::pin::pin!(ir_drain_capped_async(&mut r1, 64)); + let mut d2 = std::pin::pin!(ir_drain_capped_async(&mut r2, 64)); + let mut tx_opt = Some(tx); + let mut got = 0u32; + let mut k1 = Vec::new(); + let mut k2 = Vec::new(); + let mut sampler_done = false; + let mut d1_done = false; + let mut d2_done = false; + let deadline = tokio::time::Instant::now() + Duration::from_secs(2); + let finished = tokio::time::timeout_at(deadline, async { + loop { + eprintln!("tick s={} d1={} d2={}", sampler_done, d1_done, d2_done); + if sampler_done && d1_done && d2_done { + break; + } + tokio::select! { + biased; + v = &mut sampler, if !sampler_done => { + got = v; + sampler_done = true; + } + _ = tokio::time::sleep(Duration::from_millis(5)) => { + if d1_done && d2_done && let Some(tx) = tx_opt.take() { + let _ = tx.send(7); + } + } + r = &mut d1, if !d1_done => { + let (kept, trunc, err) = r; + k1 = kept; + assert_eq!(trunc, 0); + assert!(!err); + d1_done = true; + } + r = &mut d2, if !d2_done => { + let (kept, trunc, err) = r; + k2 = kept; + assert_eq!(trunc, 0); + assert!(!err); + d2_done = true; + } + } + } + }) + .await; + assert!(finished.is_ok(), "concurrent schedule must finish fast"); + assert_eq!(got, 7); + assert_eq!(k1, vec![1u8; 32]); + assert_eq!(k2, vec![2u8; 32]); +} From d609cb034814f67eb665958a8938205738d16673 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 14:31:14 -0500 Subject: [PATCH 09/16] test(input): preserve sampler ownership across select branches --- .../input_redirect_budget_tests.rs | 33 +++++++++---------- knowledge/operations/testing.md | 8 +++++ 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index b848dfefa..08c200f97 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -676,6 +676,11 @@ async fn sample_child_rss(pid: u32) -> Result { /// alongside drains, exit, and deadline without awaiting inline. type SampleFuture = std::pin::Pin>>>; +async fn poll_owned_sample(slot: &mut Option) -> Result { + // Losing a select branch must not destroy the pending sampler it borrows. + slot.as_mut().expect("armed with sampler").as_mut().await +} + async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); @@ -779,14 +784,10 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp err_err = err; err_done = true; } - // Take-then-poll is safe HERE (unlike one-shot-gated designs, see - // ir_helper_concurrent_schedule): each tick mints a fresh - // idempotent sampler, so losing a race only wastes one bounded ps - // run (kill_on_drop bounds it) and the ticker re-arms. The slot - // being None simply disables this arm until then. - res = async { sample_fut.take().expect("armed with sampler").await }, + res = poll_owned_sample(&mut sample_fut), if sample_fut.is_some() => { + sample_fut = None; match res { Ok(rss) => { rss_samples += 1; @@ -1269,9 +1270,8 @@ async fn ir_helper_timeout_contract() { /// Scheduling contract: a release-gated sampler, two 32B duplex drains, and /// a timer must ALL complete concurrently in one select! scope. The release -/// fires only after both drains finish, so awaiting the sampler first (as -/// the old ticker arm did) would stall at the gate until the outer bound; -/// polling everything together finishes in milliseconds. 64B total fixture +/// fires only after both drains finish. Poll the wrapper's actual owned-slot +/// helper so losing a selection cannot destroy that release path. 64B total fixture /// data, 2s outer bound, fully deterministic (no timing margins asserted, /// only completion). #[tokio::test] @@ -1287,14 +1287,13 @@ async fn ir_helper_concurrent_schedule() { .await .expect("fill two"); drop((w1, w2)); - // Pinned once and polled by &mut every round: taking it out would - // destroy the only release path on the first lost select race (that - // exact bug hung this fixture), so ownership stays in the slot. - let mut sampler = std::pin::pin!(async move { rx.await.expect("release") }); + let mut sampler: Option = Some(Box::pin(async move { + rx.await.map(u64::from).map_err(|error| error.to_string()) + })); let mut d1 = std::pin::pin!(ir_drain_capped_async(&mut r1, 64)); let mut d2 = std::pin::pin!(ir_drain_capped_async(&mut r2, 64)); let mut tx_opt = Some(tx); - let mut got = 0u32; + let mut got = 0u64; let mut k1 = Vec::new(); let mut k2 = Vec::new(); let mut sampler_done = false; @@ -1303,14 +1302,14 @@ async fn ir_helper_concurrent_schedule() { let deadline = tokio::time::Instant::now() + Duration::from_secs(2); let finished = tokio::time::timeout_at(deadline, async { loop { - eprintln!("tick s={} d1={} d2={}", sampler_done, d1_done, d2_done); if sampler_done && d1_done && d2_done { break; } tokio::select! { biased; - v = &mut sampler, if !sampler_done => { - got = v; + v = poll_owned_sample(&mut sampler), if sampler.is_some() => { + got = v.expect("sampler completed"); + sampler = None; sampler_done = true; } _ = tokio::time::sleep(Duration::from_millis(5)) => { diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 52cd23a9b..5231a51e2 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -249,6 +249,14 @@ surfaces as a 404 in an already-green job. ## Resource-cap tests must not race the deadline +The opt-in input-redirect helper polls its one in-flight RSS sampling future by +mutable reference across `select!` rounds. A losing branch must leave that future +in its owned slot; only observing its result clears the slot. The ordinary +`ir_helper_concurrent_schedule` fixture uses the wrapper's same +`poll_owned_sample` helper with 64 bytes of in-memory pipe data and a release-gated +future. This verifies scheduling ownership, not subprocess cleanup, effective +resource caps, platform coverage or merged security proof. + Controlled retained-pipe fixtures keep a caller-owned reader-handoff vector outside their cleanup guard. A bounded `Drop` returns any unfinished ORIGINAL handle to that owner rather than recording a count and detaching it. The owner From 0a858fd4a51e1632be158e1d78275183ffc7a548 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 15:21:33 -0500 Subject: [PATCH 10/16] test(input): bound child and sampler completion --- crates/bashkit/Cargo.toml | 8 +- .../input_redirect_budget_tests.rs | 675 ++++++++++++------ knowledge/operations/testing.md | 11 + 3 files changed, 484 insertions(+), 210 deletions(-) diff --git a/crates/bashkit/Cargo.toml b/crates/bashkit/Cargo.toml index e257e9a98..5185b7ad6 100644 --- a/crates/bashkit/Cargo.toml +++ b/crates/bashkit/Cargo.toml @@ -297,9 +297,6 @@ all-features = true rustdoc-args = ["--cfg", "docsrs"] [dev-dependencies] -# TEST-ONLY: process feature for the opt-in bounded child-test runner. -# Production builds never see this feature; versions and lock unchanged. -tokio = { workspace = true, features = ["process"] } tokio-test = { workspace = true } pretty_assertions = { workspace = true } insta = { workspace = true } @@ -453,3 +450,8 @@ wasm-bindgen-futures = { version = "0.4", optional = true } # not flag the vendored code. See knowledge/runtimes/coreutils-args-port.md. [lints.rust] unexpected_cfgs = { level = "warn", check-cfg = ['cfg(feature, values("i18n-decimal"))'] } + +# The process feature is native test infrastructure only; keep WASM builds +# on the workspace Tokio feature set without changing versions or the lock. +[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] +tokio = { workspace = true, features = ["process"] } diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 08c200f97..b0f1b4981 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -349,9 +349,12 @@ async fn stdin_clone_trip_recovers() { // Reuses reviewed primitives (`drain_capped`, `reap_with_grace`) with a // job-specific launcher. Shared defaults for other products are untouched. +#[cfg(not(target_arch = "wasm32"))] use crate::threat_model_tests::resource_exhaustion::bashkit_cli_path; +#[cfg(not(target_arch = "wasm32"))] use std::process::Stdio; use std::time::Duration; +#[cfg(not(target_arch = "wasm32"))] use tokio::process::Command; /// Address-space cap in KiB for the bash `ulimit -v` knob (Linux only). @@ -366,10 +369,13 @@ const IR_FILE_BLOCKS: u64 = 131_072; /// Active RSS cutoff in KiB (`ps -o rss=` reports KiB on Linux and macOS): /// catches runaway growth far below RAM/disk exhaustion while legit runs /// peak at O(MB). Policed by polling, not assumed from any cap. +#[cfg(not(target_arch = "wasm32"))] const IR_RSS_CAP_KIB: u64 = 1_048_576; // 1GiB /// Explicit allowance for one short instrumented run's coverage artifact. +#[cfg(not(target_arch = "wasm32"))] const IR_ARTIFACT_BOUND: u64 = 64 * 1024 * 1024; // 64MiB /// Per-stream kept bytes for concurrent diagnostic drains. +#[cfg(not(target_arch = "wasm32"))] const PIPE_CAP: usize = 256 * 1024; /// Opt-in launcher for the input-redirect job: shared CPU/stack caps plus an @@ -543,6 +549,7 @@ fn file_size_unit_factor(blocks: u64, bytes: u64) -> Result { /// Outcome of one opt-in bounded input-shape run: the reviewed verdict plus /// job-specific receipts. Setup faults panic with context (fail-closed); /// runtime observations are flags for the caller to assert on. +#[cfg(not(target_arch = "wasm32"))] struct InputShapeRun { exit_code: Option, stdout: String, @@ -550,6 +557,8 @@ struct InputShapeRun { timed_out: bool, oversized: bool, incomplete_readers: bool, + sampler_complete: bool, + lifecycle_error: Option, read_error: bool, reaped: bool, limits: IrLimitsNote, @@ -571,7 +580,17 @@ struct InputShapeRun { /// 1.53.2): dropping the future abandons only unread bytes, so a deadline /// timeout followed by kill+reap stays finite. Never collects unbounded /// `read_to_end`. -async fn ir_drain_capped_async(pipe: &mut R, cap: usize) -> (Vec, usize, bool) +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum StreamNotice { + Excess, + ReadError, +} + +async fn ir_drain_capped_async( + pipe: &mut R, + cap: usize, + mut notice: Option>, +) -> (Vec, usize, bool) where R: tokio::io::AsyncRead + Unpin, { @@ -584,12 +603,25 @@ where match pipe.read(&mut chunk).await { Ok(0) => break, Ok(n) => { - let room = cap.saturating_sub(total).min(n); + let room = cap.saturating_sub(kept.len()).min(n); kept.extend_from_slice(&chunk[..room]); - total += n; + total = total.saturating_add(n); + // One notice per stream; the shared channel has two slots. + // Notify before EOF while continuing bounded capture. + if total > cap + && let Some(sender) = notice.take() + && sender.try_send(StreamNotice::Excess).is_err() + { + had_error = true; + break; + } } Err(_) => { had_error = true; + if let Some(sender) = notice.take() { + // A closed receiver cannot turn a failed read into success. + let _ = sender.try_send(StreamNotice::ReadError); + } break; } } @@ -598,241 +630,455 @@ where (kept, total.saturating_sub(kept_len), had_error) } -/// Bounded RSS sampler on the same async machinery (no std `output()` or -/// `wait` escape): timeout, kill with finite reap, loud errors. Preserves -/// the effective pid receipt and distinguishes sampling failure from a -/// child that simply exited fast. -async fn sample_child_rss(pid: u32) -> Result { - use std::time::Duration; - let mut ps = tokio::process::Command::new("ps") - .arg("-o") - .arg("rss=") - .arg("-p") - .arg(pid.to_string()) - .stdin(std::process::Stdio::null()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .kill_on_drop(true) - .spawn() - .map_err(|e| format!("spawn ps for rss of {pid}: {e:?}"))?; - // Both pipes are taken upfront so the bounded drains below own capture; - // neither stream can block the sampler, and both are capped. - let mut out_pipe = ps - .stdout - .take() - .ok_or_else(|| format!("ps {pid} has no stdout"))?; - let mut err_pipe = ps - .stderr - .take() - .ok_or_else(|| format!("ps {pid} has no stderr"))?; - let deadline = tokio::time::Instant::now() + Duration::from_secs(5); - loop { - match ps.try_wait() { - Ok(Some(_)) => break, - Ok(None) => { - if tokio::time::Instant::now() >= deadline { - ps.start_kill() - .map_err(|e| format!("kill stuck ps for rss of {pid}: {e:?}"))?; - // Finite reap with explicit receipt: timeout, kill - // failure, and success are three distinct outcomes, never - // conflated into one boolean. - let reaped = - tokio::time::timeout_at(deadline + Duration::from_secs(2), ps.wait()) - .await - .is_ok(); - return Err(format!("ps rss of {pid} timed out (reaped: {reaped})")); - } - tokio::time::sleep(Duration::from_millis(50)).await; - } - Err(e) => return Err(format!("poll ps for rss of {pid}: {e:?}")), - } - } - // Reap explicitly (already exited: prompt) with all three outcomes - // distinguished, then bounded-drain both captures. - let status = match tokio::time::timeout_at( - tokio::time::Instant::now() + Duration::from_secs(2), - ps.wait(), - ) - .await - { - Ok(Ok(status)) => status, - Ok(Err(e)) => return Err(format!("reap ps for rss of {pid}: {e:?}")), - Err(_) => return Err(format!("reap ps for rss of {pid} timed out")), - }; - let (out_kept, out_trunc, out_err) = ir_drain_capped_async(&mut out_pipe, 4096).await; - let (_err_kept, err_trunc, err_err) = ir_drain_capped_async(&mut err_pipe, 4096).await; - if out_err || err_err { - return Err(format!("ps rss of {pid} output unreadable")); - } - if out_trunc > 0 || err_trunc > 0 { - return Err(format!("ps rss of {pid} output oversize")); - } - if !status.success() { - return Err(format!("ps rss of {pid} failed")); +#[derive(Debug)] +enum ReapOutcome { + Reaped(T), + Failed(std::io::Error), + TimedOut, +} + +async fn reap_before( + future: impl Future>, + deadline: tokio::time::Instant, +) -> ReapOutcome { + match tokio::time::timeout_at(deadline, future).await { + Ok(Ok(status)) => ReapOutcome::Reaped(status), + Ok(Err(error)) => ReapOutcome::Failed(error), + Err(_) => ReapOutcome::TimedOut, } - parse_ps_rss(&String::from_utf8_lossy(&out_kept)) } -/// One in-flight RSS sample: boxed so the select! arm can poll it -/// alongside drains, exit, and deadline without awaiting inline. -type SampleFuture = std::pin::Pin>>>; -async fn poll_owned_sample(slot: &mut Option) -> Result { - // Losing a select branch must not destroy the pending sampler it borrows. - slot.as_mut().expect("armed with sampler").as_mut().await +#[derive(Debug)] +struct SampleError { + message: String, + cleanup_complete: bool, } -async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { - let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); - let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); - let profile = sink.path().join("child.profraw"); - let cli = bashkit_cli_path(); - let mut cmd = Command::new("bash"); - cmd.arg("-c") - .arg(input_shape_launcher()) - .arg("ir-child") - .arg(&cli) - .arg("-c") - .arg(script) - .env("LLVM_PROFILE_FILE", &profile) - .current_dir(work.path()) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - // Backstop only: SIGKILL on drop if the explicit kill+reap below - // never runs. Not proof of anything; receipts are. - .kill_on_drop(true); - let mut child = OwnedChild::new(cmd.spawn().expect("spawn bounded child")); - let rss_pid = child.id(); - // Pipes are taken before the first poll so the concurrent drains below - // own them outright; the owner guard keeps pid and lifecycle. - let (out_pipe, err_pipe) = child.take_pipes(); - let mut out_pipe = out_pipe.expect("take child stdout"); - let mut err_pipe = err_pipe.expect("take child stderr"); - let start = tokio::time::Instant::now(); - let deadline = start + Duration::from_secs(wall_secs); - let join_deadline = start + Duration::from_secs(wall_secs + 15); - let mut ticks = 0u64; - let mut rss_samples = 0u32; - let mut peak_rss_kb = 0u64; - let mut rss_exceeded = false; - let mut exited_fast = false; - let mut overran = false; - let mut final_status: Option = None; - let mut reaped = false; +/// One lifecycle owns wait, captures and the optional RSS sampler. Errors +/// request kill once, then retain all futures through finite completion. +/// Drop remains a best-effort backstop, never a completion receipt. +#[cfg(not(target_arch = "wasm32"))] +struct ChildCompletion { + status: Option, + out: (Vec, usize, bool), + err: (Vec, usize, bool), + reaped: bool, + readers_complete: bool, + sampler_complete: bool, + timed_out: bool, + failure: Option, + rss_samples: u32, + peak_rss_kb: u64, + rss_exceeded: bool, + exited_fast: bool, +} + +#[cfg(not(target_arch = "wasm32"))] +async fn complete_owned_child( + child: &mut OwnedChild, + cap: usize, + deadline: tokio::time::Instant, + sample_pid: Option, + grace: Duration, +) -> ChildCompletion { + let (mut out_pipe, mut err_pipe) = child.take_pipes(); + let missing_pipe = out_pipe.is_none() || err_pipe.is_none(); + let (notice_tx, mut notices) = tokio::sync::mpsc::channel(2); + let err_notice = notice_tx.clone(); + let mut out_fut = std::pin::pin!(async { + match out_pipe.as_mut() { + Some(pipe) => ir_drain_capped_async(pipe, cap, Some(notice_tx)).await, + None => (Vec::new(), 0, true), + } + }); + let mut err_fut = std::pin::pin!(async { + match err_pipe.as_mut() { + Some(pipe) => ir_drain_capped_async(pipe, cap, Some(err_notice)).await, + None => (Vec::new(), 0, true), + } + }); + let mut completion = ChildCompletion { + status: None, + out: (Vec::new(), 0, false), + err: (Vec::new(), 0, false), + reaped: false, + readers_complete: false, + sampler_complete: false, + timed_out: false, + failure: missing_pipe.then(|| "child capture pipe missing".into()), + rss_samples: 0, + peak_rss_kb: 0, + rss_exceeded: false, + exited_fast: false, + }; + let mut stop_requested = missing_pipe; + let mut kill_sent = false; + let mut wait_failed = false; let mut out_done = false; let mut err_done = false; - let mut out_kept = Vec::new(); - let mut out_trunc = 0usize; - let mut out_err = false; - let mut err_kept = Vec::new(); - let mut err_trunc = 0usize; - let mut err_err = false; - let mut incomplete_readers = false; - let mut out_fut = std::pin::pin!(ir_drain_capped_async(&mut out_pipe, PIPE_CAP)); - let mut err_fut = std::pin::pin!(ir_drain_capped_async(&mut err_pipe, PIPE_CAP)); - // Sampler slot: at most one RSS sample in flight. The ticker below only - // CREATES it; a dedicated select arm below POLLS it, so slow sampling - // never suspends drains, exit detection, or the wall deadline. + let mut notices_open = true; let mut sample_fut: Option = None; + let mut ticker = tokio::time::interval_at( + tokio::time::Instant::now() + Duration::from_millis(50), + Duration::from_millis(500), + ); + ticker.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + let finish_deadline = deadline + grace; + let mut sampler_cleanup_complete = true; loop { - let running_done = final_status.is_some() || overran || rss_exceeded; - if running_done && out_done && err_done { - break; + if stop_requested && !kill_sent && completion.status.is_none() { + kill_sent = true; + if let Err(error) = child.start_kill() { + completion + .failure + .get_or_insert_with(|| format!("kill child: {error}")); + } } - if running_done && tokio::time::Instant::now() >= join_deadline { - incomplete_readers = true; + if (completion.status.is_some() || wait_failed) + && out_done + && err_done + && sample_fut.is_none() + { break; } tokio::select! { biased; - status = child.wait(), if !running_done => { - match status { - Ok(s) => { - if rss_samples == 0 { - exited_fast = true; - } - final_status = Some(s); - reaped = true; - } - Err(e) => panic!("wait child {rss_pid} failed: {e:?}"), + _ = tokio::time::sleep_until(finish_deadline) => { + completion.failure.get_or_insert_with(|| "child completion timed out".into()); + // Send before leaving the owner loop, including a failed wait. + if !kill_sent && completion.status.is_none() + && let Err(error) = child.start_kill() + { + completion.failure.get_or_insert_with(|| format!("kill child: {error}")); } + break; } - _ = tokio::time::sleep(Duration::from_millis(50)) => { - if running_done { - continue; - } - ticks += 1; - if (ticks == 1 || ticks.is_multiple_of(10)) && sample_fut.is_none() { - sample_fut = Some(Box::pin(sample_child_rss(rss_pid))); + _ = tokio::time::sleep_until(deadline), + if !stop_requested && completion.status.is_none() => + { + completion.timed_out = true; + stop_requested = true; + } + result = child.wait(), if completion.status.is_none() && !wait_failed => { + match result { + Ok(status) => { + completion.status = Some(status); + completion.reaped = true; + completion.exited_fast = completion.rss_samples == 0; + } + Err(error) => { + completion.failure = Some(format!("wait child: {error}")); + wait_failed = true; + stop_requested = true; + } } - if !rss_exceeded && final_status.is_none() && tokio::time::Instant::now() >= deadline { - child.start_kill().expect("kill overrun child"); - overran = true; + } + notice = notices.recv(), if notices_open => { + match notice { + Some(notice) => { + completion.failure.get_or_insert_with(|| format!("child stream: {notice:?}")); + stop_requested = true; + } + None => notices_open = false, } } out = &mut out_fut, if !out_done => { - let (kept, trunc, err) = out; - out_kept = kept; - out_trunc = trunc; - out_err = err; + if out.1 > 0 || out.2 { + completion.failure.get_or_insert_with(|| "stdout capture failed".into()); + stop_requested = true; + } + completion.out = out; out_done = true; } err = &mut err_fut, if !err_done => { - let (kept, trunc, err) = err; - err_kept = kept; - err_trunc = trunc; - err_err = err; + if err.1 > 0 || err.2 { + completion.failure.get_or_insert_with(|| "stderr capture failed".into()); + stop_requested = true; + } + completion.err = err; err_done = true; } - res = poll_owned_sample(&mut sample_fut), - if sample_fut.is_some() => - { + res = poll_owned_sample(&mut sample_fut), if sample_fut.is_some() => { sample_fut = None; match res { Ok(rss) => { - rss_samples += 1; - peak_rss_kb = peak_rss_kb.max(rss); + completion.rss_samples += 1; + completion.peak_rss_kb = completion.peak_rss_kb.max(rss); if rss > IR_RSS_CAP_KIB { - child.start_kill().expect("kill RSS overrun child"); - rss_exceeded = true; + completion.rss_exceeded = true; + stop_requested = true; } } - Err(e) => { - if let Some(status) = child.try_wait().expect("re-poll child") { - if rss_samples == 0 { - exited_fast = true; + Err(error) => { + if !error.cleanup_complete { + sampler_cleanup_complete = false; + completion.failure = Some(error.message); + stop_requested = true; + } else if completion.status.is_none() { + match child.try_wait() { + Ok(Some(status)) => { + completion.status = Some(status); + completion.reaped = true; + completion.exited_fast = completion.rss_samples == 0; + } + Ok(None) => { + completion.failure = Some(error.message); + stop_requested = true; + } + Err(poll_error) => { + completion.failure = Some(format!("poll child: {poll_error}; {}", error.message)); + stop_requested = true; + wait_failed = true; + } } - final_status = Some(status); - reaped = true; - } else { - panic!("RSS policing failed on live child {rss_pid}: {e}"); } } } } + _ = ticker.tick(), + if sample_pid.is_some() && !stop_requested + && completion.status.is_none() && sample_fut.is_none() => + { + sample_fut = Some(Box::pin(sample_child_rss(sample_pid.expect("sampling enabled")))); + } + } + } + completion.readers_complete = out_done && err_done; + match finish_owned_sample(&mut sample_fut, finish_deadline).await { + Ok(Some(rss)) => { + completion.rss_samples += 1; + completion.peak_rss_kb = completion.peak_rss_kb.max(rss); + completion.rss_exceeded |= rss > IR_RSS_CAP_KIB; + } + Ok(None) => {} + Err(error) => { + sampler_cleanup_complete &= error.cleanup_complete; + if !error.cleanup_complete { + completion.failure.get_or_insert(error.message); + } } } - // Finite reap for killed paths (exited paths already reaped via wait): - // Child::wait is cancel-safe; the timeout bounds it. start_kill was - // already sent on every non-exit path above. kill_on_drop stays armed - // as a backstop only, never proof. - if final_status.is_none() { - match tokio::time::timeout_at( - tokio::time::Instant::now() + Duration::from_secs(5), + completion.sampler_complete = sample_fut.is_none() && sampler_cleanup_complete; + if completion.status.is_none() { + match reap_before( child.wait(), + tokio::time::Instant::now() + grace.min(Duration::from_secs(5)), ) .await { - Ok(Ok(status)) => { - final_status = Some(status); - reaped = true; + ReapOutcome::Reaped(status) => { + completion.status = Some(status); + completion.reaped = true; } - Ok(Err(e)) => panic!("reap child {rss_pid} failed: {e:?}"), - Err(_) => { - reaped = false; + ReapOutcome::Failed(error) => { + completion + .failure + .get_or_insert_with(|| format!("reap child: {error}")); } + ReapOutcome::TimedOut => { + completion + .failure + .get_or_insert_with(|| "reap child timed out".into()); + } + } + } + completion +} + +/// RSS sampling uses the same finite owned lifecycle as the main child. +/// Sampling failure and incomplete reap/capture are distinct verdicts. +#[cfg(not(target_arch = "wasm32"))] +async fn sample_child_rss(pid: u32) -> Result { + let ps = Command::new("ps") + .arg("-o") + .arg("rss=") + .arg("-p") + .arg(pid.to_string()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true) + .spawn() + .map_err(|error| SampleError { + message: format!("spawn ps for rss of {pid}: {error}"), + cleanup_complete: true, // Spawn failed; no child exists. + })?; + let mut ps = OwnedChild::new(ps); + let capture = complete_owned_child( + &mut ps, + 4096, + tokio::time::Instant::now() + Duration::from_secs(5), + None, + Duration::from_secs(2), + ) + .await; + let cleanup_complete = capture.reaped && capture.readers_complete && capture.sampler_complete; + if !cleanup_complete + || capture.timed_out + || capture.failure.is_some() + || !capture.status.is_some_and(|status| status.success()) + { + return Err(SampleError { + message: format!( + "ps rss of {pid} failed: {} (reaped={}, captures={})", + capture.failure.as_deref().unwrap_or("unsuccessful sample"), + capture.reaped, + capture.readers_complete + ), + cleanup_complete, + }); + } + parse_ps_rss(&String::from_utf8_lossy(&capture.out.0)).map_err(|message| SampleError { + message, + cleanup_complete, + }) +} +type SampleFuture = std::pin::Pin>>>; + +async fn poll_owned_sample(slot: &mut Option) -> Result { + // Losing a select branch must not destroy the pending sampler it borrows. + slot.as_mut().expect("armed with sampler").as_mut().await +} + +async fn finish_owned_sample( + slot: &mut Option, + deadline: tokio::time::Instant, +) -> Result, SampleError> { + if slot.is_none() { + return Ok(None); + } + match tokio::time::timeout_at(deadline, poll_owned_sample(slot)).await { + Ok(result) => { + *slot = None; + result.map(Some) } + Err(_) => Err(SampleError { + message: "owned sampler completion timed out".into(), + cleanup_complete: false, + }), + } +} + +#[tokio::test] +async fn ir_helper_reap_outcomes() { + let deadline = tokio::time::Instant::now() + Duration::from_secs(1); + assert!(matches!( + reap_before(std::future::ready(Ok(7_u8)), deadline).await, + ReapOutcome::Reaped(7) + )); + match reap_before( + std::future::ready(Err::(std::io::Error::other("injected"))), + deadline, + ) + .await + { + ReapOutcome::Failed(error) => assert_eq!(error.kind(), std::io::ErrorKind::Other), + outcome => panic!("failed wait must remain an error: {outcome:?}"), } + assert!(matches!( + reap_before( + std::future::pending::>(), + tokio::time::Instant::now() + ) + .await, + ReapOutcome::TimedOut + )); +} + +#[tokio::test] +async fn ir_helper_live_excess_notice() { + let (mut writer, mut reader) = tokio::io::duplex(64); + tokio::io::AsyncWriteExt::write_all(&mut writer, &[1_u8; 32]) + .await + .unwrap(); + let (tx, mut rx) = tokio::sync::mpsc::channel(2); + let mut drain = std::pin::pin!(ir_drain_capped_async(&mut reader, 16, Some(tx))); + let notice = tokio::time::timeout(Duration::from_secs(1), async { + tokio::select! { + notice = rx.recv() => notice, + _ = &mut drain => panic!("writer is open; capture must remain pending"), + } + }) + .await + .unwrap(); + assert_eq!(notice, Some(StreamNotice::Excess)); + drop(writer); + let (kept, truncated, error) = drain.await; + assert_eq!(kept, vec![1_u8; 16]); + assert_eq!(truncated, 16); + assert!(!error); +} + +#[tokio::test] +async fn ir_helper_live_error_notice() { + let (tx, mut rx) = tokio::sync::mpsc::channel(2); + let mut reader = FailOnce { failed: false }; + let (notice, (kept, truncated, error)) = + tokio::join!(rx.recv(), ir_drain_capped_async(&mut reader, 16, Some(tx)),); + assert_eq!(notice, Some(StreamNotice::ReadError)); + assert!(kept.is_empty() && truncated == 0 && error); +} + +#[tokio::test] +async fn ir_helper_terminal_sampler_completion() { + let (tx, rx) = tokio::sync::oneshot::channel::(); + let mut slot: Option = Some(Box::pin(async move { + rx.await.map_err(|error| SampleError { + message: error.to_string(), + cleanup_complete: true, + }) + })); + let deadline = tokio::time::Instant::now() + Duration::from_secs(1); + let (result, ()) = tokio::join!(finish_owned_sample(&mut slot, deadline), async { + tx.send(7).unwrap(); + },); + assert_eq!(result.unwrap(), Some(7)); + assert!(slot.is_none()); + slot = Some(Box::pin(std::future::pending())); + let error = finish_owned_sample(&mut slot, tokio::time::Instant::now()) + .await + .unwrap_err(); + assert!(!error.cleanup_complete); + assert_eq!(error.message, "owned sampler completion timed out"); + assert!( + slot.is_some(), + "timeout cannot claim completion or discard the owned future" + ); +} + +#[cfg(not(target_arch = "wasm32"))] +async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> InputShapeRun { + let sink = TempWorkdir::create(&format!("{tag}-sink")).expect("artifact sink setup"); + let work = TempWorkdir::create(&format!("{tag}-work")).expect("producer workdir setup"); + let profile = sink.path().join("child.profraw"); + let cli = bashkit_cli_path(); + let mut cmd = Command::new("bash"); + cmd.arg("-c") + .arg(input_shape_launcher()) + .arg("ir-child") + .arg(&cli) + .arg("-c") + .arg(script) + .env("LLVM_PROFILE_FILE", &profile) + .current_dir(work.path()) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + // Backstop only: SIGKILL on drop if the explicit kill+reap below + // never runs. Not proof of anything; receipts are. + .kill_on_drop(true); + let mut child = OwnedChild::new(cmd.spawn().expect("spawn bounded child")); + let rss_pid = child.id(); + let completion = complete_owned_child( + &mut child, + PIPE_CAP, + tokio::time::Instant::now() + Duration::from_secs(wall_secs), + Some(rss_pid), + Duration::from_secs(15), + ) + .await; + let (out_kept, out_trunc, out_err) = completion.out; + let (err_kept, err_trunc, err_err) = completion.err; // Receipt first: the launcher note proves the caps ran; without it the // run is untrusted regardless of exit code. let stderr_full = String::from_utf8_lossy(&err_kept).into_owned(); @@ -868,23 +1114,25 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp sink.cleanup().expect("artifact sink cleanup"); work.cleanup().expect("work dir cleanup"); InputShapeRun { - exit_code: final_status.and_then(|s| s.code()), + exit_code: completion.status.and_then(|s| s.code()), stdout: String::from_utf8_lossy(&out_kept).into_owned(), stderr, - timed_out: overran, + timed_out: completion.timed_out, oversized: out_trunc > 0 || err_trunc > 0, - incomplete_readers, + incomplete_readers: !completion.readers_complete, + sampler_complete: completion.sampler_complete, + lifecycle_error: completion.failure, read_error: out_err || err_err, - reaped, + reaped: completion.reaped, limits, artifact, leftovers, file_unit, rss_pid, - rss_samples, - peak_rss_kb, - rss_exceeded, - exited_fast, + rss_samples: completion.rss_samples, + peak_rss_kb: completion.peak_rss_kb, + rss_exceeded: completion.rss_exceeded, + exited_fast: completion.exited_fast, } } @@ -897,7 +1145,14 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp /// (fail-closed on any unit surprise); RSS policing receipts (a real pid, /// no exceedance, peak under the cutoff, samples taken unless the child /// exited before the first poll). +#[cfg(not(target_arch = "wasm32"))] fn assert_effective_bounds(r: &InputShapeRun) { + assert!(r.sampler_complete, "owned sampler completion unknown"); + assert!( + r.lifecycle_error.is_none(), + "child lifecycle failed: {:?}", + r.lifecycle_error + ); let limits = &r.limits; assert_eq!(limits.stack_raw, "4096", "stack cap round-trip"); assert_eq!(limits.cpu_raw, "10", "cpu cap round-trip"); @@ -953,7 +1208,7 @@ fn assert_effective_bounds(r: &InputShapeRun) { /// (no exit code) and from pre-fix success (exit 0). /// NOT EXECUTED in this environment (hostile-fixture ban); kept as the /// blocked-verification target with full harness assertions intact. -#[cfg(unix)] +#[cfg(all(unix, not(target_arch = "wasm32")))] #[tokio::test] async fn original_shape_bounded_child_default_limits() { let r = @@ -1072,7 +1327,7 @@ fn ir_helper_workdir_drop_cleans() { /// Finite normal child completion through the full opt-in wrapper with an /// ordinary harmless script: proves the harness itself works end to end. /// NOT vulnerability proof: `true` exercises no budget path. -#[cfg(unix)] +#[cfg(all(unix, not(target_arch = "wasm32")))] #[tokio::test] async fn ir_helper_normal_child_completes() { let r = run_input_shape_bounded("true", 30, "ir-true").await; @@ -1143,10 +1398,12 @@ fn ir_helper_ps_parse_cases() { /// explicitly with a finite deadline and then drop the guard; Drop sends /// SIGKILL best-effort via sync `start_kill` (documented backstop, never the /// primary path) but never waits, so Drop cannot block. +#[cfg(not(target_arch = "wasm32"))] struct OwnedChild { inner: Option, } +#[cfg(not(target_arch = "wasm32"))] impl OwnedChild { fn new(child: tokio::process::Child) -> Self { Self { inner: Some(child) } @@ -1193,6 +1450,7 @@ impl OwnedChild { } } +#[cfg(not(target_arch = "wasm32"))] impl Drop for OwnedChild { fn drop(&mut self) { if let Some(child) = self.inner.as_mut() { @@ -1212,7 +1470,7 @@ async fn ir_helper_async_drain_bounded() { .await .expect("fill duplex"); drop(writer); - let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16).await; + let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16, None).await; assert_eq!(kept.len(), 16); assert_eq!(trunc, 84); assert!(!err); @@ -1242,7 +1500,7 @@ impl tokio::io::AsyncRead for FailOnce { #[tokio::test] async fn ir_helper_async_drain_error() { let mut reader = FailOnce { failed: false }; - let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16).await; + let (kept, trunc, err) = ir_drain_capped_async(&mut reader, 16, None).await; assert!(kept.is_empty()); assert_eq!(trunc, 0); assert!(err); @@ -1288,10 +1546,13 @@ async fn ir_helper_concurrent_schedule() { .expect("fill two"); drop((w1, w2)); let mut sampler: Option = Some(Box::pin(async move { - rx.await.map(u64::from).map_err(|error| error.to_string()) + rx.await.map(u64::from).map_err(|error| SampleError { + message: error.to_string(), + cleanup_complete: true, + }) })); - let mut d1 = std::pin::pin!(ir_drain_capped_async(&mut r1, 64)); - let mut d2 = std::pin::pin!(ir_drain_capped_async(&mut r2, 64)); + let mut d1 = std::pin::pin!(ir_drain_capped_async(&mut r1, 64, None)); + let mut d2 = std::pin::pin!(ir_drain_capped_async(&mut r2, 64, None)); let mut tx_opt = Some(tx); let mut got = 0u64; let mut k1 = Vec::new(); diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 5231a51e2..dd39aad23 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -257,6 +257,17 @@ in its owned slot; only observing its result clears the slot. The ordinary future. This verifies scheduling ownership, not subprocess cleanup, effective resource caps, platform coverage or merged security proof. +The native input-redirect runner shares one owned lifecycle for the main child +and RSS sampler: wait, both capped captures and live first-failure notices are +polled together. Ordinary errors request kill before finite reap/capture completion; +completion waits for an owned pending sample and records incomplete cleanup as a +negative verdict. Reap timeout, I/O failure and success are separate outcomes. +The process feature is a native-only Tokio dev-dependency; WASM builds keep the +workspace feature set. Drop remains a best-effort backstop, not evidence of +reap or capture completion on panic or external cancellation. Tiny in-memory +fixtures verify notice delivery before EOF, nested reap failures and pending-sample +ownership. They do not establish subprocess, Linux or full security shipping proof. + Controlled retained-pipe fixtures keep a caller-owned reader-handoff vector outside their cleanup guard. A bounded `Drop` returns any unfinished ORIGINAL handle to that owner rather than recording a count and detaching it. The owner From a6534ff9f39a11f189d9b7b542cefb114c856898 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 14 Oct 2026 19:45:51 -0500 Subject: [PATCH 11/16] fix(input): admit materialized input storage before growth --- crates/bashkit/src/interpreter/mod.rs | 14 +- crates/bashkit/src/interpreter/redirection.rs | 383 +++++++++++++++--- crates/bashkit/src/stream.rs | 72 ++++ .../input_redirect_budget_tests.rs | 55 +++ docs/security.md | 9 + knowledge/foundations/architecture.md | 13 + knowledge/operations/testing.md | 13 + knowledge/security/threat-model.md | 10 + 8 files changed, 496 insertions(+), 73 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 91a3fd86b..c0a013293 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -9437,8 +9437,8 @@ impl Interpreter { } Err(e) => return Err(e), }; - // Hold the stdin clone's live lease (if stdin is a budgeted - // clone minted above) for as long as `stdin` is retained below. + // Keep the materialized stdin's byte/text storage admitted + // for as long as the command retains it. let _stdin_storage = stdin_setup.stdin_lease; let stdin = stdin_setup.stdin; @@ -12925,8 +12925,7 @@ impl Interpreter { } Err(e) => return Err(e), }; - // Hold the stdin clone's live lease (if stdin is a budgeted - // clone minted above) for as long as `stdin` is retained below. + // Keep stdin's byte/text storage admitted through the body. let _stdin_storage = stdin_setup.stdin_lease; let stdin = stdin_setup.stdin; let prev_pipeline_stdin = if stdin.is_some() { @@ -13026,12 +13025,11 @@ impl Interpreter { }; match read { Ok(setup) => { - // Hold the stdin clone's live lease while the - // substitution output derived from it is live. + // Retain admitted input storage through substitution. let _stdin_storage = setup.stdin_lease; if let Some(content) = setup.stdin { - // Account for the VFS-owned input while it and the - // decoded substitution text are simultaneously live. + // Cover the temporary byte copy made by the + // command-substitution text boundary. let _content_lease = self.execution_budget.lease_bytes(content.len())?; stdout.try_push_str(&content.command_substitution_text())?; diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 46b4ca424..64d926ae0 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -56,43 +56,34 @@ fn redirect_target_may_split(redirect: &Redirect) -> bool { }) } -/// Stdin slot: the buffer plus the live-bytes lease when the buffer is a -/// budgeted clone minted here. Assigning the slot drops any previous -/// buffer and its clone lease together. -type StdinSlot = Option<( - crate::StreamData, - Option, -)>; +// Read admission, any excess backend capacity, and cached text travel together. +type InputStorage = [Option; 3]; +type StdinSlot = Option<(crate::StreamData, InputStorage)>; /// Assign the stdin slot, dropping any previous buffer and its clone lease -/// together. Fresh reads pass `lease: None`; budgeted clones pass the lease -/// from `charge_input_retention` so it lives as long as stdin does. -fn set_stdin_slot( - slot: &mut StdinSlot, - value: Option, - lease: Option, -) { +/// together, including fresh materializations as well as copies. +fn set_stdin_slot(slot: &mut StdinSlot, value: Option, lease: InputStorage) { *slot = value.map(|data| (data, lease)); } /// Result of input-redirection processing: the stdin buffer plus the -/// live-bytes lease for it when stdin is a budgeted clone minted here. +/// live-bytes leases for both representations. /// Callers hold `stdin_lease` for as long as they retain `stdin`. pub(crate) struct StdinSetup { pub stdin: Option, - pub stdin_lease: Option, + pub stdin_lease: InputStorage, } /// Numbered-fd table with replace semantics: re-redirecting an fd drops the /// superseded buffer — and its live-bytes lease — instead of piling clones. /// Aggregate input consumption stays cumulative. -type OwnFdEntry = (i32, crate::StreamData, crate::limits::ExecutionBudgetLease); +type OwnFdEntry = (i32, crate::StreamData, InputStorage); fn replace_own_fd( own_fds: &mut Vec, fd: i32, data: crate::StreamData, - lease: crate::limits::ExecutionBudgetLease, + lease: InputStorage, ) { if let Some(slot) = own_fds.iter_mut().find(|(existing, _, _)| *existing == fd) { *slot = (fd, data, lease); @@ -274,17 +265,83 @@ impl Interpreter { self.diag(format!("{path}: {reason}\n")) } - /// Charging helper for numbered input redirects (input-redirect budget fix). Every - /// retained buffer counts against the aggregate input budget and holds - /// a live-bytes lease while the redirect list is processed. Charges run - /// before the clone/allocation they cover; re-redirecting the same fd - /// replaces the superseded entry instead of piling clones. - fn charge_input_retention( + /// Admit a backend read before loading and retain its capacity through decoding. + // THREAT[TM-DOS-103]: metadata and device ceilings precede VFS allocation. + async fn read_input_stream( &self, - len: usize, - ) -> crate::error::Result { - self.execution_budget.consume_input(len)?; - Ok(self.execution_budget.lease_bytes(len)?) + path: &Path, + create: bool, + ) -> Result<(crate::StreamData, InputStorage)> { + let budget = &self.execution_budget; + let source = match budget.run(crate::fs::canonicalize(&*self.fs, path)).await? { + Ok(source) => source, + Err(crate::Error::Io(error)) + if create && error.kind() == std::io::ErrorKind::NotFound => + { + path.to_path_buf() + } + Err(error) => return Err(error), + }; + let size = match budget.run(self.fs.stat(&source)).await? { + Ok(metadata) => metadata.size, + Err(crate::Error::Io(error)) + if create && error.kind() == std::io::ErrorKind::NotFound => + { + 0 + } + Err(error) => return Err(error), + }; + let reported = usize::try_from(size) + .map_err(|_| LimitExceeded::Memory("input redirection size overflow".into()))?; + let device = match source.to_str() { + // InMemoryFs devices report zero size but materialize these bounded reads. + Some("/dev/zero") => 1 << 20, + Some("/dev/random" | "/dev/urandom") => 8192, + _ => 0, + }; + let size = reported.max(device); + budget.consume_input(size)?; + budget.consume_work(1 + size.div_ceil(1024) as u64)?; + let raw = budget.lease_bytes(size)?; + if create { + // An empty append may load an existing lazy file, so admission + // must precede opening as well as the subsequent read. + budget.run(self.fs.append_file(&source, b"")).await??; + } + let bytes = budget.run(self.fs.read_file(&source)).await??; + budget.consume_input(bytes.len().saturating_sub(size))?; + let extra = budget.lease_bytes(bytes.capacity().saturating_sub(size))?; + let (stream, text) = crate::StreamData::from_admitted_bytes(bytes, budget)?; + Ok((stream, [Some(raw), Some(extra), text])) + } + + fn input_stream_from_text( + &self, + content: String, + newline: bool, + ) -> Result<(crate::StreamData, InputStorage)> { + let budget = &self.execution_budget; + let mut text = crate::limits::BudgetedString::try_from_string(content, Some(budget))?; + if newline { + text.try_push('\n')?; + } + budget.consume_input( + text.len() + .checked_mul(2) + .ok_or_else(|| LimitExceeded::Memory("input redirection size overflow".into()))?, + )?; + let (stream, [bytes, text]) = crate::StreamData::from_budgeted_string(text, Some(budget))?; + Ok((stream, [bytes, text, None])) + } + + fn clone_input_stream( + &self, + content: &crate::StreamData, + ) -> Result<(crate::StreamData, InputStorage)> { + self.execution_budget + .consume_input(content.retained_len())?; + let (stream, [bytes, text]) = content.try_clone_budgeted(&self.execution_budget)?; + Ok((stream, [bytes, text, None])) } /// Process input redirections (< file, <<< string). `demand` is how @@ -299,7 +356,7 @@ impl Interpreter { // Stdin slot: a budgeted clone's live lease travels with the buffer // so replacing stdin drops both together; the surviving lease is // handed to the caller with stdin. - let mut stdin: StdinSlot = existing_stdin.map(|data| (data, None)); + let mut stdin: StdinSlot = existing_stdin.map(|data| (data, [None, None, None])); // Readable fds this command's own redirects open (`4< = Vec::new(); @@ -311,19 +368,19 @@ impl Interpreter { // `<>`: open (creating) the file read-write. let target_path = self.expand_word(&redirect.target).await?; let path = self.resolve_path(&target_path); - let content = if is_dev_null(&path) { - crate::StreamData::new() + let (content, storage) = if is_dev_null(&path) { + (crate::StreamData::new(), [None, None, None]) } else if !self.shell_features.has_file_redirects() { return Err(crate::error::Error::Execution( self.diag(format!("{}: filesystem redirection disabled", target_path)), )); } else { - let opened = match self.fs.append_file(&path, b"").await { - Ok(()) => self.fs.read_file(&path).await, - Err(e) => Err(e), - }; + let opened = self.read_input_stream(&path, true).await; match opened { - Ok(content) => content.into(), + Ok(content) => content, + Err(e @ (crate::Error::ResourceLimit(_) | crate::Error::Cancelled)) => { + return Err(e); + } Err(e) => { return Err(crate::error::Error::CommandFailure( self.diag(format!("{target_path}: {}\n", io_error_reason(&e))), @@ -333,10 +390,9 @@ impl Interpreter { }; match high_fd { Some(fd) => { - let lease = self.charge_input_retention(content.retained_len())?; - replace_own_fd(&mut own_fds, fd, content, lease); + replace_own_fd(&mut own_fds, fd, content, storage); } - None => set_stdin_slot(&mut stdin, Some(content), None), + None => set_stdin_slot(&mut stdin, Some(content), storage), } } RedirectKind::Input => { @@ -354,20 +410,18 @@ impl Interpreter { if self.fd_alias(&path) == Some(0) { // `< /dev/stdin` re-reads the current stdin: no-op. } else if is_dev_null(&path) { - set_stdin_slot(&mut stdin, Some(crate::StreamData::new()), None); // EOF + set_stdin_slot( + &mut stdin, + Some(crate::StreamData::new()), + [None, None, None], + ); // EOF } else if !self.shell_features.has_file_redirects() { return Err(crate::error::Error::Execution( self.diag(format!("{}: filesystem redirection disabled", target_path)), )); } else { - match self.fs.read_file(&path).await { - Ok(content) => { - let content: crate::StreamData = content.into(); - if let Some(fd) = high_fd { - let lease = - self.charge_input_retention(content.retained_len())?; - replace_own_fd(&mut own_fds, fd, content.clone(), lease); - } + match self.read_input_stream(&path, false).await { + Ok((content, storage)) => { // WTF: on a compound `3 fd == u, (Some(_), HighFdStdin::Never) => false, }; - if feeds { - set_stdin_slot(&mut stdin, Some(content), None); + if let Some(fd) = high_fd { + if feeds { + let (copy, copy_storage) = + self.clone_input_stream(&content)?; + replace_own_fd(&mut own_fds, fd, copy, copy_storage); + set_stdin_slot(&mut stdin, Some(content), storage); + } else { + replace_own_fd(&mut own_fds, fd, content, storage); + } + } else { + set_stdin_slot(&mut stdin, Some(content), storage); } } + Err(e @ (crate::Error::ResourceLimit(_) | crate::Error::Cancelled)) => { + return Err(e); + } Err(e) => { return Err(crate::error::Error::CommandFailure( self.redirect_error(&target_path, &io_error_reason(&e)), @@ -394,26 +460,24 @@ impl Interpreter { RedirectKind::HereString => { // <<< string - use the target as stdin content let content = self.expand_word(&redirect.target).await?; - let content: crate::StreamData = format!("{}\n", content).into(); + let (content, storage) = self.input_stream_from_text(content, true)?; match high_fd { Some(fd) => { - let lease = self.charge_input_retention(content.retained_len())?; - replace_own_fd(&mut own_fds, fd, content, lease); + replace_own_fd(&mut own_fds, fd, content, storage); } - None => set_stdin_slot(&mut stdin, Some(content), None), + None => set_stdin_slot(&mut stdin, Some(content), storage), } } RedirectKind::HereDoc | RedirectKind::HereDocStrip => { // << EOF / <<- EOF - use the heredoc content as stdin; // `3< { - let lease = self.charge_input_retention(content.retained_len())?; - replace_own_fd(&mut own_fds, fd, content, lease); + replace_own_fd(&mut own_fds, fd, content, storage); } - None => set_stdin_slot(&mut stdin, Some(content), None), + None => set_stdin_slot(&mut stdin, Some(content), storage), } } RedirectKind::DupInput => { @@ -437,11 +501,10 @@ impl Interpreter { if let Some((_, content, _)) = own_fds.iter().rev().find(|(fd, _, _)| *fd == src) { - let lease = self.charge_input_retention(content.retained_len())?; - let content = content.clone(); + let (content, lease) = self.clone_input_stream(content)?; match high_fd { Some(fd) => replace_own_fd(&mut own_fds, fd, content, lease), - None => set_stdin_slot(&mut stdin, Some(content), Some(lease)), + None => set_stdin_slot(&mut stdin, Some(content), lease), } continue; } @@ -467,14 +530,15 @@ impl Interpreter { text.push('\n'); } } - set_stdin_slot(&mut stdin, Some(text.into()), None); + let (content, storage) = self.input_stream_from_text(text, false)?; + set_stdin_slot(&mut stdin, Some(content), storage); } Some(coproc::InputFd::Pipe(end)) => { let pipe = Arc::clone(&end.0); set_stdin_slot( &mut stdin, Some(Box::pin(coproc::read_coproc_input(pipe, demand)).await), - None, + [None, None, None], ); } // `<&5` with fd 5 never opened (or closed): bash @@ -496,7 +560,7 @@ impl Interpreter { } } - let mut stdin_lease = None; + let mut stdin_lease = [None, None, None]; let stdin = stdin.map(|(data, lease)| { stdin_lease = lease; data @@ -1645,3 +1709,192 @@ fn redirect_reads(kind: RedirectKind) -> bool { | RedirectKind::OutputBoth ) } + +#[cfg(test)] +mod input_admission_tests { + use super::*; + + fn ordinary_input(kind: RedirectKind) -> Redirect { + Redirect { + kind, + fd: None, + fd_var: None, + target: Word::quoted_literal("/a"), + heredoc_delim: None, + } + } + + #[tokio::test] + async fn input_admission_readwrite_refuses_lazy_load_before_open() { + let loads = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let observed = Arc::clone(&loads); + let fs = Arc::new(crate::InMemoryFs::new()); + fs.add_lazy_file( + "/a", + 2, + 0o644, + Arc::new(move || { + observed.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + b"ab".to_vec() + }), + ); + let mut bash = crate::Bash::builder() + .fs(fs) + .limits(crate::ExecutionLimits::new().max_aggregate_input_bytes(1)) + .build(); + bash.interpreter.begin_execution_budget(); + let redirect = ordinary_input(RedirectKind::ReadWrite); + let result = bash + .interpreter + .process_input_redirections(None, &[redirect], StdinDemand::All, HighFdStdin::Never) + .await; + assert!(matches!(result, Err(crate::Error::ResourceLimit(_)))); + assert_eq!(loads.load(std::sync::atomic::Ordering::Relaxed), 0); + } + + #[tokio::test] + async fn input_admission_preserves_cancellation_during_tiny_read() { + for kind in [RedirectKind::Input, RedirectKind::ReadWrite] { + let fs = Arc::new(crate::InMemoryFs::new()); + let mut bash = crate::Bash::builder().fs(fs.clone()).build(); + bash.interpreter.begin_execution_budget(); + let token = bash.interpreter.cancellation_token(); + fs.add_lazy_file( + "/a", + 2, + 0o644, + Arc::new(move || { + token.store(true, std::sync::atomic::Ordering::Relaxed); + b"ab".to_vec() + }), + ); + let redirect = ordinary_input(kind); + let result = bash + .interpreter + .process_input_redirections(None, &[redirect], StdinDemand::All, HighFdStdin::Never) + .await; + assert!( + matches!(result, Err(crate::Error::Cancelled)), + "cancellation must not become a command failure" + ); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + } + + #[tokio::test] + async fn input_admission_refuses_lazy_read_before_loading() { + for limits in [ + crate::ExecutionLimits::new().max_aggregate_input_bytes(1), + crate::ExecutionLimits::new().max_live_intermediate_bytes(1), + ] { + let loads = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let observed = Arc::clone(&loads); + let fs = Arc::new(crate::InMemoryFs::new()); + fs.add_lazy_file( + "/a", + 2, + 0o644, + Arc::new(move || { + observed.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + b"ab".to_vec() + }), + ); + let mut bash = crate::Bash::builder().fs(fs).limits(limits).build(); + bash.interpreter.begin_execution_budget(); + let result = bash + .interpreter + .read_input_stream(Path::new("/a"), false) + .await; + assert!(matches!(result, Err(crate::Error::ResourceLimit(_)))); + assert_eq!(loads.load(std::sync::atomic::Ordering::Relaxed), 0); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + } + + #[tokio::test] + async fn input_admission_tiny_read_exact_boundary_and_failure_refund() { + for (cap, succeeds) in [(3, false), (4, true)] { + let mut bash = crate::Bash::builder() + .limits(crate::ExecutionLimits::new().max_live_intermediate_bytes(cap)) + .build(); + bash.fs().write_file(Path::new("/a"), b"ab").await.unwrap(); + bash.interpreter.begin_execution_budget(); + let result = bash + .interpreter + .read_input_stream(Path::new("/a"), false) + .await; + if succeeds { + let (stream, storage) = result.unwrap(); + assert_eq!(stream.as_bytes(), b"ab"); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 4); + drop((stream, storage)); + } else { + assert!(matches!(result, Err(crate::Error::ResourceLimit(_)))); + } + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + } + + #[tokio::test] + async fn input_admission_preserves_missing_file_error() { + let mut bash = crate::Bash::new(); + bash.interpreter.begin_execution_budget(); + assert!( + bash.interpreter + .read_input_stream(Path::new("/absent"), false) + .await + .is_err() + ); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + + #[test] + fn input_admission_text_and_clone_hold_both_representations() { + for newline in [false, true] { + let mut bash = crate::Bash::new(); + bash.interpreter.begin_execution_budget(); + let (stream, storage) = bash + .interpreter + .input_stream_from_text("a".into(), newline) + .unwrap(); + let expected = if newline { + b"a\n".as_slice() + } else { + b"a".as_slice() + }; + assert_eq!(stream.as_bytes(), expected); + let original_live = bash.interpreter.execution_budget.live_bytes_for_test(); + assert!(original_live >= (2 * expected.len()) as u64); + let (copy, copy_storage) = bash.interpreter.clone_input_stream(&stream).unwrap(); + assert_eq!(copy, stream); + assert_eq!( + bash.interpreter.execution_budget.live_bytes_for_test(), + original_live + (2 * expected.len()) as u64 + ); + drop((copy, copy_storage)); + assert_eq!( + bash.interpreter.execution_budget.live_bytes_for_test(), + original_live + ); + drop((stream, storage)); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + } + + #[tokio::test] + async fn input_admission_retains_fresh_stdin_storage() { + let mut bash = crate::Bash::new(); + bash.fs().write_file(Path::new("/a"), b"ab").await.unwrap(); + bash.interpreter.begin_execution_budget(); + let redirect = ordinary_input(RedirectKind::Input); + let setup = bash + .interpreter + .process_input_redirections(None, &[redirect], StdinDemand::All, HighFdStdin::Never) + .await + .unwrap(); + assert_eq!(setup.stdin.as_ref().unwrap().as_bytes(), b"ab"); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 4); + drop(setup); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } +} diff --git a/crates/bashkit/src/stream.rs b/crates/bashkit/src/stream.rs index 4cf29b875..1af845df8 100644 --- a/crates/bashkit/src/stream.rs +++ b/crates/bashkit/src/stream.rs @@ -17,6 +17,41 @@ pub struct StreamData { } impl StreamData { + /// Decode a VFS buffer whose byte capacity is already leased by its caller. + pub(crate) fn from_admitted_bytes( + bytes: Vec, + budget: &crate::limits::ExecutionBudget, + ) -> Result<(Self, Option), crate::limits::LimitExceeded> + { + let mut text = crate::limits::BudgetedString::new(Some(budget))?; + for chunk in bytes.utf8_chunks() { + budget.consume_work(1 + chunk.valid().len().div_ceil(1024) as u64)?; + budget.consume_input(chunk.valid().len())?; + text.try_push_str(chunk.valid())?; + if !chunk.invalid().is_empty() { + budget.consume_input('\u{fffd}'.len_utf8())?; + text.try_push('\u{fffd}')?; + } + } + let (text, storage) = text.into_parts(); + Ok((Self { bytes, text }, storage)) + } + + /// Copy both representations through the existing capacity-owning builders. + pub(crate) fn try_clone_budgeted( + &self, + budget: &crate::limits::ExecutionBudget, + ) -> Result<(Self, StreamStorage), crate::limits::LimitExceeded> { + budget.consume_work(1 + self.retained_len().div_ceil(1024) as u64)?; + let mut bytes = crate::limits::BudgetedBytes::new(Some(budget))?; + let mut text = crate::limits::BudgetedString::new(Some(budget))?; + bytes.try_extend_from_slice(&self.bytes)?; + text.try_push_str(&self.text)?; + let (bytes, byte_storage) = bytes.into_parts(); + let (text, text_storage) = text.into_parts(); + Ok((Self { bytes, text }, [byte_storage, text_storage])) + } + /// Convert admitted text to the byte/text stream without an uncharged copy. pub(crate) fn from_budgeted_string( text: crate::limits::BudgetedString, @@ -226,6 +261,43 @@ impl serde::Serialize for StreamData { mod tests { use super::StreamData; + #[test] + fn admitted_bytes_preserve_lossy_text_and_lease() { + let budget = crate::limits::ExecutionBudget::new( + &crate::ExecutionLimits::new(), + std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + ); + let bytes = b"a\xff\0".to_vec(); + let raw = budget.lease_bytes(bytes.capacity()).unwrap(); + let (stream, text) = StreamData::from_admitted_bytes(bytes, &budget).unwrap(); + assert_eq!(stream.as_bytes(), b"a\xff\0"); + assert_eq!(stream.text_lossy(), "a\u{fffd}\0"); + assert!(budget.live_bytes_for_test() >= 8); + drop((stream, raw, text)); + assert_eq!(budget.live_bytes_for_test(), 0); + } + + #[test] + fn budgeted_clone_exact_boundary_and_refund() { + for (limit, succeeds) in [(3, false), (4, true)] { + let budget = crate::limits::ExecutionBudget::new( + &crate::ExecutionLimits::new().max_live_intermediate_bytes(limit), + std::sync::Arc::new(std::sync::atomic::AtomicBool::new(false)), + ); + let source = StreamData::from("ab"); + let result = source.try_clone_budgeted(&budget); + if succeeds { + let (copy, storage) = result.unwrap(); + assert_eq!(copy, source); + assert_eq!(budget.live_bytes_for_test(), 4); + drop((copy, storage)); + } else { + assert!(result.is_err()); + } + assert_eq!(budget.live_bytes_for_test(), 0); + } + } + #[test] fn budgeted_string_stream_admits_both_representations() { for limit in [7, 8] { diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index b0f1b4981..ffa803b6a 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -9,6 +9,61 @@ use bashkit::{Bash, Error, ExecutionBudgetExceeded, ExecutionLimits, LimitExceeded}; use std::path::Path; +#[tokio::test] +async fn input_redirection_small_file_byte_parity() { + let mut bash = Bash::new(); + bash.fs() + .write_file(Path::new("/a"), b"a\xff\0z") + .await + .unwrap(); + for script in ["cat /new").await.unwrap(); + assert_eq!(result.exit_code, 0); + assert!( + bash.fs() + .read_file(Path::new("/new")) + .await + .unwrap() + .is_empty() + ); + bash.fs().write_file(Path::new("/a"), b"ab").await.unwrap(); + let result = bash.exec("cat <>/a").await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout.as_bytes(), b"ab"); + assert_eq!(bash.fs().read_file(Path::new("/a")).await.unwrap(), b"ab"); +} + +#[tokio::test] +async fn input_redirection_missing_file_remains_shell_failure() { + let mut bash = Bash::new(); + let result = bash.exec("cat `. +Backend capacity beyond its reported size is charged before decoding. Cached +lossy text and fd copies use the existing owning builders; byte/text leases +travel with both the numbered-fd slot and surviving stdin. Reopening a numbered +fd replaces its old storage, without refunding aggregate input consumption. +An input not needed for stdin moves into the fd table instead of being copied. +Budget and cancellation errors abort the request; ordinary backend I/O errors +remain shell command failures. A custom VFS must report reliable read sizes; +the legacy `read_file` trait cannot prevent allocations inside a backend that +underreports or concurrently changes those sizes. Persistent/coprocess fd +storage remains a separate ownership boundary. + Shell diagnostic names are truncated to 1,024 UTF-8 bytes before copying, without changing positional `$0`. Builtin diagnostic prefix rewriting uses `BudgetedBytes` and fallible `StreamData` text construction under the same diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index fd5a7b9a0..57478033e 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -172,6 +172,19 @@ plus the package-named `build/bashkit` *directory* not being mistaken for the binary and a decoy above the target directory being refused. `bashkit_cli_path()` fails closed naming every path it ruled out. +## Command-local input admission + +`interpreter::redirection::input_admission_tests` uses two-byte ordinary and +lazy files to check admission before loading, including the empty open for +`<>`, exact byte/text storage boundaries, failed-admission refunds, surviving +stdin storage, and injected cancellation during a read. `stream::tests` covers +lossy decoding without changing bytes and capacity-owned copy boundaries. +The ordinary `input_redirection_*` integration cases cover binary stdin/fd +copies, empty read-write creation, existing-file preservation, here inputs, +and nonfatal missing-file diagnostics. These fixtures require no subprocess, +device read, large allocation, or original resource-exhaustion shape. Selecting +them is scoped proof, not a claim that the unchanged full/coverage suites ran. + ## Coverage Uploaded to Codecov from three sources: Rust unit/integration coverage via diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 604df625a..dd57f45b6 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -1429,6 +1429,16 @@ This section maps former vulnerability IDs to the new threat ID scheme and track | TM-DOS-106 | Zip creation allocation-before-check | `zip` recursively collected complete file contents and encoded its output before acquiring live-byte leases, allowing transient allocations above the configured intermediate-memory ceiling | Zip traversal collects paths only; each VFS file size is leased before `read_file`, leases remain live with the entries, and archive encoding uses `BudgetedBytes` to charge capacity before growth — **MITIGATED** | | TM-DOS-108 | CLI host-stdin read can stall before execution | A producer keeps the one-shot CLI's stdin pipe open while idle or trickles fewer than 10 MiB, so the pre-execution read never reaches EOF or the byte cap | Read stdin on a dedicated thread and bound the wait with the selected execution timeout; timeout exits with a diagnostic suggesting `--no-stdin`, while the existing byte cap remains enforced — **MITIGATED** | | TM-DOS-112 | `sed r` output amplification | A script can read the complete stdin after every input line, synchronously growing output quadratically inside the sed engine before any interpreter-level truncation applies | Every sed sink (stdout, `w`/`W` files, in-place edits) is a `BudgetedString` that leases capacity from the shared live-intermediate budget (`max_live_intermediate_bytes`) *before* growing, and a refused lease becomes `sed: ` with exit 1. The stdout *capture* cap is deliberately not a sink limit: sed output is routinely piped onward or redirected to a file, neither of which is captured stdout, so bounding sinks by it truncated ordinary transformations (#2455). Regression tests: `sed_resource_tests` — **MITIGATED** | + +Command-local input redirection storage also follows TM-DOS-103: canonical +file metadata and bounded device sizes are admitted before opening/reading; +cached text and fd copies use capacity-owning builders. Leases remain attached +to fd slots and surviving stdin, including failure and replacement paths. +Tiny `input_admission_tests` and `input_redirection_*` fixtures cover the +read/open ordering, byte fidelity, error typing and lifetime contracts. +This does not certify backend-internal allocation or persistent/coprocess fd +storage; a custom backend's reported read size remains a trusted contract. + ### Accepted (Low Priority) | Threat ID | Vulnerability | Impact | Rationale | From 994f5de5cbc17df6a752d54368e1aec0ec624717 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 13:31:26 -0500 Subject: [PATCH 12/16] test(input): retain measured bounded-child CI receipts --- .../input_redirect_budget_tests.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index ffa803b6a..2a3a82856 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -1202,6 +1202,40 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp /// exited before the first poll). #[cfg(not(target_arch = "wasm32"))] fn assert_effective_bounds(r: &InputShapeRun) { + use std::io::Write as _; + + let (artifact_class, artifact_bytes) = match &r.artifact { + ArtifactCheck::Absent => ("absent", None), + ArtifactCheck::Within(bytes) => ("within", Some(*bytes)), + ArtifactCheck::Over(bytes) => ("over", Some(*bytes)), + }; + let receipt = serde_json::json!({ + "producer_exit_code": r.exit_code, + "stack_kib": r.limits.stack_raw, + "cpu_seconds": r.limits.cpu_raw, + "address_space_kib": r.limits.mem_kb_raw, + "address_space_bytes": r.limits.mem_bytes, + "file_blocks": r.limits.fileblk_raw, + "file_bytes": r.limits.file_bytes, + "file_unit_bytes": r.file_unit, + "artifact_class": artifact_class, + "artifact_bytes": artifact_bytes, + "artifact_limit_bytes": IR_ARTIFACT_BOUND, + "reaped": r.reaped, + "readers_complete": !r.incomplete_readers, + "sampler_complete": r.sampler_complete, + "lifecycle_error": r.lifecycle_error, + "read_error": r.read_error, + "timed_out": r.timed_out, + "oversized": r.oversized, + "leftover_count": r.leftovers.len(), + "rss_samples": r.rss_samples, + "peak_rss_kib": r.peak_rss_kb, + "rss_exceeded": r.rss_exceeded, + }); + // Direct stderr preserves the measured receipt in successful CI logs too. + writeln!(std::io::stderr().lock(), "input-shape-receipt: {receipt}") + .expect("write bounded input receipt"); assert!(r.sampler_complete, "owned sampler completion unknown"); assert!( r.lifecycle_error.is_none(), From 13ed780fae4c81b7eb3cb637b3899923984f23b2 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 13:40:25 -0500 Subject: [PATCH 13/16] fix(input): admit buffered process substitution reads without resolving pipe links --- crates/bashkit/src/fs/proc_sub_fds.rs | 9 ++++ crates/bashkit/src/interpreter/redirection.rs | 49 ++++++++++++++++--- .../input_redirect_budget_tests.rs | 14 ++++++ knowledge/operations/testing.md | 6 ++- knowledge/security/threat-model.md | 3 ++ 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/crates/bashkit/src/fs/proc_sub_fds.rs b/crates/bashkit/src/fs/proc_sub_fds.rs index 1aa6a0ca7..191759a69 100644 --- a/crates/bashkit/src/fs/proc_sub_fds.rs +++ b/crates/bashkit/src/fs/proc_sub_fds.rs @@ -121,6 +121,15 @@ impl ProcSubFs { self.table().iter().any(|(n, _)| *n == fd) } + /// Buffered read size for admission; pipe stat keeps its POSIX zero size. + pub(crate) fn input_len(&self, path: &Path) -> Option { + match self.slot(path) { + Slot::Open(_, ProcSubData::Input(data)) => Some(data.len()), + Slot::Open(_, ProcSubData::Output(_)) => Some(0), + Slot::Vfs | Slot::Closed => None, + } + } + pub(crate) fn open(&self, fd: i32, data: ProcSubData) { self.table().push((fd, data)); } diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 64d926ae0..81e4fdd2b 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -273,14 +273,21 @@ impl Interpreter { create: bool, ) -> Result<(crate::StreamData, InputStorage)> { let budget = &self.execution_budget; - let source = match budget.run(crate::fs::canonicalize(&*self.fs, path)).await? { - Ok(source) => source, - Err(crate::Error::Io(error)) - if create && error.kind() == std::io::ErrorKind::NotFound => - { - path.to_path_buf() + // Open substitution links describe a pipe, not a VFS symlink target. + // Its owner supplies the buffered size before read_file copies it. + let buffered = self.proc_subs.input_len(path); + let source = if buffered.is_some() { + path.to_path_buf() + } else { + match budget.run(crate::fs::canonicalize(&*self.fs, path)).await? { + Ok(source) => source, + Err(crate::Error::Io(error)) + if create && error.kind() == std::io::ErrorKind::NotFound => + { + path.to_path_buf() + } + Err(error) => return Err(error), } - Err(error) => return Err(error), }; let size = match budget.run(self.fs.stat(&source)).await? { Ok(metadata) => metadata.size, @@ -299,7 +306,7 @@ impl Interpreter { Some("/dev/random" | "/dev/urandom") => 8192, _ => 0, }; - let size = reported.max(device); + let size = reported.max(device).max(buffered.unwrap_or(0)); budget.consume_input(size)?; budget.consume_work(1 + size.div_ceil(1024) as u64)?; let raw = budget.lease_bytes(size)?; @@ -1724,6 +1731,32 @@ mod input_admission_tests { } } + #[tokio::test] + async fn input_admission_tiny_buffered_pipe() { + for (cap, succeeds) in [(1, false), (4, true)] { + let mut bash = crate::Bash::builder() + .limits(crate::ExecutionLimits::new().max_live_intermediate_bytes(cap)) + .build(); + bash.interpreter.begin_execution_budget(); + bash.interpreter + .proc_subs + .open(63, crate::fs::ProcSubData::Input(Arc::from(&b"ab"[..]))); + let path = Path::new("/dev/fd/63"); + assert_eq!(bash.interpreter.fs.stat(path).await.unwrap().size, 0); + let result = bash.interpreter.read_input_stream(path, false).await; + if succeeds { + let (stream, storage) = result.unwrap(); + assert_eq!(stream.as_bytes(), b"ab"); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 4); + drop((stream, storage)); + } else { + assert!(matches!(result, Err(crate::Error::ResourceLimit(_)))); + } + assert!(bash.interpreter.proc_subs.is_open(63)); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + } + #[tokio::test] async fn input_admission_readwrite_refuses_lazy_load_before_open() { let loads = Arc::new(std::sync::atomic::AtomicUsize::new(0)); diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 2a3a82856..7aca1f0bf 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -53,6 +53,20 @@ async fn input_redirection_missing_file_remains_shell_failure() { assert!(!result.stderr.contains("io error:")); } +#[tokio::test] +async fn input_redirection_tiny_process_substitution() { + let mut bash = Bash::new(); + for script in [ + "cat < <(printf 'x\\ny\\n')", + "mapfile -t a < <(printf 'x\\ny\\n'); printf '%s\\n' \"${a[@]}\"", + ] { + let result = bash.exec(script).await.unwrap(); + assert_eq!(result.exit_code, 0); + assert_eq!(result.stdout.as_bytes(), b"x\ny\n"); + assert!(result.stderr.is_empty(), "{}", result.stderr); + } +} + #[tokio::test] async fn input_redirection_tiny_here_inputs() { let mut bash = Bash::new(); diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 57478033e..40d428c27 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -181,7 +181,11 @@ stdin storage, and injected cancellation during a read. `stream::tests` covers lossy decoding without changing bytes and capacity-owned copy boundaries. The ordinary `input_redirection_*` integration cases cover binary stdin/fd copies, empty read-write creation, existing-file preservation, here inputs, -and nonfatal missing-file diagnostics. These fixtures require no subprocess, +and tiny process-substitution input (including mapfile). A two-byte buffered +pipe fixture checks admission and lease release while pipe stat stays zero +and the existing substitution owner remains open. The owner exposes buffered +length without copying; admission must precede its `read_file` copy. +The cases also cover nonfatal missing-file diagnostics. These fixtures require no subprocess, device read, large allocation, or original resource-exhaustion shape. Selecting them is scoped proof, not a claim that the unchanged full/coverage suites ran. diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 01d1d2131..1310094e6 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -1434,6 +1434,9 @@ Command-local input redirection storage also follows TM-DOS-103: canonical file metadata and bounded device sizes are admitted before opening/reading; cached text and fd copies use capacity-owning builders. Leases remain attached to fd slots and surviving stdin, including failure and replacement paths. +Open process-substitution input uses its owner's buffered length before the +read copy, not POSIX pipe stat's zero size; its descriptive `pipe:[N]` link is +not canonicalized as a VFS target. The original buffered owner is unchanged. Tiny `input_admission_tests` and `input_redirection_*` fixtures cover the read/open ordering, byte fidelity, error typing and lifetime contracts. This does not certify backend-internal allocation or persistent/coprocess fd From 305b01d2b654b1012fc087fafcc6665c7097f998 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 13:43:52 -0500 Subject: [PATCH 14/16] fix(input): preserve configured filesystem symlink policy --- crates/bashkit/src/interpreter/redirection.rs | 8 +++++--- .../input_redirect_budget_tests.rs | 20 +++++++++++++++++++ knowledge/operations/testing.md | 5 +++-- knowledge/security/threat-model.md | 2 ++ 4 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 81e4fdd2b..7ebdb9e55 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -289,7 +289,9 @@ impl Interpreter { Err(error) => return Err(error), } }; - let size = match budget.run(self.fs.stat(&source)).await? { + // Preserve the configured VFS's following policy for actual I/O; + // canonical resolution above only identifies bounded device reads. + let size = match budget.run(self.fs.stat(path)).await? { Ok(metadata) => metadata.size, Err(crate::Error::Io(error)) if create && error.kind() == std::io::ErrorKind::NotFound => @@ -313,9 +315,9 @@ impl Interpreter { if create { // An empty append may load an existing lazy file, so admission // must precede opening as well as the subsequent read. - budget.run(self.fs.append_file(&source, b"")).await??; + budget.run(self.fs.append_file(path, b"")).await??; } - let bytes = budget.run(self.fs.read_file(&source)).await??; + let bytes = budget.run(self.fs.read_file(path)).await??; budget.consume_input(bytes.len().saturating_sub(size))?; let extra = budget.lease_bytes(bytes.capacity().saturating_sub(size))?; let (stream, text) = crate::StreamData::from_admitted_bytes(bytes, budget)?; diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index 7aca1f0bf..7fc8551e3 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -53,6 +53,26 @@ async fn input_redirection_missing_file_remains_shell_failure() { assert!(!result.stderr.contains("io error:")); } +#[tokio::test] +async fn input_redirection_respects_symlink_configuration() { + for follows in [false, true] { + let mut bash = Bash::builder().follow_symlinks(follows).build(); + bash.fs().write_file(Path::new("/a"), b"ab").await.unwrap(); + bash.fs() + .symlink(Path::new("/a"), Path::new("/link")) + .await + .unwrap(); + for script in ["cat /link"] { + let result = bash.exec(script).await.unwrap(); + assert_eq!(result.exit_code, if follows { 0 } else { 1 }); + assert_eq!( + result.stdout.as_bytes(), + if follows { &b"ab"[..] } else { &[] } + ); + } + } +} + #[tokio::test] async fn input_redirection_tiny_process_substitution() { let mut bash = Bash::new(); diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 40d428c27..61ea6bcc3 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -185,8 +185,9 @@ and tiny process-substitution input (including mapfile). A two-byte buffered pipe fixture checks admission and lease release while pipe stat stays zero and the existing substitution owner remains open. The owner exposes buffered length without copying; admission must precede its `read_file` copy. -The cases also cover nonfatal missing-file diagnostics. These fixtures require no subprocess, -device read, large allocation, or original resource-exhaustion shape. Selecting +The cases also cover nonfatal missing-file diagnostics and enabled/disabled +symlink-following for ordinary read/read-write input. These fixtures require no +subprocess, device read, large allocation, or original resource-exhaustion shape. Selecting them is scoped proof, not a claim that the unchanged full/coverage suites ran. ## Coverage diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 1310094e6..7555d9cce 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -1437,6 +1437,8 @@ to fd slots and surviving stdin, including failure and replacement paths. Open process-substitution input uses its owner's buffered length before the read copy, not POSIX pipe stat's zero size; its descriptive `pipe:[N]` link is not canonicalized as a VFS target. The original buffered owner is unchanged. +Canonical resolution only identifies device ceilings; actual stat/open/read +use the original path through the configured VFS, preserving its symlink policy. Tiny `input_admission_tests` and `input_redirection_*` fixtures cover the read/open ordering, byte fidelity, error typing and lifetime contracts. This does not certify backend-internal allocation or persistent/coprocess fd From d88a9ebce8e2447011883feb7609e903a103c51e Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 13:56:47 -0500 Subject: [PATCH 15/16] fix(input): follow sanitizer-compatible child policy and shrink redirect frames --- crates/bashkit/src/interpreter/redirection.rs | 5 +- .../input_redirect_budget_tests.rs | 59 +++++++++---------- knowledge/operations/testing.md | 10 ++++ 3 files changed, 42 insertions(+), 32 deletions(-) diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 7ebdb9e55..8bbe987c6 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -384,7 +384,8 @@ impl Interpreter { self.diag(format!("{}: filesystem redirection disabled", target_path)), )); } else { - let opened = self.read_input_stream(&path, true).await; + // Keep backend-admission state off recursive command frames. + let opened = Box::pin(self.read_input_stream(&path, true)).await; match opened { Ok(content) => content, Err(e @ (crate::Error::ResourceLimit(_) | crate::Error::Cancelled)) => { @@ -429,7 +430,7 @@ impl Interpreter { self.diag(format!("{}: filesystem redirection disabled", target_path)), )); } else { - match self.read_input_stream(&path, false).await { + match Box::pin(self.read_input_stream(&path, false)).await { Ok((content, storage)) => { // WTF: on a compound `3 String { @@ -485,9 +484,6 @@ fn input_shape_launcher() -> String { ulimit -f {fileblk} || exit 99; \ mem_kb=unsupported; mem_bytes=unsupported; file_bytes=unsupported; \ if [ \"$(uname)\" = Linux ] && [ -r /proc/self/limits ]; then \ - ulimit -v {askib} || exit 99; \ - mem_kb=$(ulimit -Sv); \ - mem_bytes=$(awk '/Max address space/ {{print $4}}' /proc/self/limits); \ file_bytes=$(awk '/Max file size/ {{print $4}}' /proc/self/limits); \ fi; \ echo \"ir-limits: stack=$(ulimit -s) cpu=$(ulimit -t) \ @@ -495,7 +491,6 @@ fn input_shape_launcher() -> String { fileblk=$(ulimit -Sf) file_bytes=$file_bytes\" >&2; \ exec \"$@\"", fileblk = IR_FILE_BLOCKS, - askib = IR_AS_KIB, ) } @@ -641,6 +636,7 @@ fn file_size_unit_factor(blocks: u64, bytes: u64) -> Result { #[cfg(not(target_arch = "wasm32"))] struct InputShapeRun { exit_code: Option, + wall_limit_seconds: u64, stdout: String, stderr: String, timed_out: bool, @@ -1204,6 +1200,7 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp work.cleanup().expect("work dir cleanup"); InputShapeRun { exit_code: completion.status.and_then(|s| s.code()), + wall_limit_seconds: wall_secs, stdout: String::from_utf8_lossy(&out_kept).into_owned(), stderr, timed_out: completion.timed_out, @@ -1232,7 +1229,7 @@ async fn run_input_shape_bounded(script: &str, wall_secs: u64, tag: &str) -> Inp /// round-trips everywhere; kernel byte proofs on Linux (including the /// shell-unit factor for `-f`); explicit unsupported markers elsewhere /// (fail-closed on any unit surprise); RSS policing receipts (a real pid, -/// no exceedance, peak under the cutoff, samples taken unless the child +/// no observed exceedance, sampled maximum under the cutoff, samples taken unless the child /// exited before the first poll). #[cfg(not(target_arch = "wasm32"))] fn assert_effective_bounds(r: &InputShapeRun) { @@ -1245,10 +1242,15 @@ fn assert_effective_bounds(r: &InputShapeRun) { }; let receipt = serde_json::json!({ "producer_exit_code": r.exit_code, + "wall_limit_seconds": r.wall_limit_seconds, + "per_stream_capture_limit_bytes": PIPE_CAP, "stack_kib": r.limits.stack_raw, "cpu_seconds": r.limits.cpu_raw, "address_space_kib": r.limits.mem_kb_raw, "address_space_bytes": r.limits.mem_bytes, + "address_space_enforced_by_helper": false, + "memory_guard": "sampled-rss", + "rss_limit_kib": IR_RSS_CAP_KIB, "file_blocks": r.limits.fileblk_raw, "file_bytes": r.limits.file_bytes, "file_unit_bytes": r.file_unit, @@ -1264,7 +1266,7 @@ fn assert_effective_bounds(r: &InputShapeRun) { "oversized": r.oversized, "leftover_count": r.leftovers.len(), "rss_samples": r.rss_samples, - "peak_rss_kib": r.peak_rss_kb, + "sampled_peak_rss_kib": r.peak_rss_kb, "rss_exceeded": r.rss_exceeded, }); // Direct stderr preserves the measured receipt in successful CI logs too. @@ -1279,6 +1281,11 @@ fn assert_effective_bounds(r: &InputShapeRun) { let limits = &r.limits; assert_eq!(limits.stack_raw, "4096", "stack cap round-trip"); assert_eq!(limits.cpu_raw, "10", "cpu cap round-trip"); + assert_eq!( + limits.mem_kb_raw, "unsupported", + "no AS enforcement by helper" + ); + assert_eq!(limits.mem_bytes, None, "no AS byteproof claimed"); assert_eq!( limits.fileblk_raw, IR_FILE_BLOCKS.to_string(), @@ -1286,12 +1293,6 @@ fn assert_effective_bounds(r: &InputShapeRun) { ); #[cfg(target_os = "linux")] { - assert_eq!( - limits.mem_kb_raw, - IR_AS_KIB.to_string(), - "AS cap round-trip" - ); - assert_eq!(limits.mem_bytes, Some(IR_AS_KIB * 1024), "AS bytes proof"); assert!( r.file_unit.is_some(), "file unit factor must be attested on Linux" @@ -1299,8 +1300,6 @@ fn assert_effective_bounds(r: &InputShapeRun) { } #[cfg(not(target_os = "linux"))] { - assert_eq!(limits.mem_kb_raw, "unsupported", "no AS enforcement here"); - assert_eq!(limits.mem_bytes, None, "no AS byteproof here"); assert_eq!(limits.file_bytes, None, "no file byteproof here"); assert_eq!(r.file_unit, None, "no file unit attested here"); } @@ -1312,7 +1311,7 @@ fn assert_effective_bounds(r: &InputShapeRun) { ); assert!( r.peak_rss_kb < IR_RSS_CAP_KIB, - "peak RSS {} KiB over cutoff", + "sampled peak RSS {} KiB over cutoff", r.peak_rss_kb ); assert!( @@ -1373,7 +1372,7 @@ fn ir_helper_launcher_fragments() { "ulimit -s 4096 || exit 99", "ulimit -t 10 || exit 99", "ulimit -f 131072 || exit 99", - "ulimit -v 1572864 || exit 99", + "mem_kb=unsupported; mem_bytes=unsupported", "ir-limits:", "mem_bytes=$mem_bytes", "file_bytes=$file_bytes", @@ -1388,11 +1387,11 @@ fn ir_helper_launcher_fragments() { #[test] fn ir_helper_parse_receipt() { - let full = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=1572864 mem_bytes=1610612736 fileblk=131072 file_bytes=134217728").unwrap(); + let full = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=unsupported mem_bytes=unsupported fileblk=131072 file_bytes=134217728").unwrap(); assert_eq!(full.stack_raw, "4096"); assert_eq!(full.cpu_raw, "10"); - assert_eq!(full.mem_kb_raw, "1572864"); - assert_eq!(full.mem_bytes, Some(1610612736)); + assert_eq!(full.mem_kb_raw, "unsupported"); + assert_eq!(full.mem_bytes, None); assert_eq!(full.fileblk_raw, "131072"); assert_eq!(full.file_bytes, Some(134217728)); let unsupported = parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=unsupported mem_bytes=unsupported fileblk=131072 file_bytes=unsupported").unwrap(); @@ -1400,7 +1399,7 @@ fn ir_helper_parse_receipt() { assert_eq!(unsupported.file_bytes, None); assert!(parse_ir_limits_note("bogus line").is_err()); assert!(parse_ir_limits_note("ir-limits: stack=4096 cpu=10").is_err()); - assert!(parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=1572864 mem_bytes=1610612736 fileblk=131072 file_bytes=nope").is_err()); + assert!(parse_ir_limits_note("ir-limits: stack=4096 cpu=10 mem_kb=unsupported mem_bytes=unsupported fileblk=131072 file_bytes=nope").is_err()); } #[test] diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index 61ea6bcc3..a7c2f25bf 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -286,6 +286,16 @@ reap or capture completion on panic or external cancellation. Tiny in-memory fixtures verify notice delivery before EOF, nested reap failures and pending-sample ownership. They do not establish subprocess, Linux or full security shipping proof. +The input helper follows the shared sanitizer-compatible launcher +policy: no virtual-AS cap; its receipt explicitly marks AS unsupported and +labels resident-memory observations as sampled RSS with an unchanged 1 GiB +cutoff, not hard OS enforcement or a between-sample peak guarantee. CPU 10s, +stack 4096 KiB, file backstop 128 MiB on measured Bash units, separate artifact +acceptance below 64 MiB, wall deadlines and capped drains remain unchanged. +Backend-admission futures are boxed at the recursive redirect boundary to +avoid embedding their state in each command frame; layout-only observations +do not establish a depth-execution result. + Controlled retained-pipe fixtures keep a caller-owned reader-handoff vector outside their cleanup guard. A bounded `Drop` returns any unfinished ORIGINAL handle to that owner rather than recording a count and detaching it. The owner From 7ec771785ea456ba3544933ad96c941b9d4ae0e4 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 15 Oct 2026 14:27:20 -0500 Subject: [PATCH 16/16] fix(input): admit mounted paths without synthetic parent entries --- crates/bashkit/src/interpreter/redirection.rs | 36 ++++++- .../input_redirect_budget_tests.rs | 100 ++++++++++++++++++ docs/security.md | 3 + knowledge/operations/testing.md | 6 ++ knowledge/security/threat-model.md | 3 + 5 files changed, 145 insertions(+), 3 deletions(-) diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 8bbe987c6..a28d2e2d3 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -281,9 +281,9 @@ impl Interpreter { } else { match budget.run(crate::fs::canonicalize(&*self.fs, path)).await? { Ok(source) => source, - Err(crate::Error::Io(error)) - if create && error.kind() == std::io::ErrorKind::NotFound => - { + Err(crate::Error::Io(error)) if error.kind() == std::io::ErrorKind::NotFound => { + // Mounted backends can serve a path without synthetic + // parent entries. Its stat/read below decide existence. path.to_path_buf() } Err(error) => return Err(error), @@ -1734,6 +1734,36 @@ mod input_admission_tests { } } + #[tokio::test] + async fn input_admission_mount_refuses_lazy_before_loading() { + let loads = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let observed = Arc::clone(&loads); + let mounted = Arc::new(crate::InMemoryFs::new()); + mounted.add_lazy_file( + "/a", + 2, + 0o644, + Arc::new(move || { + observed.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + b"ab".to_vec() + }), + ); + let fs = crate::MountableFs::new(Arc::new(crate::InMemoryFs::new())); + fs.mount("/mnt/input", mounted).unwrap(); + let mut bash = crate::Bash::builder() + .fs(Arc::new(fs)) + .limits(crate::ExecutionLimits::new().max_live_intermediate_bytes(1)) + .build(); + bash.interpreter.begin_execution_budget(); + let result = bash + .interpreter + .read_input_stream(Path::new("/mnt/input/a"), false) + .await; + assert!(matches!(result, Err(crate::Error::ResourceLimit(_)))); + assert_eq!(loads.load(std::sync::atomic::Ordering::Relaxed), 0); + assert_eq!(bash.interpreter.execution_budget.live_bytes_for_test(), 0); + } + #[tokio::test] async fn input_admission_tiny_buffered_pipe() { for (cap, succeeds) in [(1, false), (4, true)] { diff --git a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs index b04c8f8db..200843527 100644 --- a/crates/bashkit/tests/integration/input_redirect_budget_tests.rs +++ b/crates/bashkit/tests/integration/input_redirect_budget_tests.rs @@ -53,6 +53,106 @@ async fn input_redirection_missing_file_remains_shell_failure() { assert!(!result.stderr.contains("io error:")); } +#[tokio::test] +async fn input_redirection_tiny_mount_without_parent_entries() { + let mounted = std::sync::Arc::new(bashkit::InMemoryFs::new()); + let fs = bashkit::MountableFs::new(std::sync::Arc::new(bashkit::InMemoryFs::new())); + fs.mount("/mnt/input", mounted).unwrap(); + let mut bash = Bash::builder().fs(std::sync::Arc::new(fs)).build(); + bash.fs() + .write_file(Path::new("/mnt/input/a"), b"ab") + .await + .unwrap(); + for (script, expected) in [ + ("cat /mnt/input/a", "ab"), + (": <>/mnt/input/new", ""), + ("cat = + std::sync::Arc::new(bashkit::OverlayFs::new(lower.clone())); + let backend = if readonly { + std::sync::Arc::new(bashkit::ReadOnlyFs::new(overlay)) + as std::sync::Arc + } else { + overlay + }; + let fs = bashkit::MountableFs::new(std::sync::Arc::new(bashkit::InMemoryFs::new())); + fs.mount("/nested/data", backend).unwrap(); + let mut bash = Bash::builder().fs(std::sync::Arc::new(fs)).build(); + let read = bash.exec("cat /nested/data/a").await.unwrap(); + assert_eq!(opened.exit_code, if readonly { 1 } else { 0 }); + if readonly { + assert!(opened.stderr.contains("filesystem is read-only")); + assert!(opened.stdout.is_empty()); + } else { + assert_eq!(opened.stdout, "ab"); + } + assert_eq!( + bashkit::FileSystem::read_file(&*lower, Path::new("/a")) + .await + .unwrap(), + b"ab" + ); + let missing = bash + .exec("cat /nested/data/a").await.unwrap(); + assert_eq!(opened.exit_code, 1); + assert!(!opened.stderr.is_empty()); + assert_eq!( + bash.fs() + .read_file(Path::new("/nested/data/a")) + .await + .unwrap(), + b"ab" + ); +} + #[tokio::test] async fn input_redirection_respects_symlink_configuration() { for follows in [false, true] { diff --git a/docs/security.md b/docs/security.md index dc1e53924..a8e5e07a6 100644 --- a/docs/security.md +++ b/docs/security.md @@ -43,6 +43,9 @@ opening or reading, and admit cached text and descriptor copies before growth. Both representations count toward `max_live_intermediate_bytes` and aggregate input consumption. Storage charges survive with stdin; replacing a numbered descriptor releases its old storage without resetting aggregate consumption. +Admission uses the original backend path for stat/open/read, preserving mounted +namespaces, symlink policy, and read-only diagnostics; auxiliary device-path +canonicalization does not require synthetic mount-parent entries. Budget and cancellation errors abort the request, while ordinary missing-file errors remain shell command failures. Custom filesystems must report reliable read sizes; backend-internal allocations are still the backend's responsibility. diff --git a/knowledge/operations/testing.md b/knowledge/operations/testing.md index a7c2f25bf..5442f712e 100644 --- a/knowledge/operations/testing.md +++ b/knowledge/operations/testing.md @@ -187,6 +187,12 @@ and the existing substitution owner remains open. The owner exposes buffered length without copying; admission must precede its `read_file` copy. The cases also cover nonfatal missing-file diagnostics and enabled/disabled symlink-following for ordinary read/read-write input. These fixtures require no +host mount: a two-byte in-memory mounted file without synthetic parent entries +covers direct read, read-write/create and refusal before lazy loading. A missing +canonical parent is not proof the backend cannot stat/read the original path. +Two-byte overlay/read-only and rebased-namespace fixtures also check successful +read, rejected read-write open with backend diagnostics, and unchanged content. +These fixtures require no subprocess, device read, large allocation, or original resource-exhaustion shape. Selecting them is scoped proof, not a claim that the unchanged full/coverage suites ran. diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 05e852aac..c3e72e28b 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -1439,6 +1439,9 @@ read copy, not POSIX pipe stat's zero size; its descriptive `pipe:[N]` link is not canonicalized as a VFS target. The original buffered owner is unchanged. Canonical resolution only identifies device ceilings; actual stat/open/read use the original path through the configured VFS, preserving its symlink policy. +If canonical lookup lacks synthetic mount parents, the original backend path's +stat still decides read size/existence before allocation; non-NotFound lookup +errors remain visible. Tiny `input_admission_tests` and `input_redirection_*` fixtures cover the read/open ordering, byte fidelity, error typing and lifetime contracts. This does not certify backend-internal allocation or persistent/coprocess fd