audit: blocking operations with no timeout in fbuild-packages (sub-issue of #802)

[zackees]

Sub-issue of #802. Area audited: crates/fbuild-packages/ — URL-based package downloads (toolchains, frameworks, libraries from GitHub + PlatformIO registry), archive extraction, install-lock arbitration, and per-platform framework downloaders.

Intro

fbuild-packages is the dominant network surface in the workspace. Every cache-miss for a toolchain (xtensa-esp-elf, arm-none-eabi-gcc, avr-gcc, riscv32-unknown-elf, clang, esp-qemu, ...), every framework download (ESP32 Arduino, ESP8266, ArduinoCore-API submodule), and every library resolved against api.registry.platformio.org flows through this crate. Downloads are tracked toward dl.registry.platformio.org, github.com/<org>/archive/refs/..., and dl.espressif.com — all of which periodically degrade or rate-limit. The crate's downloader.rs does have an attempt-level retry loop (MAX_ATTEMPTS=3 with 1s/3s backoff), but the per-request HTTP client is reqwest::get / reqwest::blocking::get, which constructs an anonymous default client with NO connect timeout, NO read timeout, NO total timeout. A slow CDN can hang a single attempt indefinitely; the retry loop never fires because the prior attempt never returns.

Combined with the install-lock waiter (250 ms poll, 2-hour stale guard) downstream of the download, a single stuck download on a first-build behind a flaky network can wedge the entire toolchain install for hours before the stale-lock recovery path triggers. This is exactly the daemon-side hang vector #802 is hunting.

Findings

Severity	File:line	Operation	Why unbounded	Suggested fix
CRITICAL	crates/fbuild-packages/src/downloader.rs:113	reqwest::get(url).await (async) — primary toolchain/framework/library downloader	reqwest::get builds a default Client with no connect/read/total timeout. A stalled CDN socket hangs forever and the retry loop above it never fires. Hit on every cache miss (toolchains, frameworks, libraries).	Build a module-level OnceCell<reqwest::Client> with Client::builder().connect_timeout(Duration::from_secs(15)).timeout(Duration::from_secs(300)).build() and route both get_with_retry and open_with_retry through it. For the streaming path use a separate client without .timeout() but with read_timeout so the chunk loop self-trips on stalls.
CRITICAL	crates/fbuild-packages/src/downloader.rs:178	reqwest::get(url).await (async, streaming progress variant open_with_retry)	Same default-client-no-timeout bug. Worse here: once the response opens, chunk().await at L253 has no read timeout — a stalled mid-download will hang the entire toolchain install.	Use the shared Client above with a read_timeout (reqwest 0.12 supports per-request .timeout() and a body read timeout via ClientBuilder::read_timeout). Alternative: wrap the while let Some(chunk) = stream.chunk().await in tokio::time::timeout(Duration::from_secs(60), stream.chunk()).await so a stuck socket fails the attempt; lift the retry loop in open_with_retry to cover body reads too.
CRITICAL	crates/fbuild-packages/src/downloader.rs:253	stream.chunk().await reads a streaming body chunk-by-chunk inside download_file_with_progress	The 15-second progress callback gives the operator a sign of life, but the await itself has no deadline — if the remote stops sending bytes, this future never wakes.	Replace with match tokio::time::timeout(Duration::from_secs(60), stream.chunk()).await { Ok(Ok(Some(chunk))) => ..., Ok(Ok(None)) => break, Ok(Err(e)) => return Err(...), Err(_) => return Err(PackageError("body read stalled > 60s")) }.
HIGH	crates/fbuild-packages/src/library/arduino_api.rs:51	reqwest::blocking::get(ARDUINO_API_URL) — fetches ArduinoCore-API submodule tarball synchronously from GitHub	reqwest::blocking::get also uses a default client with no timeout. Called from Arduino-core install paths (megaavr, renesas) — typically inside the install thread, but still on a CRITICAL surface (first build for those boards).	Replace with a lazy reqwest::blocking::Client built via Client::builder().connect_timeout(...).timeout(Duration::from_secs(120)).build(). Same module-level OnceCell pattern.
HIGH	crates/fbuild-packages/src/library/arduino_api.rs:63	response.bytes() (blocking) reading entire GitHub archive body	Inherits the no-timeout client. Body read can stall after a successful 200 response.	Covered by the client-level .timeout() once the Client above is wired up — .timeout() on the blocking client is a total per-request deadline including body read.
HIGH	crates/fbuild-packages/src/toolchain/clang.rs:147	reqwest::get(&url).await fetching the clang manifest from Espressif's S3 bucket	No timeout. Failure mode: clang toolchain (used by clang-tidy/iwyu) hangs on first use; daemon thread blocked.	Route through the shared client introduced for downloader.rs.
HIGH	crates/fbuild-packages/src/toolchain/clang.rs:163	resp.json().await parses the manifest body	The body read after a successful response status has no deadline.	Covered by the per-request .timeout() once the shared client is in use.
HIGH	crates/fbuild-packages/src/library/registry.rs:24	reqwest::get(&url).await — search_library on api.registry.platformio.org/v3/search	No timeout. Library resolution is on the critical-path of every first build that uses a registry library.	Use the shared client.
HIGH	crates/fbuild-packages/src/library/registry.rs:32	response.json().await — registry search body parse	Body-read tail of the same call; no read deadline.	Covered by shared client's .timeout().
HIGH	crates/fbuild-packages/src/library/registry.rs:77	reqwest::get(&url).await — resolve_library GET v3/packages/{owner}/library/{name}	No timeout. Same risk profile as search_library.	Use shared client.
HIGH	crates/fbuild-packages/src/library/registry.rs:89	response.json().await — package-details JSON body	Body-read tail.	Covered by shared client's .timeout().
HIGH	crates/fbuild-packages/src/library/registry.rs:189	reqwest::get(&url).await — resolve_library_via_search fallback	No timeout.	Use shared client.
HIGH	crates/fbuild-packages/src/library/registry.rs:196	response.json().await — fallback search body	Body-read tail.	Covered by shared client's .timeout().
MEDIUM	crates/fbuild-packages/src/install_lock.rs:46-119	acquire_install_lock_at poll-loop waiting for a peer install to finish	No wall-clock guard for the waiter. The only escape is INSTALL_LOCK_STALE_AFTER = 2 * 60 * 60 = 7200 s, checked against the lock directory's mtime. If a peer holds the lock while wedged on a no-timeout reqwest::get (see CRITICAL findings above), every other process waits up to 2 hours before recovery kicks in — perceived as a daemon hang.	Once the CRITICAL reqwest fixes land the 2 h ceiling matches the worst-case legit install, but expose a shorter ceiling via env var for CI (where 2 h is greater than the job timeout). Also publish wait-elapsed in the install-status payload so the daemon can surface it.
LOW	crates/fbuild-packages/src/toolchain/avr.rs:365	reqwest::get(&url).await inside #[test] #[ignore] fn test_download_and_verify_checksum	Test-only (#[ignore]), exercised manually. Already inside a tokio::runtime::Runtime::new() block.	No fix required; listed for completeness so a future auditor doesn't re-flag it.

Notes on the workspace-level reqwest config

Workspace Cargo.toml pins reqwest = { version = "0.12", default-features = false, features = ["json", "blocking", "stream", "rustls-tls"] }. There is no shared client construction helper — every site calls the bare reqwest::get / reqwest::blocking::get free functions. The fix is one shared helper module (e.g. fbuild-packages/src/http_client.rs) exposing pub fn shared_async() -> &'static reqwest::Client and pub fn shared_blocking() -> &'static reqwest::blocking::Client, both built with timeouts and User-Agent: fbuild/<version>. Re-export from lib.rs so the toolchain and library modules can drop their reqwest::get calls without churning every call site shape.

What was searched

Grep on crates/fbuild-packages/src/:

• reqwest:: — every call site (caught the 9 reqwest::get / reqwest::blocking::get callers above)
• Client::default / Client::builder / ClientBuilder — zero hits (no shared client anywhere)
• \.timeout\( — zero hits in the crate
• \.bytes\(\) / \.text\(\) / \.copy_to\( / bytes_stream / \.chunk\(\) — to find streaming body reads
• flate2:: / tar:: / zip:: / xz2:: / bzip2:: / zstd:: / Archive::new / GzDecoder|XzDecoder|BzDecoder|ZstdDecoder — confirmed extraction reads from on-disk file, not from a network stream (extract happens after download_file has written the full body)
• Command::new / \.output\(\) / \.wait\(\) / \.spawn\(\) / \.status\(\) — no subprocess invocations in fbuild-packages (extraction is pure-Rust)
• JoinHandle / \.join\(\) — one hit in library_compiler.rs (rayon scope joining local compile workers, not a network surface)
• \.recv\(\) / recv_timeout / mpsc:: / crossbeam — no matches (no channel-based pipeline lives in this crate; the parallel install pipeline audit: blocking operations with no timeout (meta) #802 refers to lives in fbuild-build)
• loop { / thread::sleep / std::thread::sleep — caught the install_lock poll loop (above) and the per-worker compile loop (local CPU work, not blocking I/O)
• \.head\( / Method::HEAD / reqwest::Client — no HEAD probes anywhere

Out-of-scope notes

• Pipeline / dispatcher — the meta-issue mentions "parallel pipeline (pipeline::* module)". There is no pipeline module in fbuild-packages; the orchestrating pipeline lives in fbuild-build. That belongs to a separate sub-issue.
• disk_cache/ — GC and lease management are purely local fs operations, no network or unbounded waits.
• lnk/ — link-time descriptor materialization. Local fs / zip work; no network surface beyond delegating downloads back through downloader.rs (already covered).
• Per-platform framework downloaders (esp32_framework/, avr_framework.rs, cmsis_framework.rs, esp32_platform.rs, etc.) — all delegate to crate::downloader::download_file{,_with_progress}. Fixing the shared client in downloader.rs fixes all of them transitively.
• extractor.rs — reads from std::fs::File, not from a network stream. The download-to-disk-then-extract sequence is the correct shape; no finding here.
• library_compiler.rs:259 handle.join().unwrap() — joins rayon worker threads running local C/C++ compiles. Not a network surface; covered by per-process timeouts that belong in fbuild-build.

Tagging #802.

[zackees]

Closed by PR #843 (workspace-wide timeout sweep).