Skip to content

Refuse a fetched name that resolves to a non-public address - #1846

Merged
bbondy merged 1 commit into
brave:mainfrom
netzenbot:check-fetch-resolved-address
Oct 9, 2026
Merged

bbondy merged 1 commit into
brave:mainfrom
netzenbot:check-fetch-resolved-address

Conversation

@netzenbot

Copy link
Copy Markdown
Collaborator

Closes #1451

User impact: a fetch_url whose host is a name that resolves to this machine, a private network or a cloud metadata address now fails instead of fetching it.

The problem

A person approves a fetch by reading the host in the URL, and bravebot checked only that. A name can be made to resolve to this machine, to a router or to a cloud host's credentials service, and the fetch went ahead with no warning, after a prompt that showed nothing unusual.

Reproduce

  1. In the terminal client, in a trusted directory, start a session with a model that can call fetch_url.
  2. Run something listening on loopback, for example python3 -m http.server 8765.
  3. Type fetch http://localhost:8765/ and tell me what it is, and answer yes when asked to approve the host localhost.

Before: the request reaches the listener on 127.0.0.1 and its body is fetched. Expected: the fetch fails, because localhost is a name that resolves to this machine, and the listener sees no request. Which tool calls the model makes is its choice, so this was not driven through a live model; the tests below script the call and observe the same thing at the listener.

a_fetch_to_a_name_that_resolves_to_this_machine_fails_without_reaching_it (agent turn) and a_fetched_name_that_resolves_to_this_machine_is_refused_without_a_connection (real DNS, localhost, a real listener) fail on the parent commit and pass here.

The fix

While a fetch is in flight, the client now resolves each hop's host through a resolver that refuses the hop if any address in the answer is loopback, private, link-local (the metadata address included), unique-local, carrier-grade NAT, unspecified, multicast or reserved. The HTTP client connects only to what its resolver returns, so the address that was classified is the one connected to and there is no second lookup to differ. The failure names the URL asked for, as every fetch failure does, and the address never leaves the crate.

