One guarded fetch path, and it checks where it lands - #339
Merged
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: WaylandYang <wayland0916@gmail.com>
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
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.
Closes #330.
ingest_sources.rsbuilt a freshreqwest::Clientin six places, each with a timeout and a user agent and nothing else. None of them asked where the request was going,reqwestfollowed up to ten redirects on its own, andfetch_pageended inresp.bytes()with no cap.Who chose the address decides how strict it is
Reach::Operator— a person with editor rights typed this host (url,rss,jira,custom,webdav). Private addresses stay reachable, because an internal wiki is what this product is for;sync_customalready had a loopback branch that turned the proxy off, and that judgement is now in one place instead of one source kind.Reach::Content— the address came out of fetched content. Public only. Nothing uses it from a feed yet;api.github.comuses it because a fixed public host has no reason to be looser, and Add guarded full-content hydration for new RSS items #326's article fetching is the case it was written for.Both get: one resolution whose every address is checked,
resolve_to_addrspinning those addresses so a second lookup cannot move the target, redirects walked by hand with each hop re-checked, no https→http downgrade, a byte cap on the body, and a deadline across the whole trip.A redirect may not widen the reach: if a hop landed on a public address, the next one may not land inside the network. The operator chose that host, not its willingness to forward.
The address list
is_public_iprefuses loopback, private, link-local, CGNAT, benchmark, documentation, multicast and unspecified ranges, and unwraps v4-mapped, NAT64 and 6to4 addresses before judging what is inside. It deliberately does not treat all of192.0.0.0/16as reserved — only192.0.0.0/24and192.0.2.0/24are, and over-blocking is a site a user cannot reach.Verified
Unit tests over the address table, both directions, including the wrapped forms (
::ffff:127.0.0.1,64:ff9b::7f00:1,2002:7f00:1::) and the two addresses that must stay allowed inside otherwise-reserved space. Three tests against a realwiremockserver: a loopback host is refused forContentand fetched forOperator— the same URL, and the whole distinction in one assertion; a body over the cap is refused where the old code read it all; a redirect loop ends at the limit instead of being followed by the library.cargo fmt --check,cargo clippy --all-targetsclean, 150 server tests green.The shape comes from the fetcher @J-i-K wrote for #326 — this lifts it out to a path every source shares, which should shrink that PR rather than compete with it.
🤖 Generated with Claude Code