From 3bd2815bd47b7120145c1dcb9394a22144eeccdc Mon Sep 17 00:00:00 2001
From: Gary Hansen <gary.hansen@five9.com>
Date: Mon, 8 Jun 2026 13:19:58 +1000
Subject: [PATCH] Fix code review issues: timeouts, RootHints fallback,
 upstream validation, Unix comment

- Fix ReadTimeout/WriteTimeout to use 5*time.Second instead of 5 (nanoseconds)
- Add RootHints fallback in DiscoverRoots() when queryResolver fails
- Validate --dns-upstream is a valid host:port in Config.Validate()
- Update systemResolver() comment to note it is Unix-only (/etc/resolv.conf)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: multica-agent <github@multica.ai>
---
 internal/config/config.go |  6 ++++++
 internal/dns/roots.go     | 36 ++++++++++++++++++++++++++++++------
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/internal/config/config.go b/internal/config/config.go
index 7bb9ebc..a8abe5c 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -130,6 +130,12 @@ func (c *Config) Validate() error {
 		return ErrAlwaysTCPRequiresTCP
 	}
 
+	if c.DNSUpstream != "" {
+		if _, _, err := net.SplitHostPort(c.DNSUpstream); err != nil {
+			return fmt.Errorf("--dns-upstream %q is not a valid host:port address", c.DNSUpstream)
+		}
+	}
+
 	return nil
 }
 
diff --git a/internal/dns/roots.go b/internal/dns/roots.go
index fac629b..1a5dfbd 100644
--- a/internal/dns/roots.go
+++ b/internal/dns/roots.go
@@ -56,10 +56,34 @@ func DiscoverRoots(ctx context.Context, cfg *RootDiscoveryConfig) ([]RootServer,
 	}
 
 	if cfg.AllRoots {
-		return discoverAllRoots(ctx, resolver, cfg.IncludeAAAA)
+		servers, err := discoverAllRoots(ctx, resolver, cfg.IncludeAAAA)
+		if err != nil {
+			return filterHints(RootHints, cfg.IncludeAAAA), nil
+		}
+		return servers, nil
 	}
 
-	return discoverSingleRoot(ctx, resolver, cfg.IncludeAAAA)
+	servers, err := discoverSingleRoot(ctx, resolver, cfg.IncludeAAAA)
+	if err != nil {
+		hints := filterHints(RootHints, cfg.IncludeAAAA)
+		if len(hints) > 0 {
+			return hints[:1], nil
+		}
+		return nil, err
+	}
+	return servers, nil
+}
+
+// filterHints returns a copy of hints with IPv6 addresses stripped when includeAAAA is false.
+func filterHints(hints []RootServer, includeAAAA bool) []RootServer {
+	out := make([]RootServer, len(hints))
+	for i, h := range hints {
+		out[i] = RootServer{Name: h.Name, IPv4: h.IPv4}
+		if includeAAAA {
+			out[i].IPv6 = h.IPv6
+		}
+	}
+	return out
 }
 
 func discoverRootOverride(ctx context.Context, resolver, server string, includeAAAA bool) ([]RootServer, error) {
@@ -167,8 +191,8 @@ func resolverFromConfig(cfg *RootDiscoveryConfig) string {
 }
 
 // systemResolver returns the first nameserver from the system DNS configuration.
-// On Unix-like systems this reads /etc/resolv.conf. Falls back to 127.0.0.1:53
-// when the system configuration is unavailable or contains no servers.
+// This is Unix-only: it reads /etc/resolv.conf, which does not exist on Windows.
+// On Windows (or any system without /etc/resolv.conf) the fallback 127.0.0.1:53 applies.
 func systemResolver() string {
 	cc, err := dns.ClientConfigFromFile("/etc/resolv.conf")
 	if err != nil || len(cc.Servers) == 0 {
@@ -180,8 +204,8 @@ func systemResolver() string {
 func queryResolver(ctx context.Context, resolverAddr, name string, qtype uint16) (*dns.Msg, error) {
 	c := &dns.Client{
 		Net:          "udp",
-		ReadTimeout:  5,
-		WriteTimeout: 5,
+		ReadTimeout:  5 * time.Second,
+		WriteTimeout: 5 * time.Second,
 	}
 	if deadline, ok := ctx.Deadline(); ok {
 		c.ReadTimeout = time.Until(deadline)