fix(router,admin): trust full-length ECS only, gate PTR on selected upstream, harden hostname cache

Review findings on the DNS attribution and hostname series:

- ClientIPOf only trusts EDNS Client Subnet addresses with a full-length
  source prefix (32/128); a truncated prefix like dnsmasq --add-subnet=24
  carries only the subnet base and is ignored, so attribution stays on the
  real client instead of the /24 network address.
- The internal PTR gate now judges the currently selected upstream rather
  than the whole pool, so a degraded mixed pool cannot forward internal
  reverse lookups to a public fallback.
- Failed hostname lookups are cached for five minutes instead of the full
  hour, so a resolver blip stops suppressing names quickly while keeping
  retries rare.
- Duplicate IPs in one decoration batch resolve once instead of once per
  entry.
- Traffic-view hostname decoration runs on a private copy of the cached
  snapshot outside trafficMu, so a stalled resolver neither blocks other
  readers nor races on the shared backing array.
- Local DNS replies (failures and empty answers) echo an OPT record when
  the query carried EDNS0, per RFC 6891.

ARCHITECTURE.md

@@ -80,9 +80,9 @@ For non-transport fallback traffic, it can route by TLS SNI to per-domain upstre
    Proxy-routed domains return local A/AAAA records, suppress HTTPS/SVCB and other non-address metadata locally, and never leak proxy-matched names to direct upstream DNS.
    Direct upstream DNS failures fall back only for retryable upstream service errors; when no fallback succeeds, the last upstream DNS response code is returned as-is.
    Service discovery names are matched against both the full query name and the base domain only for service record types.
-   Reverse lookups (PTR) for internal ranges (RFC1918, CGNAT 100.64/10, link-local, loopback, IPv6 ULA) are answered with NXDOMAIN locally unless an internal DNS server is among the upstreams, so internal layout never leaks to public DNS and internal reverse resolution still works when an internal DNS server is configured.
-   Client attribution for DNS statistics prefers the EDNS Client Subnet (ECS) address when a forwarding resolver (e.g. dnsmasq --add-subnet) carries the real client in the query, falling back to the transport source address; ECS is stripped before any query is forwarded upstream so client subnets never reach public resolvers.
-   When `dns.reverse` is configured, the traffic console resolves client IPs to hostnames via PTR queries to that resolver (typically the local dnsmasq, which answers LAN leases and tailnet names and forwards everything else); results are cached for an hour and failures degrade to the raw IP.
+   Reverse lookups (PTR) for internal ranges (RFC1918, CGNAT 100.64/10, link-local, loopback, IPv6 ULA) are answered with NXDOMAIN locally unless the upstream that would serve the query is an internal DNS server. The gate judges the currently selected upstream rather than the whole pool, so internal layout never leaks to public DNS — including a degraded mixed pool that fell back to a public resolver — and internal reverse resolution still works while an internal DNS server is selected.
+   Client attribution for DNS statistics prefers the EDNS Client Subnet (ECS) address when a forwarding resolver (e.g. dnsmasq --add-subnet) carries the real client in the query with a full-length source prefix (32/128; a truncated prefix such as --add-subnet=24 only identifies the subnet base and is ignored), falling back to the transport source address; ECS is stripped before any query is forwarded upstream so client subnets never reach public resolvers. ECS is trusted only from the local forwarding resolver — a direct client can spoof it, which affects console attribution only.
+   When `dns.reverse` is configured, the traffic console resolves client IPs to hostnames via PTR queries to that resolver (typically the local dnsmasq, which answers LAN leases and tailnet names and forwards everything else); results are cached for an hour, failed lookups for five minutes, and failures degrade to the raw IP.
 9. For DNS-mode transparent HTTP traffic, parse the target host from the request line and always forward through the upstream proxy. For DNS-mode transparent HTTPS traffic, peek the TLS ClientHello to extract SNI and always forward through the upstream proxy.
    These transparent `80/443` listeners are second-stage proxy-only handlers for domains already mapped to local proxy IPs by DNS; they do not run smart routing again.
    HTTPS transparent proxying reads only the TLS ClientHello, then replays the untouched bytes to the selected upstream; it must not complete or terminate TLS locally.

internal/admin/hostname.go

