Skip to content

Commit 371fb27

Browse files
authored
fix(builtins): markkit/fetchkit respect the exec deadline off the async worker (#2870)
The markkit and fetchkit CLIs run on the blocking pool, and their timeouts are capped at the remaining exec budget, so a large document can no longer outlive the exec timeout or stall co-tenants on the worker (TM-DOS-143).
1 parent a318206 commit 371fb27

7 files changed

Lines changed: 126 additions & 7 deletions

File tree

‎crates/bashkit/docs/threat-model.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ through configurable limits.
3535
| Pagination amplification (TM-DOS-137) | Long `pr -h` headers repeated on many short pages | Check the 16 MiB cap before every append; lease input, rows and output capacity before growth; borrow cells/page slices and retain date timestamps; charge input/work and poll cancellation/deadlines, including skipped pages | MITIGATED |
3636
| Embedded CLI limit escalation (TM-DOS-139) | `markkit extract x --max-input-size 99999999999` | `markkit` limits can only be lowered below their defaults; `fetchkit` bodies capped by HTTP limits; `--ocr` refused | MITIGATED |
3737
| Tree-walk memory (TM-DOS-140) | `find`, `du` or `rg` over a tree with millions of files | `FileSystem::walk` answers are bounded batches (10k entries) refilled as `find`/`du` descend; `rg` searches each file as it reads it without holding the text, charges it to the aggregate-input budget, stops at the first refusal and streams its walk; `ls -R` stops at the stdout limit | MITIGATED |
38+
| Extraction outlives deadline (TM-DOS-143) | `markkit extract` on a huge document under a short exec timeout | markkit/fetchkit run off the async worker; their timeouts are capped at the remaining exec budget | MITIGATED |
3839
| Root real mount drops limits (TM-DOS-144) | Writing past `filesystem_limits` with a host dir mounted at `/` | The root overlay keeps the configured `FsLimits` | MITIGATED |
3940
| Variable explosion (TM-DOS-003) | `x=$(cat /dev/urandom)` | /dev/urandom returns bounded 8KB | MITIGATED |
4041
| Array growth (TM-DOS-004) | `arr+=(element)` in loop | Command limit | MITIGATED |

‎crates/bashkit/src/builtins/fetchkit.rs‎

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
//! (bashkit signs in `HttpClient`; a key in argv would double-sign and land in
2121
//! history), `--allow-remote-conversion` (uploads documents to a third
2222
//! party), and the `mcp` subcommand. All still parse and show in `--help`.
23+
//! - **Off the async worker (TM-DOS-143).** HTML-to-Markdown conversion is
24+
//! synchronous CPU work; the run happens on tokio's blocking pool so the
25+
//! `exec` timeout can cancel the caller and co-tenants keep the worker. Per
26+
//! request, the transport timeout is capped at the remaining `exec` budget.
2327
//! - **Stateless.** No globals; every run builds its own fetchkit `Tool`.
2428
2529
use std::io;
@@ -48,9 +52,15 @@ impl Builtin for Fetchkit {
4852
stderr: Mutex::new(String::new()),
4953
env: ctx.env.clone(),
5054
http: ctx.http_client.cloned(),
55+
deadline: ctx
56+
.remaining_deadline()
57+
.map(|left| crate::time_compat::Instant::now() + left),
5158
};
52-
let args = std::iter::once("fetchkit".to_string()).chain(ctx.args.iter().cloned());
53-
let exit_code = fetchkit_cli::run(args, &host).await;
59+
let args: Vec<String> = std::iter::once("fetchkit".to_string())
60+
.chain(ctx.args.iter().cloned())
61+
.collect();
62+
let host = Arc::new(host);
63+
let exit_code = run_off_worker(args, host.clone()).await;
5464
Ok(ExecResult {
5565
stdout: take(&host.stdout).into(),
5666
stderr: take(&host.stderr).into(),
@@ -78,6 +88,19 @@ struct Host {
7888
stderr: Mutex<String>,
7989
env: std::collections::HashMap<String, String>,
8090
http: Option<Arc<HttpClient>>,
91+
/// End of the `exec` wall-clock budget, if limited.
92+
deadline: Option<crate::time_compat::Instant>,
93+
}
94+
95+
/// Runs the CLI on tokio's blocking pool (TM-DOS-143). `Handle::block_on`
96+
/// drives the transport there; on a current-thread runtime the caller, parked
97+
/// awaiting the join handle, keeps driving IO and timers.
98+
async fn run_off_worker(args: Vec<String>, host: Arc<Host>) -> i32 {
99+
let Ok(handle) = tokio::runtime::Handle::try_current() else {
100+
return fetchkit_cli::run(args, &*host).await;
101+
};
102+
let job = tokio::task::spawn_blocking(move || handle.block_on(fetchkit_cli::run(args, &*host)));
103+
job.await.unwrap_or(1)
81104
}
82105

83106
#[async_trait]
@@ -106,6 +129,7 @@ impl CliHost for Host {
106129
builder
107130
.transport(Arc::new(SandboxTransport {
108131
http: self.http.clone(),
132+
deadline: self.deadline,
109133
}))
110134
.block_private_ips(false)
111135
.use_env_proxy(false)
@@ -121,6 +145,7 @@ impl CliHost for Host {
121145
/// fetchkit follows redirects itself, so every hop is re-checked here.
122146
struct SandboxTransport {
123147
http: Option<Arc<HttpClient>>,
148+
deadline: Option<crate::time_compat::Instant>,
124149
}
125150

126151
#[async_trait]
@@ -139,7 +164,15 @@ impl HttpTransport for SandboxTransport {
139164
TransportMethod::Head => Method::Head,
140165
};
141166
// bashkit clamps the deadline to its own bounds; round sub-second up.
142-
let timeout_secs = req.timeout.map(|t| t.as_secs().max(1));
167+
let left = self.deadline.map(|end| {
168+
end.checked_duration_since(crate::time_compat::Instant::now())
169+
.unwrap_or_default()
170+
});
171+
let timeout = match (req.timeout, left) {
172+
(Some(t), Some(left)) => Some(t.min(left)),
173+
(t, left) => t.or(left),
174+
};
175+
let timeout_secs = timeout.map(|t| t.as_secs().max(1));
143176
let response = http
144177
.request_with_timeout(method, req.url.as_str(), None, &req.headers, timeout_secs)
145178
.await

‎crates/bashkit/src/builtins/markkit.rs‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
//! - **Limits only go down.** `--max-*`/`--timeout` flags may lower markkit's
2020
//! default limits but never raise them (`Host::limits`), so one tenant's
2121
//! script cannot buy a larger CPU/memory budget from a shared process.
22+
//! - **Deadline (TM-DOS-143).** Extraction is synchronous CPU work. Inline
23+
//! it held the tenant's async worker (starving co-tenants scheduled there)
24+
//! and the `exec` timeout could not interrupt it. The run now happens on
25+
//! tokio's blocking pool, so the caller's future stays cancellable and the
26+
//! worker stays free, and markkit's cooperative `timeout` is capped at the
27+
//! remaining `exec` deadline so abandoned work stops soon after.
2228
//! - **Refused in the sandbox:** `--ocr` (runs a host Tesseract binary, reads
2329
//! host model files) and the `mcp` subcommand (long-running stdio server).
2430
//! Both still parse and show in `--help`; they fail with exit 1.
@@ -51,9 +57,15 @@ impl Builtin for Markkit {
5157
cwd: ctx.cwd.clone(),
5258
#[cfg(feature = "http_client")]
5359
http: ctx.http_client.cloned(),
60+
deadline: ctx
61+
.remaining_deadline()
62+
.map(|left| crate::time_compat::Instant::now() + left),
5463
};
55-
let args = std::iter::once("markkit".to_string()).chain(ctx.args.iter().cloned());
56-
let exit_code = markkit_cli::run(args, &host).await;
64+
let args: Vec<String> = std::iter::once("markkit".to_string())
65+
.chain(ctx.args.iter().cloned())
66+
.collect();
67+
let host = Arc::new(host);
68+
let exit_code = run_off_worker(args, host.clone()).await;
5769
Ok(ExecResult {
5870
stdout: host.take_stdout().into(),
5971
stderr: host.take_stderr().into(),
@@ -71,6 +83,18 @@ impl Builtin for Markkit {
7183
}
7284
}
7385

86+
/// Runs the CLI on tokio's blocking pool (TM-DOS-143). `Handle::block_on`
87+
/// drives the host's async VFS/HTTP calls there; on a current-thread runtime
88+
/// the caller, parked awaiting the join handle, keeps driving IO and timers.
89+
async fn run_off_worker(args: Vec<String>, host: Arc<Host>) -> i32 {
90+
let Ok(handle) = tokio::runtime::Handle::try_current() else {
91+
return markkit_cli::run(args, &*host).await;
92+
};
93+
let job = tokio::task::spawn_blocking(move || handle.block_on(markkit_cli::run(args, &*host)));
94+
// A markkit panic that unwinds reports as a plain failure.
95+
job.await.unwrap_or(1)
96+
}
97+
7498
/// [`CliHost`] over one builtin invocation.
7599
struct Host {
76100
stdout: Mutex<String>,
@@ -80,6 +104,9 @@ struct Host {
80104
cwd: PathBuf,
81105
#[cfg(feature = "http_client")]
82106
http: Option<Arc<crate::network::HttpClient>>,
107+
/// End of the `exec` wall-clock budget, if limited. An instant, not a
108+
/// duration, so time queued behind a busy blocking pool still counts.
109+
deadline: Option<crate::time_compat::Instant>,
83110
}
84111

85112
impl Host {
@@ -148,7 +175,11 @@ impl CliHost for Host {
148175
}
149176

150177
fn limits(&self, requested: Limits) -> Limits {
151-
clamp_limits(requested, &Limits::default())
178+
let left = self.deadline.map(|end| {
179+
end.checked_duration_since(crate::time_compat::Instant::now())
180+
.unwrap_or_default()
181+
});
182+
clamp_limits(requested, &deadline_cap(left))
152183
}
153184

154185
/// Fail closed: every host-only feature (today `--ocr`) is refused.
@@ -172,6 +203,15 @@ fn to_io_error(err: Error) -> io::Error {
172203
}
173204
}
174205

206+
/// markkit's defaults, with `timeout` capped at the remaining `exec` budget.
207+
fn deadline_cap(deadline: Option<std::time::Duration>) -> Limits {
208+
let mut cap = Limits::default();
209+
if let Some(remaining) = deadline {
210+
cap.timeout = cap.timeout.min(remaining);
211+
}
212+
cap
213+
}
214+
175215
/// Each limit is the smaller of what the flags asked for and the cap.
176216
fn clamp_limits(requested: Limits, cap: &Limits) -> Limits {
177217
let mut l = requested;
@@ -298,6 +338,16 @@ mod tests {
298338
assert_eq!(got, cap);
299339
}
300340

341+
#[test]
342+
fn timeout_capped_at_remaining_deadline() {
343+
let remaining = std::time::Duration::from_millis(1500);
344+
let got = clamp_limits(Limits::default(), &deadline_cap(Some(remaining)));
345+
assert_eq!(got.timeout, remaining);
346+
// No deadline: markkit's default stands.
347+
let got = clamp_limits(Limits::default(), &deadline_cap(None));
348+
assert_eq!(got.timeout, Limits::default().timeout);
349+
}
350+
301351
#[test]
302352
fn clamp_keeps_lowered_limits() {
303353
let cap = Limits::default();

‎crates/bashkit/tests/integration/markkit_builtin_tests.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,35 @@ mod network {
251251
assert_eq!(site.calls.lock().unwrap().len(), 6);
252252
}
253253
}
254+
255+
// THREAT[TM-DOS-143]: extraction runs synchronously where the exec timeout
256+
// cannot cancel it, so markkit's own timeout is capped at the remaining
257+
// exec budget. Without the cap this ran to markkit's 60 s default.
258+
#[tokio::test]
259+
async fn extraction_stops_at_exec_deadline() {
260+
use bashkit::ExecutionLimits;
261+
use std::time::{Duration, Instant};
262+
263+
let mut bash = Bash::builder()
264+
.limits(ExecutionLimits::new().timeout(Duration::from_millis(300)))
265+
.build();
266+
let page = "<div><p>word <b>bold</b> <a href=\"/x\">link</a></p></div>".repeat(170_000);
267+
bash.fs()
268+
.write_file(std::path::Path::new("/big.html"), page.as_bytes())
269+
.await
270+
.unwrap();
271+
let started = Instant::now();
272+
let r = bash
273+
.exec("markkit extract --file /big.html > /dev/null")
274+
.await;
275+
let elapsed = started.elapsed();
276+
assert!(
277+
elapsed < Duration::from_secs(3),
278+
"markkit ran {elapsed:?} past a 300 ms exec timeout ({r:?})" // debug-ok: test diagnostics
279+
); // The timeout itself must have fired, not a fast success.
280+
let timed_out = match &r {
281+
Ok(r) => r.exit_code == 124,
282+
Err(e) => e.to_string().contains("timed out") || e.to_string().contains("timeout"),
283+
};
284+
assert!(timed_out, "exec did not time out: {r:?}"); // debug-ok: test diagnostics
285+
}

‎knowledge/log.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## 2026-11-06
44

5+
* **markkit/fetchkit honor the exec deadline**: a security audit found `markkit extract` on a large document ran synchronously on the tenant's async worker, so a 300 ms `exec` timeout returned after 7+ s and co-tenants on that worker stalled. Both builtins now run on tokio's blocking pool, and their timeouts are capped at the remaining `exec` budget (302 ms in the regression test). See [markkit/fetchkit builtins](runtimes/markkit-fetchkit-builtins.md), TM-DOS-143.
56
* **`json.dumps` indent preflight**: a security audit found `json.dumps([1], indent=10**12)` (and huge string indents or separators on deep or long data) allocated past the Python memory limit unchecked; the metered allocator cannot refuse, so one tenant aborted the process. The Monty fork now preflights the indent repeat and every separator/indent write, raising `MemoryError`. See [Threat Model](security/threat-model.md), TM-PY-038.
67
* **Re-entry depth and on-demand stack growth**: a security audit found three ways one tenant could overflow the native stack and abort the shared process: `f(){ eval f; }` and `compgen -C` (plus `fc` and PROMPT_COMMAND) re-entered the interpreter with no depth count; `./s.sh` running itself skipped the `sh s.sh` child-shell guard; and the product of capped nesting kinds (compounds x functions x eval) still exceeded a 2 MiB release worker stack. Re-entry now counts against `max_function_depth` and `MAX_REENTRY_DEPTH` (8), path-invoked scripts take the child-shell guard, and `execute_command` grows the stack on the heap through `stacker` (already in the graph via Monty; not on wasm32). See [Threat Model](security/threat-model.md), TM-DOS-125, TM-DOS-141, TM-DOS-142.
78
* **Real mount hardening**: a security audit found `chmod 4755` on a read-write real mount set setuid on the host file, and a host dir mounted at `/` lost the configured `filesystem_limits` (the root overlay used defaults). `RealFs::chmod` now passes only `mode & 0o777`, and the root overlay keeps the base limits. See [Threat Model](security/threat-model.md), TM-ESC-010, TM-DOS-144.

‎knowledge/runtimes/markkit-fetchkit-builtins.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ The `CliHost` trait is the only boundary:
4848
| file reads (`--file`, `--rules`) | VFS, resolved against `cwd` | n/a |
4949
| env | n/a | shell env (`ctx.env`), never host env |
5050
| URL egress | `HttpClient` per hop, redirects followed here | fetchkit `HttpTransport` over `HttpClient` |
51-
| limits | `--max-*`/`--timeout` may lower, never raise, markkit defaults | bashkit `HttpLimits` cap every body |
51+
| limits | `--max-*`/`--timeout` may lower, never raise, markkit defaults; `timeout` also capped at the remaining `exec` deadline | bashkit `HttpLimits` cap every body; each request's timeout capped at the remaining `exec` deadline |
52+
| scheduling | runs on tokio's blocking pool (`Handle::block_on`), so the `exec` timeout cancels the caller and co-tenants keep the async worker (TM-DOS-143) | same |
5253
| `mcp` subcommand | refused | refused |
5354

5455
### Network

‎knowledge/security/threat-model.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ on default 2 MiB test threads (TM-DOS-044).
314314
| TM-DOS-138 | Join matching-row output growth | Equal-key row groups multiply generated output and synchronous work before dispatcher capture | Parsed rows, fields, keys, decoded input and generated output own shared-budget capacity before growth. VFS reads charge aggregate input and admit metadata size; output fields append directly without temporary row storage, and the byte-stream copy is admitted. Parsing, grouping (including suppressed matches), comparisons and emissions charge work and poll cancellation/deadlines. Errors abort the request. Ordinary two-row fixtures cover memory/work rejection, cancellation/deadlines, session recovery, and complete pipeline/redirect output | **MITIGATED** |
315315
| TM-DOS-139 | Embedded CLI limit escalation | `markkit extract x --max-input-size 99999999999 --timeout 1e9` buys a larger CPU/memory budget in a shared process | Host clamps every markkit limit to its default after flags (`CliHost::limits`); fetchkit bodies capped by `HttpLimits`; OCR (host binaries) refused | **MITIGATED** |
316316
| TM-DOS-140 | Tree-walk memory on huge trees | A tenant mounts or creates a tree with millions of files and runs `find`, `du` or `rg` over it; a bulk prefetch of every listing, or rg collecting every file's text before charging the input budget, holds the whole tree in memory | `FileSystem::walk` answers are bounded batches (`WalkOptions::max_entries`, default 10k entries; a listing is never split); `find`/`du` refill through `builtins/walk_cache.rs` one batch at a time, consume listings as they descend, and fall back to `read_dir` if an answer does not hold the requested directory; rg searches each file as it is read and drops its text, charges it to the aggregate-input budget and stops at the first refusal, and in path order its walk reads files as it finds them, one root after another (no candidate list); `ls` (recursive or one wide directory) stops appending lines at `max_stdout_bytes`. Heap peaks over a 200k/2M-file procedural tree are pinned by `large_tree_stress_tests` (walkers under 8 MiB). Residuals: a non-path `--sort`, `--files`, or nested roots still build the full candidate list (paths + metadata per file, no budget charge) before reading; one wide directory is held whole by `read_dir`. Regressions: `walk_cache::tests::batches_stay_bounded_and_cover_every_directory`, `remote_fs_round_trip_tests::walk_batches_are_bounded_*`, `rg_stops_reading_at_input_budget` | **MITIGATED** |
317+
| TM-DOS-143 | Document extraction outlives the exec deadline | `markkit extract` on a large document (or `fetchkit` converting a large page) ran synchronously on the tenant's async worker: the `exec` timeout could not interrupt it and co-tenants scheduled on that worker stalled, up to markkit's 60 s default | Both builtins run their CLI on tokio's blocking pool, so the caller's future stays cancellable and the worker free; markkit's cooperative `timeout` and fetchkit's per-request timeout are capped at the remaining `exec` budget. Residual: abandoned extraction keeps a blocking-pool thread until markkit's own (coarse) deadline check fires. fetchkit request timeouts have the HTTP client's 1 s floor, so an abandoned request can outlive a sub-second remaining budget by up to 1 s. Regression: `extraction_stops_at_exec_deadline` | **MITIGATED** |
317318
| TM-DOS-141 | Re-entry recursion via eval/compgen/fc | `f(){ eval f; }`, `compgen -C f`, `fc` re-exec, or PROMPT_COMMAND re-enter the interpreter without a function frame, so recursion was bounded only by the native stack and aborted the shared process | Each re-entry counts against `max_function_depth` and a separate `MAX_REENTRY_DEPTH` (8); over the cap the command fails `maximum nesting depth exceeded` (exit 2). Regressions: `recursive_eval_is_bounded`, `recursive_compgen_command_is_bounded` | **MITIGATED** |
318319
| TM-DOS-142 | Product of nesting caps overflows worker stack | Each nesting kind has its own cap, but their product (compound nesting x functions x eval x child shells) can exceed a 2 MiB tokio worker stack in release builds | `execute_command` polls through `interpreter/grow_stack.rs`, which uses `stacker::maybe_grow` (512 KiB red zone, 4 MiB segments) so deep but capped nesting extends the stack on the heap instead of aborting. Not on wasm32 (single-threaded, host stack). Regression: `nested_compounds_inside_recursion_do_not_overflow` | **MITIGATED** |
319320
| TM-DOS-144 | Root real mount drops FsLimits | `mount_real_readonly(dir)` / `mount_real_readwrite(dir)` at `/` wrapped the host dir in `OverlayFs::new`, whose upper layer used default limits, so a tenant could write past the configured `filesystem_limits` | The root overlay is built with `OverlayFs::with_limits` and the base filesystem's limits. Regression: `root_real_mount_keeps_filesystem_limits` | **MITIGATED** |

0 commit comments

Comments
 (0)