fix(cli): harden source-fetch SSRF redirects + size cap (SP42#43)#51
Merged
Conversation
CliHttpClient checked only the initial URL and relied on reqwest::redirect::Policy::limited(5), so a public-looking citation host could 302-redirect to loopback/private/link-local infrastructure and be fetched. The 8 MB cap was also enforced only via Content-Length, so a chunked / no-length response was read unbounded with `.bytes()`. - Disable auto-redirects (Policy::none()) and follow them in a manual loop, re-running check_fetchable_source_url() on every hop's destination before connecting (capped at MAX_SOURCE_REDIRECTS=5). - Read the body via Response::chunk(), aborting as soon as the accumulated bytes would exceed the cap — covering chunked / no-Content-Length bodies. Content-Length fast-reject is kept for honest servers. - GET-only and default-deny private/loopback behavior preserved. Cap is now a CliHttpClient field (prod = MAX_SOURCE_BYTES) so tests inject a small one. Tests (raw loopback servers + reqwest .resolve() so a floor-passing host can reach them): redirect-to-private is blocked, an oversized chunked body is rejected, and an allowed redirect is followed and its body read. Closes #43 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 25, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The source-fetch boundary had two gaps once citation URLs come from (attacker-influenceable) wiki content:
Content-Length, so a chunked / no-length body was read unbounded.This PR:
Policy::none()) and follows them in a manual loop, re-runningcheck_fetchable_source_url()on every hop's destination before connecting (max 5 hops).Response::chunk(), aborting as soon as accumulated bytes would exceed the cap — covering chunked / no-Content-Lengthresponses. TheContent-Lengthfast-reject is kept for honest servers.CliHttpClientfield (prod =MAX_SOURCE_BYTES) so tests can inject a small value.Tests use raw loopback servers + reqwest
.resolve()(so a floor-passing hostname can reach them while the floor stays active): redirect-to-private is blocked, an oversized chunked body is rejected, and an allowed redirect is followed and its body read.reqwest'sstreamfeature is not enabled, so the cap usesResponse::chunk()rather thanbytes_stream()— no new feature flags.Closes #43
🤖 Generated with Claude Code