feat(health): flag speakers whose runtime bmxRegistryUrl is still on the Bose cloud (#577)

## What Adds a new `runtime_bmx_url_stale` health check to `soundtouch-service`. For each reachable speaker it reads the **runtime** `bmxRegistryUrl` (from the on-device `SoundTouchSdkPrivateCfg.xml` via SSH, or `getpdo CurrentSystemConfiguration` over telnet) and warns when it still points at the shut-down Bose cloud, offering a copy-paste re-migrate command. ## Why Radio source types (TUNEIN / RADIO_BROWSER / LOCAL_INTERNET_RADIO) are delivered to the speaker through the BMX registry. A speaker whose runtime `bmxRegistryUrl` still names the Bose cloud can never mount them, even though the service's own `/sources` listing is correct. The existing `sources_xml_diff` check reports the *symptom* ("missing 3 source types"); this check reports the *per-device cause*, so an operator sees exactly which speakers still need re-migrating. This is the recurring "radio missing after migration" theme (relates to #549, #547, #546, #493). In the #549 diagnostic, 6 of 9 speakers had never actually been migrated (all four runtime URLs still on `content.api.bose.io` / `streaming.bose.com`) while the service itself looked healthy; this check would have surfaced that per device immediately. ## False-positive guard Under a DNS-based migration (AfterTouch acting as the speaker's DNS server) a cloud URL is legitimate: the redirect happens at the DNS layer, not by rewriting the on-device URL. So the check stays silent while the service's own DNS interception is running (`GetDNSRunning`). The router-DNS variant (the LAN's DNS points at AfterTouch without our DNS server running) cannot be detected here, so it is called out as a known exception in the finding text rather than suppressed. ## Changes - `pkg/service/health/checks_runtime_bmx_url.go` (+ unit test): the check, following the injected-closure pattern of `checks_marge_url.go`. `assessRuntimeBmxURL` is the pure, testable core; `isBoseCloudHost` does a domain-suffix match on the Bose cloud domains. - `pkg/service/handlers/handlers_export.go`: a lightweight `readSpeakerBmxRegistryURL(ip)` reader (SSH then telnet), reusing the existing export imports. The diagnostic export path itself is unchanged. - `pkg/service/handlers/server.go`: registers the check, wiring `GetDNSRunning` as the guard. The health package stays free of the SSH / `setup` imports (the reader lives in the handlers layer), matching the existing dependency boundary. ## Testing - `go test ./pkg/service/health/` green (new tests: cloud URL warns; AfterTouch URL and empty URL do not; `isBoseCloudHost` matrix). - `go vet` and `golangci-lint` clean on both packages; `go build ./cmd/soundtouch-service/` succeeds. Not tied to a single issue to close; it complements the #549 / #547 / #546 / #493 cluster as a diagnostic aid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-18 16:46:17 +00:00 · 2026-07-26 21:11:10 +02:00
parent bb9bce440e
commit 4a76805df4
4 changed files with 236 additions and 0 deletions
@@ -593,6 +593,36 @@ func (s *Server) collectSpeakerRedirectConfig(tw *tar.Writer) map[string]*redire
 	return out
 }

