From aa47985f9fb58e1ecbbfc2ac20fe99611cbc0264 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Fri, 21 Aug 2026 18:01:48 -0500 Subject: [PATCH] fix(host-call): enforce parked execution timeout --- Cargo.lock | 1 + crates/bashkit/Cargo.toml | 1 + crates/bashkit/docs/custom_builtins.md | 6 +- crates/bashkit/docs/threat-model.md | 2 +- crates/bashkit/src/host_call.rs | 96 ++++++++++++++----- .../integration/host_call_execution_tests.rs | 15 ++- knowledge/foundations/architecture.md | 11 ++- knowledge/foundations/builtins.md | 12 ++- knowledge/security/threat-model.md | 2 +- 9 files changed, 102 insertions(+), 44 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc92eb9d1..b44c1674a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -439,6 +439,7 @@ dependencies = [ "turso_core", "unit-prefix", "url", + "wasm-bindgen-futures", "web-time", "zapcode-core", "zeroize", diff --git a/crates/bashkit/Cargo.toml b/crates/bashkit/Cargo.toml index 8205a49ad..9cdf0cb8a 100644 --- a/crates/bashkit/Cargo.toml +++ b/crates/bashkit/Cargo.toml @@ -287,6 +287,7 @@ web-time = { workspace = true } [target.'cfg(all(target_arch = "wasm32", target_os = "unknown"))'.dependencies] gloo-timers = { version = "0.3", features = ["futures"] } send_wrapper = { version = "0.6", features = ["futures"] } +wasm-bindgen-futures = "0.4" # Vendored uucore modules (e.g. generated/format/human.rs) carry upstream # `#[cfg(feature = "i18n-decimal")]` gates. bashkit has no i18n surface — diff --git a/crates/bashkit/docs/custom_builtins.md b/crates/bashkit/docs/custom_builtins.md index cf2f6b969..70e5cdf6b 100644 --- a/crates/bashkit/docs/custom_builtins.md +++ b/crates/bashkit/docs/custom_builtins.md @@ -191,8 +191,10 @@ This is live-future suspension only. `ExecutionHandle` owns its `Bash` while the execution is active; after completion, call `into_bash()` to recover the session. Dropping a suspended handle drops the session so partially unwound interpreter state cannot be reused. Neither the handle nor a pending request is -serializable. Execution limits, including wall-clock timeout, remain active -while the host call is parked. Calling an event-backed command through ordinary +serializable. An independent driver keeps execution limits, including the +wall-clock timeout, active while the host call is parked. Timeout drops the +session even if the host never polls the handle again, so `into_bash()` cannot +recover a timed-out execution. Calling an event-backed command through ordinary `exec()` fails immediately because no execution driver is present. ## BuiltinRegistry — Runtime-Mutable Builtins diff --git a/crates/bashkit/docs/threat-model.md b/crates/bashkit/docs/threat-model.md index 25e0463f1..0d4b0b765 100644 --- a/crates/bashkit/docs/threat-model.md +++ b/crates/bashkit/docs/threat-model.md @@ -156,7 +156,7 @@ let bash = Bash::builder() | Multi-component glob amplification (TM-DOS-095) | `/*/*/*/*` multiplies the candidate set at each component | Reject patterns deeper than `max_path_depth`; cap live candidates at `max_file_count` | FIXED | | Aggregate budget refresh (TM-DOS-096) | Nest/mix parsers, pipelines, traversal, runtimes, archives, and callbacks to restart local ceilings | One poisoned request-scoped `ExecutionBudget` meters aggregate work/input/live bytes without replacing subsystem caps | MITIGATED | | Contradictory execution-profile limits (TM-DOS-097) | Host config silently requests ineffective or impossible limits | Validate profile cross-field invariants before `BashBuilder` accepts it | MITIGATED | -| Suspended host-call retention (TM-DOS-098) | Script repeats event-backed calls or host never resumes one | Capacity-one channel, normal execution limits, and handle-owned session released on drop | MITIGATED | +| Suspended host-call retention (TM-DOS-098) | Script repeats event-backed calls or host never resumes one | Capacity-one channel; independent driver enforces timeout and drops expired sessions without another host poll; handle drop aborts the driver | MITIGATED | | `time` report amplification (TM-DOS-099) | Attacker-controlled `-f` format expands repeatedly or targets the VFS with `-o` | Incremental rendering is capped by the stderr limit before emission or file replacement | MITIGATED | | jq control normalization amplification (TM-DOS-100) | Literal controls expand sixfold as `\u00XX` | Charge single-pass work and lease live bytes before allocation growth | MITIGATED | | yq structured-data amplification (TM-DOS-101) | Deep/multi-document YAML or JSON, runaway filters, expanded output | Parser depth and 4096-document caps, aggregate budgets, shared jaq work/deadline/output limits, final render cap | MITIGATED | diff --git a/crates/bashkit/src/host_call.rs b/crates/bashkit/src/host_call.rs index 5d60d1b23..04bbb56ed 100644 --- a/crates/bashkit/src/host_call.rs +++ b/crates/bashkit/src/host_call.rs @@ -1,5 +1,6 @@ -// Host calls deliberately suspend only the live Rust execution future. They -// are not interpreter snapshots and cannot be serialized or resumed elsewhere. +// Host calls use an independently driven live Rust future so execution +// deadlines remain active while the host is parked. They are not interpreter +// snapshots and cannot be serialized or resumed elsewhere. use std::collections::HashMap; use std::future::Future; @@ -146,24 +147,29 @@ impl Builtin for HostCallBuiltin { } } -type ExecutionFuture = Pin, Result)> + Send>>; +type ExecutionFuture = Pin + Send>>; +type ExecutionCompletion = (Option>, Result); /// Drives one process-local execution across event-backed builtin calls. /// -/// The handle owns the [`crate::Bash`] instance until execution completes. -/// Recover it with [`ExecutionHandle::into_bash`]. Dropping a suspended handle -/// drops the session rather than exposing partially unwound interpreter state. +/// The handle controls the driver that owns the [`crate::Bash`] instance until +/// execution completes. Recover it with [`ExecutionHandle::into_bash`] after +/// normal completion. Timeout or handle drop discards the session rather than +/// exposing partially unwound interpreter state. pub struct ExecutionHandle { - future: Option, + unstarted: Option, + completion: oneshot::Receiver, + abort: futures_util::future::AbortHandle, requests: mpsc::Receiver, pending: HashMap>, completed_bash: Option>, + finished: bool, } impl std::fmt::Debug for ExecutionHandle { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ExecutionHandle") - .field("active", &self.future.is_some()) + .field("active", &!self.finished) .field("pending_calls", &self.pending.len()) .field("completed", &self.completed_bash.is_some()) .finish() @@ -172,24 +178,41 @@ impl std::fmt::Debug for ExecutionHandle { impl ExecutionHandle { pub(crate) fn new(bash: crate::Bash, script: String, mut options: ExecOptions) -> Self { - // THREAT[TM-DOS-098]: one bounded slot prevents script-driven request - // accumulation while the existing execution deadline remains active. + // THREAT[TM-DOS-098]: the bounded request slot prevents accumulation; + // the independently driven future enforces the deadline while parked. let (requests, request_rx) = mpsc::channel(1); + let (completion_tx, completion) = oneshot::channel(); + let (abort, abort_registration) = futures_util::future::AbortHandle::new_pair(); let broker = HostCallBroker { requests, next_id: Arc::new(AtomicU64::new(1)), }; let _ = options.extensions.insert(broker); let mut bash = Box::new(bash); - let future = Box::pin(async move { + let execution = async move { let result = bash.exec_with_options(&script, options).await; (bash, result) + }; + let future = Box::pin(async move { + let (bash, result) = execution.await; + let timed_out = matches!( + result, + Err(Error::ResourceLimit(crate::LimitExceeded::Timeout(_))) + ); + let completion = (if timed_out { None } else { Some(bash) }, result); + let _ = completion_tx.send(completion); + }); + let future = Box::pin(async move { + let _ = futures_util::future::Abortable::new(future, abort_registration).await; }); Self { - future: Some(future), + unstarted: Some(future), + completion, + abort, requests: request_rx, pending: HashMap::new(), completed_bash: None, + finished: false, } } @@ -197,17 +220,22 @@ impl ExecutionHandle { pub async fn next_event(&mut self) -> Result { enum Next { Request(Option), - Complete((Box, Result)), + Complete(ExecutionCompletion), } - let Some(future) = self.future.as_mut() else { + if let Some(future) = self.unstarted.take() { + spawn_execution(future); + } + if self.finished { return Err(Error::Execution( "execution handle has already completed".to_string(), )); - }; + } let next = tokio::select! { request = self.requests.recv() => Next::Request(request), - result = future => Next::Complete(result), + result = &mut self.completion => Next::Complete(result.map_err(|_| { + Error::Execution("host-call execution driver was dropped".to_string()) + })?), }; match next { Next::Request(Some(envelope)) => { @@ -216,20 +244,18 @@ impl ExecutionHandle { Ok(ExecutionEvent::HostCall(envelope.request)) } Next::Request(None) => { - let (bash, result) = self - .future - .as_mut() - .expect("execution future checked above") - .await; - self.future = None; + let (bash, result) = (&mut self.completion).await.map_err(|_| { + Error::Execution("host-call execution driver was dropped".to_string()) + })?; self.pending.clear(); - self.completed_bash = Some(bash); + self.completed_bash = bash; + self.finished = true; result.map(ExecutionEvent::Complete) } Next::Complete((bash, result)) => { - self.future = None; self.pending.clear(); - self.completed_bash = Some(bash); + self.completed_bash = bash; + self.finished = true; result.map(ExecutionEvent::Complete) } } @@ -256,3 +282,23 @@ impl ExecutionHandle { } } } + +impl Drop for ExecutionHandle { + fn drop(&mut self) { + self.abort.abort(); + } +} + +#[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))] +fn spawn_execution(future: ExecutionFuture) { + tokio::spawn(async move { + let _ = future.await; + }); +} + +#[cfg(all(target_arch = "wasm32", target_os = "unknown"))] +fn spawn_execution(future: ExecutionFuture) { + wasm_bindgen_futures::spawn_local(async move { + let _ = future.await; + }); +} diff --git a/crates/bashkit/tests/integration/host_call_execution_tests.rs b/crates/bashkit/tests/integration/host_call_execution_tests.rs index 75c3bd221..9c14be3c3 100644 --- a/crates/bashkit/tests/integration/host_call_execution_tests.rs +++ b/crates/bashkit/tests/integration/host_call_execution_tests.rs @@ -127,14 +127,19 @@ async fn suspended_host_call_remains_inside_the_execution_timeout() { let mut execution = bash.start_execution_with_options("lookup alice", ExecOptions::new().stdin("unused")); - assert!(matches!( - execution.next_event().await.unwrap(), - ExecutionEvent::HostCall(_) - )); + let request = match execution.next_event().await.unwrap() { + ExecutionEvent::HostCall(request) => request, + ExecutionEvent::Complete(_) => panic!("execution completed before its host call"), + }; tokio::time::advance(Duration::from_secs(3)).await; + tokio::task::yield_now().await; + let resume_error = execution + .resume(request.id(), ExecResult::ok("too late\n")) + .unwrap_err(); + assert!(resume_error.to_string().contains("no longer active")); let error = execution.next_event().await.unwrap_err(); assert!(error.to_string().contains("timeout")); - let _bash = execution.into_bash().unwrap(); + assert!(execution.into_bash().is_err()); } #[tokio::test] diff --git a/knowledge/foundations/architecture.md b/knowledge/foundations/architecture.md index 01fac774d..e43b7b44c 100644 --- a/knowledge/foundations/architecture.md +++ b/knowledge/foundations/architecture.md @@ -75,11 +75,12 @@ parse checks that can return early cannot leave state behind. ### Process-local execution suspension Event-backed host calls reuse the async-first execution model: an -`ExecutionHandle` owns and polls the live execution future, yields an owned -request to the host, then resolves a one-shot response when the host resumes -it. The handle exclusively owns the interpreter and its timeout remains -active. On completion, `into_bash` returns the reusable session; dropping a -suspended handle drops the session. This is process-local scheduling, not a +`ExecutionHandle` starts an independently driven live execution future, yields +an owned request to the host, then resolves a one-shot response when the host +resumes it. The driver keeps the interpreter timeout active while the host is +parked and drops the session autonomously on timeout. On normal completion, +`into_bash` returns the reusable session; dropping a suspended handle aborts +the driver and drops the session. This is process-local scheduling, not a serializable continuation; snapshots still capture state only between executions. See [Builtin Commands](builtins.md) and [Snapshot History](snapshot-history.md). diff --git a/knowledge/foundations/builtins.md b/knowledge/foundations/builtins.md index 074896bdc..9ad1db286 100644 --- a/knowledge/foundations/builtins.md +++ b/knowledge/foundations/builtins.md @@ -249,17 +249,19 @@ in the interpreter's plan fulfillment code (`interpreter/mod.rs`). ### Process-Local Host-Call Suspension `BashBuilder::host_call_builtin(name)` registers a command fulfilled by the -host through `Bash::start_execution`. `ExecutionHandle::next_event()` polls the -ordinary interpreter future until it completes or the builtin sends a +host through `Bash::start_execution`. The first `ExecutionHandle::next_event()` +starts an independent interpreter driver and waits until it completes or the builtin sends a `HostCallRequest`; `resume(id, ExecResult)` resolves the one-shot response and lets the same future continue. The bounded request channel applies backpressure, request IDs prevent mismatched responses, ordinary `exec()` fails the builtin promptly, and the normal execution timeout remains armed -while a request is pending. +while a request is pending. A timeout drops the session without requiring +another handle poll, so timed-out executions cannot be recovered with +`into_bash`. This mechanism intentionally does not change interpreter control flow into a -serializable state machine. The handle owns both a pinned Rust future and the -`Bash` instance; completion makes the session recoverable through `into_bash`, +serializable state machine. The driver owns both a pinned Rust future and the +`Bash` instance; normal completion makes the session recoverable through `into_bash`, while dropping a suspended handle drops the session so partially unwound state cannot be reused. Pending calls cannot be included in snapshots or resumed in another process. Portable mid-execution resume would require explicit diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index a210272be..95e5b1731 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -1211,7 +1211,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track | TM-DOS-095 | Multi-component glob cross-product amplification | Per-component pathname expansion walks a candidate set that multiplies at every glob component, so a wide tree plus a pattern like `/*/*/*/*` can materialize far more candidates than the tree has leaves | `expand_glob` rejects patterns deeper than `FsLimits::max_path_depth` and truncates the live candidate set to `FsLimits::max_file_count` after each component (`interpreter/glob.rs`) — **FIXED** | | TM-DOS-096 | Aggregate execution-budget refresh | Nested substitutions, pipelines, traversal, builtin/runtime interpreters, archive expansion, and host callbacks each relied on independent ceilings, letting mixed workloads repeatedly restart resource accounting inside one request | `ExecutionBudget` is created once per host `exec_with_options`, cloned into parser/interpreter descendants and execution extensions, and monotonically meters aggregate work, aggregate input, and live intermediate leases. Any ceiling, deadline, or cancellation poisons the shared request while existing subsystem limits remain enforced — **MITIGATED** | | TM-DOS-097 | Contradictory execution-profile limits | A host supplies a profile whose single-file quota exceeds its total VFS quota, requests an AST depth above the parser hard cap, or configures a zero runtime budget and assumes the ineffective value is enforced | `ExecutionProfileBuilder::build()` validates cross-field invariants and compiled-feature support before a profile can reach `BashBuilder`; typed named profiles are valid by construction — **MITIGATED** | -| TM-DOS-098 | Suspended host-call retention or request accumulation | An untrusted script repeatedly invokes an event-backed builtin, or the host never resumes a yielded request, retaining interpreter and request data indefinitely | The sequential interpreter can reach only one call at a time; a capacity-one channel adds backpressure; the existing command, aggregate-budget, output, and wall-clock limits remain in force; `ExecutionHandle` exclusively owns the session so dropping it releases all retained state rather than exposing a partially unwound interpreter — **MITIGATED** | +| TM-DOS-098 | Suspended host-call retention or request accumulation | An untrusted script repeatedly invokes an event-backed builtin, or the host never resumes a yielded request, retaining interpreter and request data indefinitely | The sequential interpreter can reach only one call at a time; a capacity-one channel adds backpressure; an independent execution driver keeps the wall-clock timeout armed while parked and drops the timed-out session without another host poll; dropping `ExecutionHandle` aborts the driver and releases all retained state — **MITIGATED** | | TM-DOS-099 | `time -f/-o` report amplification bypasses output limits | A large attacker-controlled format repeats expanding fields and writes the result to the VFS instead of stderr | Report rendering is capped by `ExecutionLimits::max_stderr_bytes` before either stderr emission or VFS write; invalid/over-limit reports do not replace an existing `-o` target — **MITIGATED** | | TM-DOS-100 | jq control-character normalization amplification | A jq JSON string consisting of literal controls expands sixfold when each byte becomes `\u00XX`; an unmetered compatibility copy can exhaust memory or CPU before strict parsing | The jq-only normalizer charges input-length work before its single pass, borrows unchanged input, and acquires/grows a shared live-intermediate lease before every allocation growth (`builtins/jq/input.rs`) — **MITIGATED** | | TM-DOS-101 | yq structured-data amplification | YAML/JSON nesting, multi-document floods, filters, and output format expansion are all attacker-controlled | Real parsers with recursion/depth caps, aggregate budgets, shared jaq work/deadline/output controls, and a final rendered-output cap; integration tests cover YAML+JSON depth/document/input/output bounds, proptest composes arbitrary YAML+filters, and `yq_fuzz` covers stdin/in-place format paths — **MITIGATED** |