fix(setup): re-install the CA when it was regenerated, not just label-matched (refs #517)

checkCACertTrusted matched only the static "# AfterTouch" label in the
device's trust bundle. After the service CA was regenerated (e.g. a
recreated container with a fresh/empty data dir), the stale label was
still present, so the migration wrongly reported the speaker as already
trusting the new CA and skipped re-installing it, leaving the speaker
unable to validate TLS to the service.

When the service CA is available, compare the actual cert payload and
re-install on mismatch; fall back to the label only when the CA can't be
read (CLI callers without Crypto). Adds regression tests for the
stale-label and no-Crypto cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

pkg/service/setup/setup.go

@@ -807,58 +807,62 @@ func applyURLOverrides(cfg *PrivateCfg, options map[string]string) {
 	}
 }
 
-// checkCACertTrusted checks if the local CA certificate is already in
-// the device's trust store. The CALabel grep works regardless of whether
-// Manager.Crypto is configured — only the secondary "match cert payload"
-// fallback needs it. CLI callers without Crypto can therefore still
-// detect a previously-trusted CA.
+// checkCACertTrusted decides whether the device already trusts *this* service's
+// CA.
+//
+// When the service CA is available (Manager.Crypto set), the certificate
+// *payload* is authoritative: a previous migration may have left our label
+// (CALabel) in the bundle even though the actual CA has since changed — e.g. the
+// service CA was regenerated after its data dir was recreated without a
+// persistent volume. Matching the label alone would then falsely report the
+// device as trusted and skip re-installing the new CA, leaving the speaker
+// unable to validate TLS to the service (the symptom seen in #517). So we match
+// the current CA's payload and deliberately ignore the (possibly stale) label.
+//
+// Only when the CA can't be read (e.g. a CLI caller without Crypto) do we fall
+// back to the label, which is the best signal available there.
 func (m *Manager) checkCACertTrusted(summary *MigrationSummary, deviceIP string) {
 	client := m.NewSSH(deviceIP)
 	bundlePath := "/etc/pki/tls/certs/ca-bundle.crt"
 
-	// Primary check: our injected label.
-	output, err := client.Run(fmt.Sprintf("grep -F %q %s", CALabel, bundlePath))
-	if err == nil && strings.Contains(output, CALabel) {
-		summary.CACertTrusted = true
-		return
-	}
+	if m.Crypto != nil {
+		if certData, ok := m.firstCACertBodyLine(); ok {
+			// Payload present -> trusted; absent -> not trusted (re-install),
+			// regardless of a possibly-stale label.
+			if _, err := client.Run(fmt.Sprintf("grep -F %q %s", certData, bundlePath)); err == nil {
+				summary.CACertTrusted = true
+			}
 
-	// Secondary check (only when Manager.Crypto is configured): match
-	// the actual cert payload — covers older injections that lack the
-	// label.
-	if m.Crypto == nil {
-		return
-	}
-
-	caCertPEM, err := os.ReadFile(m.Crypto.GetCACertPath())
-	if err != nil {
-		return
-	}
-
-	// We look for the first part of the certificate (e.g. the first 64 chars of the base64 data)
-	// to see if it's already in the bundle.
-	lines := strings.Split(string(caCertPEM), "\n")
-
-	var certData string
-
-	for _, line := range lines {
-		if !strings.Contains(line, "BEGIN CERTIFICATE") && !strings.Contains(line, "END CERTIFICATE") && line != "" {
-			certData = line
-			break
+			return
 		}
 	}
 
-	if certData == "" {
-		return
-	}
-
-	// Use grep to check for the certificate data in the bundle
-	_, err = client.Run(fmt.Sprintf("grep -F %q %s", certData, bundlePath))
-	if err == nil {
+	// Fallback for callers without the CA at hand: match our injected label.
+	output, err := client.Run(fmt.Sprintf("grep -F %q %s", CALabel, bundlePath))
+	if err == nil && strings.Contains(output, CALabel) {
 		summary.CACertTrusted = true
 	}
 }
 
+// firstCACertBodyLine returns the first base64 body line of the service CA
+// certificate, used as a cheap fingerprint to check whether this exact CA is
+// already present in a device's trust bundle. Returns false when the cert can't
+// be read or has no body line.
+func (m *Manager) firstCACertBodyLine() (string, bool) {
+	caCertPEM, err := os.ReadFile(m.Crypto.GetCACertPath())
+	if err != nil {
+		return "", false
+	}
+
+	for _, line := range strings.Split(string(caCertPEM), "\n") {
+		if line != "" && !strings.Contains(line, "BEGIN CERTIFICATE") && !strings.Contains(line, "END CERTIFICATE") {
+			return line, true
+		}
+	}
+
+	return "", false
+}
+
 // MigrateSpeaker configures the speaker at the given IP to use this service.
 func (m *Manager) MigrateSpeaker(deviceIP, targetURL, proxyURL string, options map[string]string, method MigrationMethod) (string, error) {
 	if targetURL == "" {

pkg/service/setup/setup_test.go

@@ -453,6 +453,67 @@ func TestCheckCACertTrusted(t *testing.T) {
 	}
 }
 
+// TestCheckCACertTrustedStaleLabel is the #517 regression: a previous
+// migration's CALabel survived a service-CA regeneration, so the label is in the
+// bundle but the *current* CA payload is not. The device must then be treated as
+// NOT trusting the new CA (so the migration re-installs it), instead of being
+// falsely skipped on the stale label.
+func TestCheckCACertTrustedStaleLabel(t *testing.T) {
+	tempDir, err := os.MkdirTemp("", "ca-trust-stale")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	cm := certmanager.NewCertificateManager(filepath.Join(tempDir, "certs"))
+	if err := cm.EnsureCA(); err != nil {
+		t.Fatalf("Failed to ensure CA: %v", err)
+	}
+
+	m := NewManager("http://localhost:8000", nil, cm)
+	m.NewSSH = func(_ string) SSHClient {
+		return &mockSSH{
+			runFunc: func(command string) (string, error) {
+				// Stale label is still present...
+				if strings.Contains(command, CALabel) {
+					return CALabel, nil
+				}
+				// ...but the current CA payload is not in the bundle.
+				return "", fmt.Errorf("not found")
+			},
+		}
+	}
+
+	summary := &MigrationSummary{}
+	m.checkCACertTrusted(summary, "192.0.2.10")
+	if summary.CACertTrusted {
+		t.Error("stale label without a matching CA payload must not count as trusted (should re-install)")
+	}
+}
+
+// TestCheckCACertTrustedNoCryptoFallback verifies that when the service CA is
+// not available (e.g. a CLI caller without Crypto), the injected label remains
+// the trust signal.
+func TestCheckCACertTrustedNoCryptoFallback(t *testing.T) {
+	m := NewManager("http://localhost:8000", nil, nil)
+	m.NewSSH = func(_ string) SSHClient {
+		return &mockSSH{
+			runFunc: func(command string) (string, error) {
+				if strings.Contains(command, CALabel) {
+					return CALabel, nil
+				}
+				return "", fmt.Errorf("not found")
+			},
+		}
+	}
+
+	summary := &MigrationSummary{}
+	m.checkCACertTrusted(summary, "192.0.2.10")
+	if !summary.CACertTrusted {
+		t.Error("without Crypto, a present label should count as trusted")
+	}
+}
+
 func TestTestConnection(t *testing.T) {
 	tempDir, err := os.MkdirTemp("", "test-connection")
 	if err != nil {