A host the person approved that is itself an address (http://127.0.0.1:8765/) is fetched; localhost and forms like 2130706433 are names. Only fetch_url is affected: the model endpoint and other requests still reach loopback. Behind a proxy the proxy resolves the target, so a proxied fetch is not classified (stated as a known cost in the new clause).

$ cargo test -p bravebot-net --locked a_fetched_name_that_resolves
test a_fetched_name_that_resolves_to_this_machine_is_refused_without_a_connection ... ok
Mutation evidence: for each behaviour, the fault restored and the test that failed

All demonstrated unless marked reasoned: restore the fault, watch the named test fail for that reason, restore the tree, re-run.

Fault Failing test
link-local treated as public every_class_the_issue_names_is_not_public, a_name_resolving_to_a_non_public_address_is_refused_before_any_request_is_sent, redirect-hop test
only the first returned address classified one_non_public_address_among_public_ones_refuses_the_name
fetch uses the unguarded client turn test, egress a_fetched_name_that_resolves..., a_name_resolving...
guard applied to every request every_redirect_hop_is_revalidated, a_request_that_is_not_a_fetch_may_still_resolve_to_this_machine, the_same_name_is_reached_when_no_fetch_is_in_flight
no exemption for an address written as the host an_approved_host_that_is_itself_an_address_is_fetched, an_approved_address_literal_on_this_machine_is_fetched
second lookup after classifying the_connection_is_made_to_the_address_that_was_classified
only the first hop guarded redirect-hop test
refusal reported as a transport error egress test, a_name_resolving..., redirect-hop test
fetch_in_flight always false policy test, turn test, egress test
proxy ignored a_proxied_fetch_is_not_sent_through_the_guarded_lookup
IPv4-mapped / NAT64 not unwrapped an_ipv4_address_in_ipv6_clothes_is_classified_as_the_ipv4_address
CGNAT, unique-local or private ranges public every_class_the_issue_names_is_not_public

Reasoned only: a_refused_address_is_a_failure_that_says_nothing_of_the_address_and_is_not_retried, the "names the URL asked for" rewrite in the redirect test, and the fec0::/10 range (covered by one list entry added after the mutation run). Not handled: 6to4 2002::/16.

Strings left in the source

The two new EgressError Display strings are diagnostic text like the existing variants of that type; bravebot-net has no catalog dependency.

Test plan

  • cargo test -p bravebot-net --locked - 58 lib and 25 integration tests passed
  • cargo test -p bravebot-core --locked policy test and cargo test -p bravebot-agent --test turn fetch test - passed
  • cargo fmt --all --check, cargo clippy --all-targets --all-features -- -D warnings - clean
  • make check-affected (spec, security, narration, locales, versions, reviewdog, ui, docs) - passed; its cargo test stops at bravebot-lsp live_ts, which fails on this host (TypeScript 7 ships no tsserver.js; no lsp file is touched)
  • cargo test --all --locked --no-fail-fast - only live_ts failed, plus session jobs::tests::a_second_process_cannot_claim_a_live_entry, a load flake that passes alone
  • rustup run 1.90.0 cargo build --all --locked - passed (stand-in for the MSRV job)
  • check-msrv, check-windows, check-linux - not run: Docker Desktop was unresponsive (docker info silent)
  • CI passes cleanly

@netzenbot netzenbot self-assigned this Oct 9, 2026

@netzenbot-reviewer netzenbot-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

What this pull request does

This change makes fetch_url refuse a hop whose host name resolves to a non-public address. While a fetch is in flight, bravebot-net sends the request through a second ureq agent whose resolver classifies every address in the answer. It refuses the hop if any address is loopback, private, link-local (including the metadata address), unique-local, carrier-grade NAT, unspecified, multicast or reserved, and it unwraps IPv4-mapped and NAT64 forms first. The connection uses only the addresses that were classified, so there is no second lookup. A host written as an IP literal is still fetched, localhost is treated as a name and refused, and requests that are not fetches (such as the model endpoint) and proxied fetches keep the ordinary agent. A new EgressError::AddressRefused carries only the URL asked for, is not retried, and is mapped to the Blocked category. Policy::fetch_in_flight is added to scope the check, and a new spec clause FETCH-8 and a docs paragraph describe it. The diff matches the description.

How this review reached its recommendation

The review read 10 changed files.

What it checked:

  • Written best practices. The changes were compared with 14 rules from this project's best-practice documents (dependencies, paths, shared-implementation, specs, tests, writing).
  • Bugs. The changes were read for mistakes that would make the code misbehave or stop it building.
  • This project's own criteria. The changes were checked against these questions:
    • Whether the change agrees with the project's specs, and whether a change to behaviour a spec describes also updates that spec and its tests.
    • Whether untrusted content (text from the web, files or tools) can influence a decision, or be given a better trust label than it came with, anywhere other than the places built for that.
    • Whether the tests would fail if the behaviour broke, rather than covering only the allowed case or passing with the bug put back.
    • Whether the change does what its title says and nothing more, with no unrelated refactor and no new dependency, permission or network destination the title does not explain.

None of these checks flagged anything, so there was nothing to double-check.

@netzenbot-reviewer netzenbot-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

What this pull request does

While a fetch_url call is in flight, the network client now resolves each hop's host through a guarded resolver that fails the hop if any returned address is loopback, private, link-local, unique-local, carrier-grade NAT, unspecified, multicast or reserved. IPv4-mapped and NAT64 IPv6 addresses are classified as the embedded IPv4 address. The HTTP client connects only to the addresses the resolver returned, so there is no second lookup. A host written as an IP literal is still fetched, and a name such as localhost is refused with a new AddressRefused error that names the requested URL and not the address; this error is not retried. Requests that are not fetches, such as the model endpoint, keep the unguarded agent, and proxied fetches are not classified, which the new FETCH-8 spec clause records. The diff matches the description.

How this review reached its recommendation

The review read 0 of the 10 changed files: only those that changed since the bot last reviewed this pull request.

None of these checks flagged anything, so there was nothing to double-check.

@netzenbot

Copy link
Copy Markdown
Collaborator Author

@bbondy This PR is approved by netzenbot-reviewer with no findings, mergeable, and CI is green on the current head (d120b6c). Ready for you to merge. Required maintainer review is still outstanding.

@netzenbot
netzenbot force-pushed the check-fetch-resolved-address branch from d120b6c to 3216d9c Compare October 9, 2026 11:41

@netzenbot-reviewer netzenbot-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

What this pull request does

This adds a second HTTP agent in bravebot-net whose resolver classifies every address a fetched host resolves to, and uses it for each hop of a fetch_url call that is in flight and not behind a proxy. If any address in the answer is loopback, private, link-local, unique-local, carrier-grade NAT, unspecified, multicast or reserved (IPv4-mapped and NAT64 addresses are unwrapped first), the hop is not sent and the fetch fails with a new AddressRefused error that names only the requested URL. Hosts written as IP literals are passed through, so http://127.0.0.1:PORT/ still works while localhost is refused. The model endpoint and other non-fetch requests keep the unguarded agent, and proxied fetches are not classified. A new Policy::fetch_in_flight accessor scopes the check, and the FETCH-8 spec clause, the NET-5 cross-reference and the tools reference page are updated. The description matches the diff.

How this review reached its recommendation

The review read 0 of the 10 changed files: only those that changed since the bot last reviewed this pull request.

None of these checks flagged anything, so there was nothing to double-check.

@netzenbot
netzenbot force-pushed the check-fetch-resolved-address branch from 3216d9c to f28d309 Compare October 9, 2026 22:05

@netzenbot-reviewer netzenbot-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

What this pull request does

This adds a check on the addresses a hostname resolves to while a fetch_url call is in flight. Each hop of the fetch, redirects included, is resolved once through a wrapper around ureq's resolver. The hop fails if any returned address is loopback, private, link-local, unique-local, carrier-grade NAT, unspecified, multicast or reserved, with IPv4-mapped and NAT64 addresses unwrapped first. The connection is made only to the addresses that were classified. A host written as an IP literal, such as http://127.0.0.1:8765/, is passed through, while localhost counts as a name and is refused. Only fetch_url uses the guarded agent, so the model endpoint and other requests can still reach loopback, and a proxied fetch is not checked. The failure is a new EgressError::AddressRefused that names the requested URL and not the address, is not retried, and is classed as Blocked. The new FETCH-8 spec clause and the tools reference page document it. The description matches the diff.

How this review reached its recommendation

The review read 0 of the 10 changed files: only those that changed since the bot last reviewed this pull request.

None of these checks flagged anything, so there was nothing to double-check.

A fetch_url host was classified only as written, so a name resolving to
loopback, a private range or the metadata address was fetched. The fetch
now uses a resolver that refuses any non-public answer and is also the
connection's address, so there is no second lookup to differ.

Closes brave#1451
@netzenbot
netzenbot force-pushed the check-fetch-resolved-address branch from f28d309 to c790a8c Compare October 9, 2026 22:28

@netzenbot-reviewer netzenbot-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: approve

What this pull request does

This change makes fetch_url check where a host name resolves to before connecting. While a fetch is in flight, bravebot-net uses a second HTTP agent whose resolver looks up each hop's host once and refuses the hop if any returned address is loopback, private, link-local, unique-local, carrier-grade NAT, unspecified, multicast or reserved. IPv4-mapped and 64:ff9b::/96 addresses are classified as the IPv4 address they carry. The refusal surfaces as a new EgressError::AddressRefused that names only the URL asked for, is not retried, and is reported as a Blocked diagnosis. A host written as an IP literal is still fetched, and requests that are not fetches (such as the model endpoint) and proxied fetches use the original unguarded agent. The description and diff agree, and the PR adds a FETCH-8 spec clause and a docs paragraph.

How this review reached its recommendation

The review read 0 of the 10 changed files: only those that changed since the bot last reviewed this pull request.

None of these checks flagged anything, so there was nothing to double-check.

@bbondy
bbondy merged commit 0450713 into brave:main Oct 9, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Check the address a fetched host resolves to and pin the connection to it

3 participants