+// readSpeakerBmxRegistryURL reads a single speaker's runtime bmxRegistryUrl from
+// its on-device SoundTouchSdkPrivateCfg.xml (via SSH), falling back to `getpdo
+// CurrentSystemConfiguration` over telnet. It returns the URL and whether the
+// runtime config could be read at all (SSH or telnet succeeded). Unlike
+// collectSpeakerRedirectConfig it archives nothing; it's the lightweight read
+// used by the runtime_bmx_url_stale health check.
+func (s *Server) readSpeakerBmxRegistryURL(ip string) (string, bool) {
+	if ip == "" {
+		return "", false
+	}
+
+	// 1. Prefer the persisted XML over SSH.
+	sc := speakerssh.NewClient(ip)
+	if data, err := sc.ReadFile(setup.SoundTouchSdkPrivateCfgPath); err == nil {
+		var cfg setup.PrivateCfg
+		if xml.Unmarshal(data, &cfg) == nil {
+			return cfg.BmxRegistryUrl, true
+		}
+	}
+
+	// 2. Fall back to telnet getpdo when SSH gave us nothing.
+	if raw, ok := readTelnetSystemConfig(ip); ok {
+		if fields := setup.ParseGetpdoConfig(raw); len(fields) > 0 {
+			return fields["bmxRegistryUrl"], true
+		}
+	}
+
+	return "", false
+}
+
 // addServiceLog appends the in-memory service log buffer as logs/service.txt.
 // Each entry is formatted as "2006-01-02T15:04:05Z <message>".
 func (s *Server) addServiceLog(tw *tar.Writer) {
@@ -147,6 +147,15 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red
 	health.RegisterSpeakerInfoReachable(s.healthRegistry, ds)
 	health.RegisterSourcesXMLDiff(s.healthRegistry, ds)
 	health.RegisterSpeakerMargeURLCheck(s.healthRegistry, ds, s.ExpectedHosts)
+	health.RegisterRuntimeBmxURLStaleCheck(
+		s.healthRegistry,
+		ds,
+		s.readSpeakerBmxRegistryURL,
+		func() bool {
+			running, _ := s.GetDNSRunning()
+			return running
+		},
+	)
 	health.RegisterCertChainCheck(
 		s.healthRegistry,
 		func() string {
@@ -0,0 +1,133 @@
+package health
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/gesellix/bose-soundtouch/pkg/service/datastore"
+)
+
+// CheckIDRuntimeBmxURLStale is the registry id of the runtime BMX-URL
+// staleness check.
+const CheckIDRuntimeBmxURLStale = "runtime_bmx_url_stale"
+
+// RegisterRuntimeBmxURLStaleCheck registers a check that reads each reachable
+// speaker's *runtime* bmxRegistryUrl (from its on-device
+// SoundTouchSdkPrivateCfg.xml via SSH, or `getpdo` over telnet) and flags
+// speakers still pointed at the shut-down Bose cloud.
+//
+// Radio source types (TUNEIN / RADIO_BROWSER / LOCAL_INTERNET_RADIO) are
+// delivered to the speaker through the BMX registry, so a speaker whose
+// bmxRegistryUrl still names the Bose cloud can never mount them. That is the
+// most common "radio missing after migration" cause. The service's own /sources
+// listing is correct, which is why the existing sources_xml_diff check only
+// reports the symptom ("missing 3 source types"); this check reports the
+// per-device cause so the operator knows exactly which speakers need
+// re-migrating.
+//
+// readBmxURL reads a speaker's runtime bmxRegistryUrl by IP, returning the URL
+// and whether the runtime config could be read at all. It lives in the handlers
+// layer because the health package deliberately avoids importing the SSH/telnet
+// and setup packages (see the boundary comments in server.go).
+//
+// dnsRunningFn reports whether this service is running its own DNS interception.
+// This is the false-positive guard: under a DNS-based migration (AfterTouch
+// acting as the speaker's DNS server) a cloud URL is EXPECTED, because the
+// redirect happens at the DNS layer rather than by rewriting the on-device URL,
+// so when our DNS is running the check stays silent. The router-DNS variant (the
+// LAN's DNS points at AfterTouch without our DNS server running) is called out
+// as a known exception in the finding text rather than suppressed, since we
+// cannot detect it here.
+func RegisterRuntimeBmxURLStaleCheck(r *Registry, ds *datastore.DataStore, readBmxURL func(ip string) (string, bool), dnsRunningFn func() bool) {
+	r.Register(Check{
+		ID:    CheckIDRuntimeBmxURLStale,
+		Title: "Speaker runtime bmxRegistryUrl points at AfterTouch, not the Bose cloud",
+		Run: func() []Finding {
+			return runRuntimeBmxURLStaleCheck(ds, readBmxURL, dnsRunningFn)
+		},
+	})
+}
+
+func runRuntimeBmxURLStaleCheck(ds *datastore.DataStore, readBmxURL func(ip string) (string, bool), dnsRunningFn func() bool) []Finding {
+	if ds == nil || readBmxURL == nil {
+		return nil
+	}
+
+	// When this service intercepts DNS, a speaker legitimately keeps the Bose
+	// cloud hostnames in its on-device config (they resolve to AfterTouch), so a
+	// cloud URL is not evidence of a stale migration. Don't second-guess it.
+	if dnsRunningFn != nil && dnsRunningFn() {
+		return nil
+	}
+
+	devices, err := ds.ListAllDevices()
+	if err != nil {
+		return []Finding{{
+			Severity: SeverityError,
+			Message:  "Could not enumerate devices: " + err.Error(),
+		}}
+	}
+
+	var findings []Finding
+
+	for i := range devices {
+		dev := &devices[i]
+		if dev.IPAddress == "" || dev.DeviceID == "" {
+			continue
+		}
+
+		bmxURL, ok := readBmxURL(dev.IPAddress)
+		if !ok || bmxURL == "" {
+			// Couldn't read the runtime config (speaker offline, or neither SSH
+			// nor telnet available). speaker_info_reachable / sources_xml_diff
+			// cover the reachability angle; nothing to assert here.
+			continue
+		}
+
+		findings = append(findings, assessRuntimeBmxURL(dev.AccountID, dev.DeviceID, dev.IPAddress, bmxURL)...)
+	}
+
+	return findings
+}
+
+// assessRuntimeBmxURL is the pure, per-device core: given a speaker's runtime
+// bmxRegistryUrl, it returns a warning finding when the URL still names the Bose
+// cloud. Split out from the datastore iteration so it can be unit-tested
+// directly (mirrors assessMargeURLForDeviceWithURL).
+func assessRuntimeBmxURL(account, deviceID, ipAddress, bmxURL string) []Finding {
+	host := hostFromURL(bmxURL)
+	if host == "" || !isBoseCloudHost(host) {
+		return nil
+	}
+
+	return []Finding{{
+		Severity: SeverityWarning,
+		Target:   Target{Account: account, Device: deviceID},
+		Message:  fmt.Sprintf("Speaker's runtime BMX registry URL still points at the Bose cloud (%s).", host),
+		Details:  "Radio source types (TUNEIN, RADIO_BROWSER, LOCAL_INTERNET_RADIO) are fetched from the BMX registry, so while this URL names the shut-down Bose cloud the speaker can never mount them, even though the service's own /sources listing looks correct. Re-migrate this speaker so its runtime bmxRegistryUrl points at AfterTouch, then reboot it. (Exception: if you migrate via DNS, meaning AfterTouch acts as the speaker's DNS server or your router points the LAN's DNS at AfterTouch, a cloud URL is expected and this warning can be ignored.)",
+		ManualCommands: []ManualCommand{{
+			Label:   "Re-migrate this speaker (the telnet method rewrites all runtime URLs):",
+			Command: fmt.Sprintf("soundtouch-cli --host %s setup migrate --method telnet --service-url http://<aftertouch-host>:8000", ipAddress),
+			Hint:    "Replace <aftertouch-host> with a LAN-resolvable name or IP of this service. Reboot the speaker afterwards so it reloads the new config.",
+		}},
+	}}
+}
+
+// isBoseCloudHost reports whether host belongs to one of Bose's (shut-down)
+// cloud domains: content.api.bose.io, streaming.bose.com, events.api.bosecm.com,
+// worldwide.bose.com, and the like. A domain-suffix match keeps it robust
+// against the various sub-domains seen across firmware versions.
+func isBoseCloudHost(host string) bool {
+	host = strings.ToLower(strings.TrimSpace(host))
+	if host == "" {
+		return false
+	}
+
+	for _, domain := range []string{"bose.io", "bose.com", "bosecm.com"} {
+		if host == domain || strings.HasSuffix(host, "."+domain) {
+			return true
+		}
+	}
+
+	return false
+}
@@ -0,0 +1,64 @@
+package health
+
+import "testing"
+
+func TestRuntimeBmxURL_WarnsWhenStillOnBoseCloud(t *testing.T) {
+	got := assessRuntimeBmxURL("1000001", "DEVICEID01", "192.0.2.10",
+		"https://content.api.bose.io/bmx/registry/v1/services")
+	if len(got) != 1 {
+		t.Fatalf("expected 1 finding for a Bose-cloud bmx URL, got %d: %+v", len(got), got)
+	}
+
+	f := got[0]
+	if f.Severity != SeverityWarning {
+		t.Errorf("expected SeverityWarning, got %q", f.Severity)
+	}
+	if f.Target.Account != "1000001" || f.Target.Device != "DEVICEID01" {
+		t.Errorf("unexpected target: %+v", f.Target)
+	}
+	if len(f.ManualCommands) != 1 {
+		t.Fatalf("expected a re-migrate manual command, got %+v", f.ManualCommands)
+	}
+}
+
+func TestRuntimeBmxURL_NoFindingWhenPointingAtAfterTouch(t *testing.T) {
+	got := assessRuntimeBmxURL("1000001", "DEVICEID01", "192.0.2.10",
+		"http://192.0.2.10:8000/bmx/registry/v1/services")
+	if len(got) != 0 {
+		t.Errorf("expected no findings for an AfterTouch bmx URL, got %+v", got)
+	}
+}
+
+func TestRuntimeBmxURL_NoFindingWhenEmpty(t *testing.T) {
+	if got := assessRuntimeBmxURL("1000001", "DEVICEID01", "192.0.2.10", ""); len(got) != 0 {
+		t.Errorf("expected no findings for an empty bmx URL, got %+v", got)
+	}
+}
+
+func TestIsBoseCloudHost(t *testing.T) {
+	cloud := []string{
+		"content.api.bose.io",
+		"streaming.bose.com",
+		"events.api.bosecm.com",
+		"worldwide.bose.com",
+		"bose.com",
+	}
+	for _, h := range cloud {
+		if !isBoseCloudHost(h) {
+			t.Errorf("expected %q to be a Bose cloud host", h)
+		}
+	}
+
+	local := []string{
+		"aftertouch.local",
+		"192.0.2.10",
+		"",
+		"notbose.example.com",
+		"bose.io.evil.example.com",
+	}
+	for _, h := range local {
+		if isBoseCloudHost(h) {
+			t.Errorf("expected %q NOT to be a Bose cloud host", h)
+		}
+	}
+}