Repository navigation
Read/fork handlers run blocking git subprocesses on the tokio runtime (no spawn_blocking, no timeout) #204
Description
Activity
- addedcrate:nodegitlawb-node — the serving node and REST APIgitlawb-node — the serving node and REST APIkind:securityVulnerability fix or hardeningVulnerability fix or hardeningsev:highMajor break or real security/trust risk, no easy workaroundMajor break or real security/trust risk, no easy workaroundsubsystem:apiNode REST API request/response surfaceNode REST API request/response surfacesubsystem:storageBlob/object store, Arweave, IPFS, archivesBlob/object store, Arweave, IPFS, archives
on Jul 15, 2026 A third site, on the background path rather than a request handler, so it escapes the
git_service_timeout_secsframing above in a way worth noting separately.encrypt_and_pinispub async fn(crates/gitlawb-node/src/encrypted_pin.rs:109) and calls the
synchronous read bare inside its loop (:155):let data = match crate::git::store::read_object(repo_path, oid) {
No
spawn_blockingand noblock_in_placeanywhere in the file.read_object
(crates/gitlawb-node/src/git/store.rs:681-688) composesobject_typeandread_object_content,
both plainstd::process::Command::output()with no deadline and no process-group reaping
(store.rs:277-281,store.rs:309-313). The bounded twin's own docstring states the rule this
breaks (store.rs:643-645): "This is SYNCHRONOUS blocking work ... Async callers must run it under
tokio::task::spawn_blocking... calling it directly from a runtime task blocks a worker."What makes it worth adding here rather than leaving to a later sweep is that it is residue from a fix
that already named it. Commit03dde22e(#174) touchedencrypted_pin.rsby exactly one line,
boundingpin_git_objectonly, and its message says: "Not bounded: the git read, since
store::read_object shells two bare Command::output() calls with no timeout or process-group reaping
and this loop does not run it under spawn_blocking." Both sibling pin loops got the bounded read
afterwards (ipfs_pin.rs:299-306,pinata.rs:166-173, eachspawn_blocking(|| read_object_bounded(..., read_deadline))).
This loop did not.Reachability differs from the sites above. Post-receive only:
run_encrypt_pin_taskinto
pin_and_encrypt_objects(repos.rs:1181) intoencrypt_and_pin(repos.rs:1224), gated on the repo
carrying a path-scoped rule with withheld blobs. So it needs push rights plus rule setup, though on an
open node any registered user can arrange both.The impact shape is also different, and is why the request-path framing does not cover it: the push
response has already returned, so there is no slow request to observe. Per withheld blob, two blocking
git cat-filechildren hold a runtime worker for their full duration with no ceiling.EncryptInflight
caps one task per repo, so concurrency across repos is bounded by nothing, and N concurrent repos hold
N workers, which starves the shared runtime including/healthand/readyexactly as described
above.Fix is the same shape as the one already proposed here, with one wrinkle:
encrypt_and_pincurrently
takes neither agit_binnor a deadline, so both have to be threaded in fromEncryptTaskCtxbefore
it can callread_object_bounded.Not verified by execution; the regression test needs a live Postgres and a full node build. The static
path is unambiguous, since both constituents are visibly untimedCommand::output()and no wrapper
exists in the file.- addedsev:mediumDegraded but workaround existsDegraded but workaround existsand removedsev:highMajor break or real security/trust risk, no easy workaroundMajor break or real security/trust risk, no easy workaround
on Sep 5, 2026 - added 2 commits that reference this issue
on Sep 9, 2026 - added 2 commits that reference this issue
on Sep 20, 2026
Surfaced during review of #196 (pre-existing;
git/store.rsand the fork clone are untouched by that PR's diff, so splitting here rather than widening #196's scope).fork_repo —
crates/gitlawb-node/src/api/repos.rs:1567runsstd::process::Command::new("git").args(["clone","--mirror",…]).output()inside the async handler.output()blocks the worker thread; agit clone --mirrorof a non-trivial repo can take minutes and there is no timeout, unlike other git walks in this file that usetokio::task::spawn_blocking(repos.rs:67,110,661,1143) ortokio::process::Command.Read handlers —
list_commits/get_tree/get_blobcallstore::{resolve_head,log,ls_tree,read_file}(api/repos.rs:360-361,397,443,481), which are synchronousstd::process::Command::output()calls (git/store.rs:92-231). A slow disk or large repo blocks a worker for seconds; with a finite worker pool a handful of concurrent /commits, /tree, /blob, or one /fork can starve the runtime, including /health and /ready.Impact: availability/reliability regression;
git_service_timeout_secsis bypassed on these paths.Fix direction: wrap the blocking store calls (and the fork clone) in
spawn_blockingbounded bytokio::time::timeout(git_service_timeout_secs, …), returningAppError::Timeout. Verified real by execution during the #196 review.