fix(security): guard redirects against SSRF and bound decompression in the parsers

[runyourempire]

Two defects on the untrusted-input path — data fetched from remote feeds, and files opened from the user's disk.

Defect 1 (HIGH) — the SSRF guard was pre-flight only

url_validation::is_internal_url is a good implementation wired at ten production sites. But no client in the codebase configured a redirect::Policy, so all 21 production ClientBuilders inherited reqwest's default Policy::limited(10).

A hostile RSS feed (user-addable) or a hijacked curated-feed domain answering 302 Location: http://127.0.0.1:4446/api/dna was followed, the pre-flight check never re-ran, and the internal body was stored as feed content (content_enrichment.rs).

The fix

http_client.rs gains two policies:

Policy	Behaviour	Applied to
ssrf_guarded_redirect_policy()	refuses any hop to an internal address	10 clients: HTTP_CLIENT, PROBE_CLIENT, TEAM_CLIENT, client_builder_with_proxy, crates.io, OSV cache + sync, model registry, translation, settings validation, team-sync scheduler, webhooks, fastembed download, live reality check
local_aware_redirect_policy()	refuses an internal hop unless the request already started internal	11 clients: LLMClient, Ollama (x2), ollama_capability, embeddings, local-server probes (x3), calibration check, dev-frontend gate

The local-aware variant exists so a user who deliberately points 4DA at http://127.0.0.1:11434 keeps working, while a cloud provider still cannot redirect its way inward. It mirrors the existing if self.provider.provider != "ollama" exemption at llm.rs:492.

Policy::custom replaces reqwest's hop limit wholesale, so the policy counts hops itself — without that a redirect loop never terminates. There is a test for exactly that.

Secondary: the hand-rolled URL parser

extract_host split on / ? # : but never on @, so http://evil.com@127.0.0.1/ produced a host of evil.com@127.0.0.1, failed parse::<IpAddr>(), fell through a failing DNS lookup, and returned "not internal". It also missed the WHATWG-legal obfuscated literals (http://2130706433/, http://0x7f000001/, http://127.1/, http://0177.0.0.1/).

Host extraction now goes through the url crate — the same parser reqwest uses, so there is no parser differential between validation time and request time. Credentials-in-URL are rejected outright, matching what ipc_guard::validate_url_safe_for_request already did.

Correction to the brief

The report suggested rss.rs:483 / hackernews.rs:301 would "inherit the protection" once the redirect policy landed. They would not. Those two fetch feed-supplied URLs guarded only by starts_with("http") — a direct SSRF where the first request is not a redirect, so no redirect policy can see it. Both now pre-flight through validate_not_internal.

Defect 2 (HIGH) — decompression bombs in default-reachable parsers

extractors/office.rs (default-build reachable). The 100 MB cap was on the compressed file, with no decompressed bound, entry count, or ratio check. A 100 MB DOCX of low-entropy XML expands to ~100 GB, buffered by read_docx/calamine before we ever see a parse result. guard_ooxml_bomb now streams every part through io::sink() — decompression CPU, no allocation — and counts the bytes that actually come out, bounding total size (250 MB), ratio (200:1), and part count (10,000).

extractors/archive.rs (feature-gated). All three ZIP guards read file.size() — the header-declared size, which the archive's author controls. zip 0.6 builds the DEFLATE reader as DeflateDecoder::new(take(compressed_size)) (read.rs:277), so the declared size bounds nothing: declare 1 KB, ship 45 MB. Sizes and ratios now come from capped reads.

TAR gains the input cap it never had, a Take on the decompressed stream (a .tar.gz had no bound at all), and an entry-scan counter — MAX_FILE_COUNT only incremented after a successful read, so an archive of skippable entries looped once per entry on the watcher thread while holding the DB mutex.

osv/cache.rs. The ecosystem download called .bytes() — unbounded in-memory buffering of a several-hundred-MB body — and read_to_string per entry. Now streams to disk under a 1 GiB cap and reads each advisory under 8 MiB.

Zip-slip is untouched — enclosed_name() and the ParentDir rejection were already correct, and nothing is written to disk.

Every new test was proven to fail against the pre-fix code

The existing extractor "integration" tests were decorative: #[ignore]d and self-neutering, each wrapping its only assertion in if test_docx.exists() against a temp path nothing creates. Those two are deleted. New fixtures are built programmatically at test time — no binaries committed.

Each guard was reverted in place and the tests re-run. Observed failures:

Test	Failure against pre-fix code
blocks_userinfo_masking_loopback	assertion failed: is_internal_url("http://evil.com@127.0.0.1/")
blocks_obfuscated_ipv4_literals	assertion failed: is_internal_url("http://2130706433/")
validate_not_internal_rejects_credentials	no error raised
shared_http_client_refuses_redirect_to_loopback	HTTP_CLIENT followed the hop: Ok(200)
probe_and_team_clients_refuse_redirect_to_loopback	PROBE_CLIENT followed the hop
docx_decompression_bomb_is_refused_on_size	got: Failed to parse DOCX structure — i.e. after inflating
docx_decompression_bomb_is_refused_on_ratio	same
xlsx_decompression_bomb_is_refused	got: Failed to open Excel workbook — after inflating
ooxml_entry_count_bomb_is_refused	same
zip_entry_lying_about_its_size_is_refused	bomb payload reached the output (47185977 bytes of text)
targz_decompressed_stream_is_bounded	the walk ran past the extraction budget to reach the trailing entry
tar_input_over_compressed_cap_is_refused	got: No extractable text content found in archive

The redirect tests drive a real loopback HTTP fixture serving a real 302 — reqwest::redirect::Attempt cannot be constructed outside its crate, so a live redirect is the only way to exercise a policy. A companion test asserts the stock reqwest policy does follow the hop, so the fixture cannot silently rot into proving nothing.

The ZIP bomb fixture forges the declared uncompressed size at known byte offsets in both the local file header (+22) and the central directory record (+24) — which is exactly what an attacker does by hand.

legitimate_docx_still_extracts is the anti-false-positive test: a genuine DOCX built with docx-rs must still parse. Guards that reject everything are not guards.

Verification

• cargo clippy --lib -- -D warnings — clean, default features
• cargo clippy --lib --features experimental -- -D warnings — clean
• cargo clippy --lib --features archive -- -D warnings — clean (archive.rs only compiles under this feature)
• cargo fmt --check — clean
• cargo test --lib — 4356 passed, 0 failed, 8 ignored
• cargo test --lib --features archive — 4367 passed, 0 failed, 8 ignored
• Integration tests — 154 passed (migration 12, pipeline 13, source_resilience 5, stack_simulation 124)

Rebased onto 8c60321b, so this is verified against #433's calamine 0.36 / docx-rs 0.4.22. office.rs needed no API changes for that bump; the rebase applied with no conflicts.

The rebase caught one real flake: the ratio fixture at deflate level 1 landed at ~199:1 against a 200:1 threshold — sitting on its own boundary. It now builds at level 9 (~1000:1) and asserts the margin up front, so a future flate2 bump fails with "fixture is no longer a convincing bomb" rather than a confusing parser error.

Note

src-tauri/src/osv/cache.rs crosses the 700-line soft warning (726). Warning only, non-blocking; splitting that module is a separate refactor.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUeKTKwNmdow8yUk3q8RB2