@@ -13,19 +13,25 @@ type HostnameResolver interface {
 	Hostname(ctx context.Context, ip string) string
 }
 
-// hostnameTTL bounds how long a resolved hostname is reused before the IP is
-// queried again. Client IPs are stable in practice, so an hour keeps reverse
-// lookups at a few queries per active client per hour.
-const hostnameTTL = time.Hour
-
-// hostnameTimeout bounds each individual reverse lookup. A stalled resolver
-// must never block the traffic view.
-const hostnameTimeout = 2 * time.Second
-
-// hostnameCacheMax bounds the number of cached hostnames. Client counts are
-// small (the stats clients map itself is bounded), so the cache is cleared
-// wholesale when full rather than paying for an eviction structure.
-const hostnameCacheMax = 1024
+const (
+	// hostnameTTL bounds how long a resolved hostname is reused before the
+	// IP is queried again. Client IPs are stable in practice, so an hour
+	// keeps reverse lookups at a few queries per active client per hour.
+	hostnameTTL = time.Hour
+	// hostnameNegativeTTL bounds how long a failed lookup (empty hostname)
+	// is remembered. A resolver blip must not suppress hostnames for the
+	// full positive TTL; five minutes keeps retries rare while recovering
+	// quickly from transient failures.
+	hostnameNegativeTTL = 5 * time.Minute
+	// hostnameTimeout bounds each individual reverse lookup. A stalled
+	// resolver must never block the traffic view.
+	hostnameTimeout = 2 * time.Second
+	// hostnameCacheMax bounds the number of cached hostnames. Client counts
+	// are small (the stats clients map itself is bounded), so the cache is
+	// cleared wholesale when full rather than paying for an eviction
+	// structure.
+	hostnameCacheMax = 1024
+)
 
 type hostnameEntry struct {
 	host string
@@ -34,15 +40,25 @@ type hostnameEntry struct {
 
 // hostnameCache memoizes reverse lookups keyed by client IP. It is
 // goroutine-safe: lookups run outside the lock, and only the map writes are
-// serialized.
+// serialized. now is injectable for tests.
 type hostnameCache struct {
 	mu       sync.Mutex
 	byIP     map[string]hostnameEntry
 	resolver HostnameResolver
+	now      func() time.Time
 }
 
 func newHostnameCache(resolver HostnameResolver) *hostnameCache {
-	return &hostnameCache{byIP: make(map[string]hostnameEntry), resolver: resolver}
+	return &hostnameCache{byIP: make(map[string]hostnameEntry), resolver: resolver, now: time.Now}
+}
+
+// entryTTL selects the reuse window for a cached entry: failed lookups
+// (empty hostname) are retried sooner than successful ones.
+func (c *hostnameCache) entryTTL(host string) time.Duration {
+	if host == "" {
+		return hostnameNegativeTTL
+	}
+	return hostnameTTL
 }
 
 // attachHostnames decorates client stats with resolved hostnames. It is a
@@ -55,17 +71,17 @@ func (s *Server) attachHostnames(clients []ClientStat) {
 }
 
 // attach fills Hostname on each client stat, resolving unknown IPs through
-// the injected resolver and reusing cached results within hostnameTTL.
+// the injected resolver and reusing cached results within their TTL.
 func (c *hostnameCache) attach(clients []ClientStat) {
 	if c == nil || len(clients) == 0 {
 		return
 	}
 
-	now := time.Now()
+	now := c.now()
 	var unknown []int
 	c.mu.Lock()
 	for i := range clients {
-		if e, ok := c.byIP[clients[i].IP]; ok && now.Sub(e.at) < hostnameTTL {
+		if e, ok := c.byIP[clients[i].IP]; ok && now.Sub(e.at) < c.entryTTL(e.host) {
 			clients[i].Hostname = e.host
 			continue
 		}
@@ -77,30 +93,39 @@ func (c *hostnameCache) attach(clients []ClientStat) {
 		return
 	}
 
-	// Resolve unknown IPs concurrently so a stalled resolver delays the
-	// traffic view by at most one lookup, not by every client in turn.
-	// Each write targets a distinct slice element; the cache itself is
+	// Deduplicate the batch: the same IP can appear under several entries
+	// (per-domain breakdown plus cumulative totals), and one lookup is
+	// enough. Resolve unique IPs concurrently so a stalled resolver delays
+	// the traffic view by at most one lookup, not by every client in turn.
+	// Each write targets distinct slice elements; the cache itself is
 	// guarded by its own mutex.
-	var wg sync.WaitGroup
-	wg.Add(len(unknown))
+	unique := make(map[string][]int, len(unknown))
 	for _, i := range unknown {
-		go func(i int) {
+		unique[clients[i].IP] = append(unique[clients[i].IP], i)
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(len(unique))
+	for ip, idxs := range unique {
+		go func(ip string, idxs []int) {
 			defer wg.Done()
 			ctx, cancel := context.WithTimeout(context.Background(), hostnameTimeout)
-			host := c.resolver.Hostname(ctx, clients[i].IP)
+			host := c.resolver.Hostname(ctx, ip)
 			cancel()
-			c.store(clients[i].IP, host, now)
-			clients[i].Hostname = host
-		}(i)
+			c.store(ip, host)
+			for _, i := range idxs {
+				clients[i].Hostname = host
+			}
+		}(ip, idxs)
 	}
 	wg.Wait()
 }
 
-func (c *hostnameCache) store(ip, host string, now time.Time) {
+func (c *hostnameCache) store(ip, host string) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	if len(c.byIP) >= hostnameCacheMax {
 		c.byIP = make(map[string]hostnameEntry)
 	}
-	c.byIP[ip] = hostnameEntry{host: host, at: now}
+	c.byIP[ip] = hostnameEntry{host: host, at: c.now()}
 }

internal/admin/hostname_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"sync/atomic"
 	"testing"
+	"time"
 )
 
 type fakeHostnameResolver struct {
@@ -89,3 +90,74 @@ func TestAttachHostnamesConcurrent(t *testing.T) {
 		<-done
 	}
 }
+
+func TestAttachHostnamesDeduplicatesBatch(t *testing.T) {
+	t.Parallel()
+
+	resolver := &fakeHostnameResolver{byIP: map[string]string{"192.168.1.2": "phone.lan"}}
+	cache := newHostnameCache(resolver)
+
+	// The same IP can appear several times in one batch (per-domain
+	// breakdown plus cumulative totals); one lookup must cover them all.
+	clients := []ClientStat{{IP: "192.168.1.2"}, {IP: "192.168.1.2"}, {IP: "192.168.1.2"}}
+	cache.attach(clients)
+	if resolver.lookups.Load() != 1 {
+		t.Fatalf("duplicate IPs must resolve once, lookups=%d", resolver.lookups.Load())
+	}
+	for i := range clients {
+		if clients[i].Hostname != "phone.lan" {
+			t.Fatalf("entry %d hostname: %q", i, clients[i].Hostname)
+		}
+	}
+}
+
+func TestAttachHostnamesNegativeTTLExpires(t *testing.T) {
+	t.Parallel()
+
+	resolver := &fakeHostnameResolver{} // no PTR data for any IP
+	cache := newHostnameCache(resolver)
+	now := time.Now()
+	cache.now = func() time.Time { return now }
+
+	clients := []ClientStat{{IP: "1.2.3.4"}}
+	cache.attach(clients)
+	if resolver.lookups.Load() != 1 {
+		t.Fatalf("expected 1 lookup, got %d", resolver.lookups.Load())
+	}
+
+	// Within the negative TTL the failed lookup is remembered.
+	now = now.Add(hostnameNegativeTTL - time.Minute)
+	cache.attach(clients)
+	if resolver.lookups.Load() != 1 {
+		t.Fatalf("negative result should be cached, lookups=%d", resolver.lookups.Load())
+	}
+
+	// Past the negative TTL the IP is queried again.
+	now = now.Add(2 * time.Minute)
+	cache.attach(clients)
+	if resolver.lookups.Load() != 2 {
+		t.Fatalf("negative result should expire, lookups=%d", resolver.lookups.Load())
+	}
+}
+
+func TestAttachHostnamesPositiveTTLKeepsResolved(t *testing.T) {
+	t.Parallel()
+
+	resolver := &fakeHostnameResolver{byIP: map[string]string{"192.168.1.2": "phone.lan"}}
+	cache := newHostnameCache(resolver)
+	now := time.Now()
+	cache.now = func() time.Time { return now }
+
+	clients := []ClientStat{{IP: "192.168.1.2"}}
+	cache.attach(clients)
+	if resolver.lookups.Load() != 1 {
+		t.Fatalf("expected 1 lookup, got %d", resolver.lookups.Load())
+	}
+
+	// A resolved hostname stays cached for the positive TTL.
+	now = now.Add(hostnameTTL - time.Minute)
+	cache.attach(clients)
+	if resolver.lookups.Load() != 1 {
+		t.Fatalf("resolved hostname should stay cached, lookups=%d", resolver.lookups.Load())
+	}
+}

internal/admin/traffic.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"log/slog"
 	"net/http"
+	"slices"
 	"time"
 )
 
@@ -62,6 +63,10 @@ func (s *Server) handleHistory(w http.ResponseWriter, r *http.Request) {
 
 // streamTrafficSnapshot caches the unfiltered live view for one stream tick.
 // Filtered views remain immediate to avoid an unbounded per-client cache.
+// Hostname decoration runs on a private copy of the cached Clients slice
+// outside the lock: the cached snapshot is shared across streams, and a
+// stalled resolver must not block other traffic-view readers, so decoration
+// must neither hold the lock nor race on the shared backing array.
 func (s *Server) streamTrafficSnapshot(sort DomainSort, source Source, client string) TrafficSnapshot {
 	var snap TrafficSnapshot
 	if sort != DomainSortBytes || source != SourceAll || client != "" {
@@ -69,14 +74,21 @@ func (s *Server) streamTrafficSnapshot(sort DomainSort, source Source, client st
 	} else {
 		now := time.Now()
 		s.trafficMu.Lock()
-		defer s.trafficMu.Unlock()
 		if !s.traffic.at.IsZero() && now.Sub(s.traffic.at) < trafficCacheTTL {
-			return s.traffic.snapshot
+			snap = s.traffic.snapshot
+		} else {
+			s.traffic.snapshot = s.opts.Stats.Snapshot(sort, source, client)
+			s.traffic.at = now
+			snap = s.traffic.snapshot
 		}
-		s.traffic.snapshot = s.opts.Stats.Snapshot(sort, source, client)
-		s.traffic.at = now
-		snap = s.traffic.snapshot
+		s.trafficMu.Unlock()
+
+		// The cached snapshot's Clients slice is shared across streams;
+		// decorate a private copy so concurrent readers never race on the
+		// shared backing array.
+		snap.Clients = slices.Clone(snap.Clients)
 	}
+
 	s.attachHostnames(snap.Clients)
 	return snap
 }

router/dns.go

@@ -45,9 +45,12 @@ func (r *Router) ServeDNS(w dns.ResponseWriter, req *dns.Msg) {
 
 	// Internal reverse lookups (RFC1918, CGNAT, link-local, loopback, ULA)
 	// have no data on public upstream DNS and would leak internal network
-	// layout there. Answer NXDOMAIN locally unless an internal DNS server is
-	// among the upstreams and can answer authoritatively.
-	if arpaIP, ok := parseReverseName(domain); ok && isInternalIP(arpaIP) && !r.dnsUpstreamIsInternal() {
+	// layout there. Answer NXDOMAIN locally unless the upstream that would
+	// serve the query is an internal DNS server that can answer
+	// authoritatively. Judging the currently selected upstream (not the
+	// whole pool) keeps a degraded mixed pool from forwarding internal PTR
+	// queries to a public fallback.
+	if arpaIP, ok := parseReverseName(domain); ok && isInternalIP(arpaIP) && !r.dnsSelectedUpstreamIsInternal() {
 		_ = w.WriteMsg(r.dnsFail(req, dns.RcodeNameError))
 		return
 	}
@@ -81,6 +84,12 @@ func (r *Router) ServeDNS(w dns.ResponseWriter, req *dns.Msg) {
 func (r *Router) dnsFail(req *dns.Msg, rcode int) *dns.Msg {
 	m := new(dns.Msg)
 	m.SetRcode(req, rcode)
+	if opt := req.IsEdns0(); opt != nil {
+		// RFC 6891: a response to an EDNS query should carry an OPT record,
+		// otherwise strict clients may treat the reply as truncated or
+		// downgrade their retry behavior.
+		m.SetEdns0(opt.UDPSize(), false)
+	}
 	return m
 }
 
@@ -215,20 +224,20 @@ func isInternalIP(ip net.IP) bool {
 	return false
 }
 
-// dnsUpstreamIsInternal reports whether any configured or discovered DNS
-// upstream is an internal address. Internal reverse lookups are only
-// forwarded when an internal DNS server can answer them.
-func (r *Router) dnsUpstreamIsInternal() bool {
+// dnsSelectedUpstreamIsInternal reports whether the upstream that would
+// serve the next query is an internal address. Internal reverse lookups are
+// only forwarded when they can reach an internal DNS server; judging the
+// currently selected upstream rather than the whole pool keeps a degraded
+// mixed pool from forwarding internal PTR queries to a public fallback.
+func (r *Router) dnsSelectedUpstreamIsInternal() bool {
 	r.dns.Lock()
 	defer r.dns.Unlock()
 
 	if len(r.dns.upstreamAddrs) > 0 {
-		for _, addr := range r.dns.upstreamAddrs {
-			if isInternalHost(addr) {
-				return true
-			}
+		idx := r.dns.upstreamIndex
+		if idx >= 0 && idx < len(r.dns.upstreamAddrs) {
+			return isInternalHost(r.dns.upstreamAddrs[idx])
 		}
-		return false
 	}
 	return isInternalHost(r.dns.upstreamDNS) || isInternalHost(r.dns.fallbackDNS)
 }
@@ -284,6 +293,9 @@ func stripDNSServicePrefix(domain string) (string, bool) {
 func dnsReply(req *dns.Msg) *dns.Msg {
 	reply := new(dns.Msg)
 	reply.SetReply(req)
+	if opt := req.IsEdns0(); opt != nil {
+		reply.SetEdns0(opt.UDPSize(), false)
+	}
 	return reply
 }
 
@@ -660,13 +672,18 @@ func (r *Router) exchangeTCP(req *dns.Msg, addr string) (*dns.Msg, error) {
 
 // ClientIPOf returns the client IP to attribute a DNS query to. Queries
 // forwarded by a local resolver (e.g. dnsmasq --add-subnet) carry the real
-// client address in the EDNS Client Subnet option; when present it takes
-// precedence over the transport source address, which would otherwise be the
-// resolver itself.
+// client address in the EDNS Client Subnet option; when present with a
+// full-length source prefix it takes precedence over the transport source
+// address, which would otherwise be the resolver itself. A truncated prefix
+// (e.g. --add-subnet=24) carries only the subnet base, which is not a
+// usable client identity, so it is ignored. ECS is trusted only from the
+// local forwarding resolver: a client that reaches the listener directly
+// can spoof the option, but that only affects console attribution because
+// ECS is stripped before any query is forwarded upstream.
 func ClientIPOf(req *dns.Msg, remote net.Addr) string {
 	if opt := req.IsEdns0(); opt != nil {
 		for _, o := range opt.Option {
-			if sub, ok := o.(*dns.EDNS0_SUBNET); ok && sub.Address != nil {
+			if sub, ok := o.(*dns.EDNS0_SUBNET); ok && fullLengthECS(sub) {
 				if ip := sub.Address.String(); ip != "" {
 					return ip
 				}
@@ -682,6 +699,20 @@ func ClientIPOf(req *dns.Msg, remote net.Addr) string {
 	return remote.String()
 }
 
+// fullLengthECS reports whether the subnet option carries the complete
+// client address: source prefix length 32 for IPv4 (family 1) or 128 for
+// IPv6 (family 2). Truncated prefixes only identify a network, not a client.
+func fullLengthECS(sub *dns.EDNS0_SUBNET) bool {
+	switch sub.Family {
+	case 1:
+		return sub.SourceNetmask == 32
+	case 2:
+		return sub.SourceNetmask == 128
+	default:
+		return false
+	}
+}
+
 // stripECS removes the EDNS Client Subnet option before a query is forwarded
 // upstream. The subnet is added by a forwarding resolver (dnsmasq
 // --add-subnet) and is only meaningful between that resolver and this proxy;

router/dns_ecs_test.go

@@ -39,6 +39,16 @@ func TestClientIPOfPrefersECS(t *testing.T) {
 		t.Fatalf("ECS v6: got %q", got)
 	}
 
+	// Truncated prefixes (--add-subnet=24) carry only the subnet base,
+	// which is not a usable client identity: fall back to the transport
+	// address instead of attributing the /24 base.
+	if got := ClientIPOf(ecsMsg("192.168.1.0", 1, 24), remote); got != "127.0.0.1" {
+		t.Fatalf("truncated v4 ECS must fall back, got %q", got)
+	}
+	if got := ClientIPOf(ecsMsg("fd00::", 2, 64), remote); got != "127.0.0.1" {
+		t.Fatalf("truncated v6 ECS must fall back, got %q", got)
+	}
+
 	// No ECS: fall back to the transport remote address.
 	plain := new(dns.Msg)
 	plain.SetQuestion("example.com.", dns.TypeA)

router/dns_reverse_test.go

@@ -81,40 +81,87 @@ func TestIsInternalIP(t *testing.T) {
 	}
 }
 
-func TestDNSUpstreamIsInternal(t *testing.T) {
+func TestDNSSelectedUpstreamIsInternal(t *testing.T) {
 	t.Parallel()
 
 	r := newTestRouter(t, nil, "", "", "", nil)
 
+	// The gate judges the currently selected upstream, not the pool: a
+	// degraded mixed pool must not forward internal PTR queries to the
+	// public fallback it degraded onto.
 	r.dns.upstreamAddrs = []string{"8.8.8.8:53", "223.5.5.5:53"}
-	if r.dnsUpstreamIsInternal() {
-		t.Fatal("public upstreams reported as internal")
+	r.dns.upstreamIndex = 0
+	if r.dnsSelectedUpstreamIsInternal() {
+		t.Fatal("public selected upstream reported as internal")
 	}
 
-	r.dns.upstreamAddrs = []string{"8.8.8.8:53", "192.168.1.1:53"}
-	if !r.dnsUpstreamIsInternal() {
-		t.Fatal("internal upstream not reported")
+	r.dns.upstreamAddrs = []string{"192.168.1.1:53"}
+	r.dns.upstreamIndex = 0
+	if !r.dnsSelectedUpstreamIsInternal() {
+		t.Fatal("internal selected upstream not reported")
+	}
+
+	// Same pool, different selection: internal at index 0, public after
+	// degradation at index 1 must not count as internal.
+	r.dns.upstreamAddrs = []string{"192.168.1.1:53", "8.8.8.8:53"}
+	r.dns.upstreamIndex = 1
+	if r.dnsSelectedUpstreamIsInternal() {
+		t.Fatal("degraded public upstream reported as internal")
+	}
+
+	// Out-of-range index degrades to the configured/fallback judgment.
+	r.dns.upstreamAddrs = []string{"192.168.1.1:53"}
+	r.dns.upstreamIndex = 5
+	if r.dnsSelectedUpstreamIsInternal() {
+		t.Fatal("out-of-range index must fall back to configured judgment")
 	}
 
 	r.dns.upstreamAddrs = []string{"[::1]:53"}
-	if !r.dnsUpstreamIsInternal() {
+	r.dns.upstreamIndex = 0
+	if !r.dnsSelectedUpstreamIsInternal() {
 		t.Fatal("ipv6 loopback upstream not reported")
 	}
 
 	r.dns.upstreamAddrs = nil
 	r.dns.upstreamDNS = "192.168.1.1"
 	r.dns.fallbackDNS = "223.5.5.5"
-	if !r.dnsUpstreamIsInternal() {
+	if !r.dnsSelectedUpstreamIsInternal() {
 		t.Fatal("internal configured upstream not reported")
 	}
 
 	r.dns.upstreamDNS = ""
 	r.dns.fallbackDNS = "223.5.5.5"
-	if r.dnsUpstreamIsInternal() {
+	if r.dnsSelectedUpstreamIsInternal() {
 		t.Fatal("public fallback reported as internal")
 	}
 }
 
+func TestLocalNXDOMAINCarriesOPTForEDNSQueries(t *testing.T) {
+	t.Parallel()
+
+	r := newTestRouter(t, []string{"127.0.0.2"}, "", "223.5.5.5", "", nil)
+	r.dns.upstreamAddrs = []string{"8.8.8.8:53"}
+	r.dns.upstreamIndex = 0
+
+	// ecsMsg builds a query with an EDNS0 OPT record; RFC 6891 requires the
+	// response to an EDNS query to carry an OPT record as well.
+	req := ecsMsg("192.168.1.42", 1, 32)
+	req.SetQuestion("1.1.168.192.in-addr.arpa.", dns.TypePTR)
+	writer := &mockDNSWriter{localAddr: &net.UDPAddr{IP: net.ParseIP("127.0.0.2"), Port: 53}}
+
+	r.ServeDNS(writer, req)
+
+	if writer.msg == nil {
+		t.Fatal("expected response")
+	}
+	if writer.msg.Rcode != dns.RcodeNameError {
+		t.Fatalf("expected NXDOMAIN, got %s", dns.RcodeToString[writer.msg.Rcode])
+	}
+	if writer.msg.IsEdns0() == nil {
+		t.Fatal("NXDOMAIN reply to an EDNS query must carry an OPT record")
+	}
+}
+
 func TestIsInternalHost(t *testing.T) {
 	t.Parallel()
 
@@ -222,6 +269,53 @@ func TestServeDNSInternalReverseLookup(t *testing.T) {
 		}
 	})
 
+	t.Run("degraded mixed pool does not forward internal reverse", func(t *testing.T) {
+		r := newTestRouter(t, []string{"127.0.0.2"}, "", "223.5.5.5", "", nil)
+		// The pool contains an internal server, but the current selection
+		// degraded onto the public fallback: internal PTR must not reach it.
+		r.dns.upstreamAddrs = []string{"192.168.1.1:53", "8.8.8.8:53"}
+		r.dns.upstreamIndex = 1
+
+		req := new(dns.Msg)
+		req.SetQuestion("1.1.168.192.in-addr.arpa.", dns.TypePTR)
+		writer := &mockDNSWriter{localAddr: &net.UDPAddr{IP: net.ParseIP("127.0.0.2"), Port: 53}}
+
+		r.ServeDNS(writer, req)
+
+		if writer.msg == nil {
+			t.Fatal("expected response")
+		}
+		if writer.msg.Rcode != dns.RcodeNameError {
+			t.Fatalf("expected NXDOMAIN, got %s", dns.RcodeToString[writer.msg.Rcode])
+		}
+	})
+
+	t.Run("selected internal upstream in mixed pool forwards", func(t *testing.T) {
+		var upstreamQueries atomic.Int32
+		upstreamAddr := startUDPTestDNSServer(t, dns.HandlerFunc(func(w dns.ResponseWriter, req *dns.Msg) {
+			upstreamQueries.Add(1)
+			resp := new(dns.Msg)
+			resp.SetReply(req)
+			_ = w.WriteMsg(resp)
+		}))
+
+		r := newTestRouter(t, []string{"127.0.0.2"}, "", "223.5.5.5", "", nil)
+		// Internal server selected first: forward even though a public
+		// fallback sits behind it.
+		r.dns.upstreamAddrs = []string{upstreamAddr, "8.8.8.8:53"}
+		r.dns.upstreamIndex = 0
+
+		req := new(dns.Msg)
+		req.SetQuestion("1.1.168.192.in-addr.arpa.", dns.TypePTR)
+		writer := &mockDNSWriter{localAddr: &net.UDPAddr{IP: net.ParseIP("127.0.0.2"), Port: 53}}
+
+		r.ServeDNS(writer, req)
+
+		if upstreamQueries.Load() != 1 {
+			t.Fatalf("expected forward to selected internal upstream, got %d queries", upstreamQueries.Load())
+		}
+	})
+
 	t.Run("ipv6 internal reverse with public upstream answers NXDOMAIN locally", func(t *testing.T) {
 		r := newTestRouter(t, []string{"127.0.0.2"}, "", "223.5.5.5", "", nil)
 		r.dns.upstreamAddrs = []string{"8.8.8.8:53"}