From ab65dceb9a838edf30ca4f476e74693454116491 Mon Sep 17 00:00:00 2001 From: Tobias Gesellchen Date: Thu, 14 May 2026 13:30:21 +0200 Subject: [PATCH] feat(service): validate server_url and surface resolved DNS intercept IP Refuse to start the DNS server and reject Settings updates whose server_url does not resolve to a routable IP. Without this, a misconfigured hostname caused the DNS server to answer every intercepted Bose hostname with `CNAME .`, leaving speakers unable to reach the service while everything looked healthy. The Settings page now displays the resolved intercept IP (or the resolve error) next to "Target Domain", so misconfigurations are visible up front instead of buried in the DNS log. Refs #269 Co-Authored-By: Claude Opus 4.7 (1M context) --- pkg/service/handlers/dns_settings_test.go | 2 + pkg/service/handlers/handlers_setup.go | 71 ++++++++++++------- pkg/service/handlers/handlers_setup_test.go | 6 +- pkg/service/handlers/mirror_preferred_test.go | 1 + pkg/service/handlers/server.go | 64 +++++++++++++++-- pkg/service/handlers/web/index.html | 1 + pkg/service/handlers/web/js/script.js | 13 ++++ pkg/service/setup/build_https_url_test.go | 12 ++-- 8 files changed, 128 insertions(+), 42 deletions(-) diff --git a/pkg/service/handlers/dns_settings_test.go b/pkg/service/handlers/dns_settings_test.go index c40fe5f..569cd06 100644 --- a/pkg/service/handlers/dns_settings_test.go +++ b/pkg/service/handlers/dns_settings_test.go @@ -25,6 +25,7 @@ func TestDNSSettingsValidation(t *testing.T) { // Test Case 1: Enable DNS with empty upstream (should fallback to system DNS) update := map[string]interface{}{ + "server_url": "http://localhost:8001", "dns_enabled": true, "dns_upstream": "", "dns_bind_addr": ":5353", @@ -55,6 +56,7 @@ func TestDNSSettingsValidation(t *testing.T) { // Test Case 2: Enable DNS with valid upstream // Using a random port to avoid conflicts and ensure it's fast updateValid := map[string]interface{}{ + "server_url": "http://localhost:8001", "dns_enabled": true, "dns_upstream": "8.8.8.8", "dns_bind_addr": "127.0.0.1:0", // Random port diff --git a/pkg/service/handlers/handlers_setup.go b/pkg/service/handlers/handlers_setup.go index cabb984..8df6797 100644 --- a/pkg/service/handlers/handlers_setup.go +++ b/pkg/service/handlers/handlers_setup.go @@ -172,6 +172,14 @@ func (s *Server) HandleGetSettings(w http.ResponseWriter, _ *http.Request) { dnsRunning, actualBind := s.GetDNSRunning() + var serverURLResolvedIP, serverURLResolveError string + + if ip, err := s.resolveServerURLIP(serverURL); err == nil { + serverURLResolvedIP = ip + } else { + serverURLResolveError = err.Error() + } + // Mask secrets: return "***" if set so the UI can show "configured" without exposing the value. if spotifyClientSecret != "" { spotifyClientSecret = "***" @@ -182,32 +190,34 @@ func (s *Server) HandleGetSettings(w http.ResponseWriter, _ *http.Request) { } if err := json.NewEncoder(w).Encode(map[string]interface{}{ - "server_url": serverURL, - "https_server_url": httpsServerURL, - "discovery_interval": discoveryInterval, - "discovery_enabled": discoveryEnabled, - "dns_enabled": dnsEnabled, - "dns_running": dnsRunning, - "dns_actual_bind": actualBind, - "dns_upstream": strings.Join(dnsUpstream, ","), - "dns_bind_addr": dnsBindAddr, - "mirror_enabled": mirrorEnabled, - "mirror_endpoints": mirrorEndpoints, - "skip_mirror_endpoints": skipMirrorEndpoints, - "preferred_source": preferredSource, - "internal_paths": internalPaths, - "redact_logs": redact, - "log_bodies": logBody, - "record_interactions": record, - "shortcuts": shortcuts, - "spotify_configured": spotifyConfigured, - "spotify_client_id": spotifyClientID, - "spotify_client_secret": spotifyClientSecret, - "spotify_redirect_uri": spotifyRedirectURI, - "amazon_configured": amazonConfigured, - "amazon_client_id": amazonClientID, - "amazon_client_secret": amazonClientSecret, - "amazon_redirect_uri": amazonRedirectURI, + "server_url": serverURL, + "server_url_resolved_ip": serverURLResolvedIP, + "server_url_resolve_error": serverURLResolveError, + "https_server_url": httpsServerURL, + "discovery_interval": discoveryInterval, + "discovery_enabled": discoveryEnabled, + "dns_enabled": dnsEnabled, + "dns_running": dnsRunning, + "dns_actual_bind": actualBind, + "dns_upstream": strings.Join(dnsUpstream, ","), + "dns_bind_addr": dnsBindAddr, + "mirror_enabled": mirrorEnabled, + "mirror_endpoints": mirrorEndpoints, + "skip_mirror_endpoints": skipMirrorEndpoints, + "preferred_source": preferredSource, + "internal_paths": internalPaths, + "redact_logs": redact, + "log_bodies": logBody, + "record_interactions": record, + "shortcuts": shortcuts, + "spotify_configured": spotifyConfigured, + "spotify_client_id": spotifyClientID, + "spotify_client_secret": spotifyClientSecret, + "spotify_redirect_uri": spotifyRedirectURI, + "amazon_configured": amazonConfigured, + "amazon_client_id": amazonClientID, + "amazon_client_secret": amazonClientSecret, + "amazon_redirect_uri": amazonRedirectURI, }); err != nil { http.Error(w, "Failed to encode response", http.StatusInternalServerError) return @@ -247,6 +257,15 @@ func (s *Server) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) { log.Printf("[DNS] DNS Discovery enabled without explicit upstreams, will try system DNS.") } + // Validate server_url: the same value the DNS server uses to derive its + // intercept IP. Reject anything that does not resolve to a routable IP so + // users see the error in the UI instead of getting a silently-broken setup + // where DNS replies with `CNAME .` for every Bose hostname. + if _, err := s.resolveServerURLIP(settings.ServerURL); err != nil { + http.Error(w, "Invalid server_url: "+err.Error(), http.StatusBadRequest) + return + } + interval, err := time.ParseDuration(settings.DiscoveryInterval) if err != nil && settings.DiscoveryInterval != "" { http.Error(w, "Invalid discovery interval: "+err.Error(), http.StatusBadRequest) diff --git a/pkg/service/handlers/handlers_setup_test.go b/pkg/service/handlers/handlers_setup_test.go index 8b713c5..6124435 100644 --- a/pkg/service/handlers/handlers_setup_test.go +++ b/pkg/service/handlers/handlers_setup_test.go @@ -101,7 +101,7 @@ func TestProxySettingsAPI(t *testing.T) { // 3. Test System Settings POST sysUpdate := map[string]string{ - "server_url": "http://new-server:8000", + "server_url": "http://127.0.0.1:8000", } sysBody, err := json.Marshal(sysUpdate) @@ -122,13 +122,13 @@ func TestProxySettingsAPI(t *testing.T) { // Verify server state sURL, _ := server.GetSettings() - if sURL != "http://new-server:8000" { + if sURL != "http://127.0.0.1:8000" { t.Errorf("POST /setup/settings: Server state did not update: serverURL=%s", sURL) } // 4. Test Mirror Settings persistence mirrorUpdate := map[string]interface{}{ - "server_url": "http://mirror-test:8000", + "server_url": "http://127.0.0.1:8000", "mirror_enabled": true, "mirror_endpoints": []string{"/test/*"}, "internal_paths": []string{"/setup/*"}, diff --git a/pkg/service/handlers/mirror_preferred_test.go b/pkg/service/handlers/mirror_preferred_test.go index 47939eb..4875f0c 100644 --- a/pkg/service/handlers/mirror_preferred_test.go +++ b/pkg/service/handlers/mirror_preferred_test.go @@ -134,6 +134,7 @@ func TestSettingsAPI_PreferredSource(t *testing.T) { // Test UPDATE update := map[string]interface{}{ + "server_url": "http://localhost:8000", "preferred_source": "upstream", } body, err := json.Marshal(update) diff --git a/pkg/service/handlers/server.go b/pkg/service/handlers/server.go index 2652e01..0168803 100644 --- a/pkg/service/handlers/server.go +++ b/pkg/service/handlers/server.go @@ -238,18 +238,68 @@ func (s *Server) SetDNSSettings(enabled bool, upstream, bind string) { } } +// resolveServerURLIP returns the IP that the DNS server would hand out as the +// intercept answer for the given server URL. An empty URL, empty hostname, or a +// hostname that cannot be resolved to an IP is reported as an error so callers +// can refuse to start (or reject user input) instead of silently degrading. +// "localhost" is treated as 127.0.0.1. +func (s *Server) resolveServerURLIP(serverURL string) (string, error) { + if strings.TrimSpace(serverURL) == "" { + return "", fmt.Errorf("server URL is empty") + } + + u, err := url.Parse(serverURL) + if err != nil { + return "", fmt.Errorf("invalid server URL %q: %w", serverURL, err) + } + + hostname := u.Hostname() + if hostname == "" { + return "", fmt.Errorf("server URL %q has no hostname", serverURL) + } + + if hostname == "localhost" { + return "127.0.0.1", nil + } + + if ip := net.ParseIP(hostname); ip != nil { + return ip.String(), nil + } + + // Prefer the setup manager's resolver (it cascades through device SSH ping + // then system DNS). Fall back to plain system DNS when no manager is wired, + // so this works in tests and lightweight server constructions. + if s.sm != nil { + if resolved := s.sm.GetResolvedIP(hostname); net.ParseIP(resolved) != nil { + return resolved, nil + } + } else if ips, lookupErr := net.LookupIP(hostname); lookupErr == nil { + for _, ip := range ips { + if v4 := ip.To4(); v4 != nil { + return v4.String(), nil + } + } + + if len(ips) > 0 { + return ips[0].String(), nil + } + } + + return "", fmt.Errorf("hostname %q did not resolve to an IP — "+ + "set the server URL to an IP, or to a hostname this host can resolve", + hostname) +} + func (s *Server) startDNSDiscovery(bind string, upstreamList []string) { log.Printf("[DNS] Starting DNS discovery server on %s", bind) - u, _ := url.Parse(s.serverURL) + serviceIP, err := s.resolveServerURLIP(s.serverURL) + if err != nil { + log.Printf("[DNS] Cannot start DNS discovery server: %v", err) - serviceIP := u.Hostname() - if serviceIP == "localhost" || serviceIP == "" { - serviceIP = "127.0.0.1" - } + s.dnsEnabled = false - if s.sm != nil { - serviceIP = s.sm.GetResolvedIP(serviceIP) + return } s.dnsDiscovery = discovery.NewDNSDiscovery(upstreamList, serviceIP) diff --git a/pkg/service/handlers/web/index.html b/pkg/service/handlers/web/index.html index 08f70e8..c814d88 100644 --- a/pkg/service/handlers/web/index.html +++ b/pkg/service/handlers/web/index.html @@ -159,6 +159,7 @@ (Standard services URL) +
Device Discovery: diff --git a/pkg/service/handlers/web/js/script.js b/pkg/service/handlers/web/js/script.js index 86394fb..cfa8ad2 100644 --- a/pkg/service/handlers/web/js/script.js +++ b/pkg/service/handlers/web/js/script.js @@ -126,6 +126,19 @@ async function fetchSettings() { if (settings.server_url) { document.getElementById("target-domain").value = settings.server_url; } + const resolved = document.getElementById("target-domain-resolved"); + if (resolved) { + if (settings.server_url_resolved_ip) { + resolved.style.color = "#2e7d32"; + resolved.innerHTML = "✅ DNS will hand out " + settings.server_url_resolved_ip + + " for intercepted Bose hostnames. Speakers must be able to reach this address."; + } else if (settings.server_url_resolve_error) { + resolved.style.color = "#c62828"; + resolved.innerText = "❌ " + settings.server_url_resolve_error; + } else { + resolved.innerText = ""; + } + } if (settings.discovery_interval) { document.getElementById("discovery-interval").value = settings.discovery_interval; } diff --git a/pkg/service/setup/build_https_url_test.go b/pkg/service/setup/build_https_url_test.go index a87634a..7564f67 100644 --- a/pkg/service/setup/build_https_url_test.go +++ b/pkg/service/setup/build_https_url_test.go @@ -10,16 +10,16 @@ func TestBuildServerHTTPSURL_PortResolution(t *testing.T) { // of each subtest. tests := []struct { - name string - targetURL string + name string + targetURL string envHTTPSPort string - want string + want string }{ { - name: "https with explicit port wins over HTTPS_PORT env", - targetURL: "https://soundtouch.fritz.box:443", + name: "https with explicit port wins over HTTPS_PORT env", + targetURL: "https://soundtouch.fritz.box:443", envHTTPSPort: "8443", - want: "https://soundtouch.fritz.box:443/health", + want: "https://soundtouch.fritz.box:443/health", }, { name: "https without explicit port uses 443",