Warn about local destinations named by host, not just by address - #707
Merged
kasnder merged 1 commit intoAug 5, 2026
Conversation
isConfigured() classifies the stored configuration, so it only sees addresses: a DoH endpoint at https://pi.hole/dns-query, or a WireGuard peer on a dynamic DNS name, resolves to the LAN but reads as remote. Those users get no banner and no prompt — just a resolver that stopped answering. Classifying them up front would mean resolving a name on whichever thread asked, including the main one, which is exactly what the address-only rule exists to avoid. Take the address from the code that already resolved it instead. OkHttp resolves the DoH endpoint anyway, so hang a Dns off the client and read what it got; WgEgress.resolveEndpoint() likewise has the peer's address in hand. Both hand it to LocalNetworkAccess.reportDestination(), which latches if it is local, and isMissing() ors that in. The destination is what matters, not whether the connection failed: if it is local and the permission is missing, that traffic is blocked. The latch is in-memory only — a stale observation dies with the process, and anything still pointing at the LAN re-reports on next use. Changing a relevant setting clears it, since the new value may point elsewhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kasnder
force-pushed
the
fix/local-network-observed-destinations
branch
from
August 5, 2026 09:13
bbf7313 to
bbda98b
Compare
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.
Stacked on #704 — base is
claude/trackercontrol-issue-701-9x335i, so review that first. Merge #704 and this retargets tomasteron its own.The gap
LocalNetworkAccess.isConfigured()in #704 classifies the stored configuration, and deliberately only looks at address literals: resolving a name there would mean a blocking lookup on whichever thread asked, including the main one. That leaves a hole:https://pi.hole/dns-queryBoth resolve to the local network, both are blocked on Android 17 without
ACCESS_LOCAL_NETWORK, and both read as remote. Those users get no banner and no prompt — just a resolver that quietly stopped answering, which is the exact failure #701 reported.The change
Take the address from the code that has already resolved it, so nothing new is looked up and no thread blocks:
Dnsthat delegates toDns.SYSTEMand notes what came back.WgEgress.resolveEndpoint()already holds the peer's address when it buildsip:port.Both call
LocalNetworkAccess.reportDestination(address), which latches when the address is local.isMissing()ors that in besideisConfigured(), so the banner and the prompt behave identically whether the configuration named an address or a host.Two properties worth stating:
reportDestination()is deliberately not gated onisEnforced()—isMissing()applies that gate anyway, and leaving the branch out makes the latch testable under Robolectric, which runs below the enforcement level.Testing
Four new unit tests: a LAN destination latches, a public one and a host name do not, reset clears it, and an observation never leaks into
isConfigured(). 27 tests inLocalNetworkAccessTest, all passing.Verified on a Pixel 8 (Android 17, API 37) with
ACCESS_LOCAL_NETWORKdenied, using a DoH endpoint athttps://MAC-G0WPC6L7Y4.fritz.box:8443/dns-query— a host name, with no address literal anywhere in the configuration:ActivityMain.isConfigured()cannot classify a host name, which is the gap this PR closes.onResume.https://dns.quad9.net/dns-queryin a fresh process: no banner. The latch fires on the address, not on the mere fact that DoH ran.One observation from testing, not a defect in this PR but worth recording: the FRITZ!Box returned both a private A record (
192.168.178.128) and a global AAAA (2a02:a470:...), because it delegates a public IPv6 prefix to LAN hosts. The latch fired on the IPv4. A LAN host reachable only over a global IPv6 address would not be classified as local byisLocalAddress(), which goes by address range. Whether Android's own local-network enforcement is purely range-based or also same-link aware is untested here — if it is same-link aware, that is a gap inisLocalAddress()itself, predating this PR and worth its own investigation.