From 4f6f4c497a4a759f370d84c2954202ea9ea18802 Mon Sep 17 00:00:00 2001
From: Tobias Gesellchen <tobias@gesellix.de>
Date: Tue, 19 May 2026 22:27:29 +0200
Subject: [PATCH] fix(health): self-signed AfterTouch chain is INFO, not WARN
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous classifier always returned SeverityWarning when the
served leaf didn't validate against the service host's system
trust store. For AfterTouch's *default* deployment shape (its
own self-signed CA), that's the expected, healthy state — the
service host's trust store deliberately doesn't include our CA;
speakers establish trust via `setup install-ca`, not via system
roots. Reporting it as a warning misled non-technical operators
into thinking something was broken.

Rework the severity matrix:

  - leafFromOwnCA (signature-verified): INFO. Message says
    "AfterTouch is serving its own self-signed CA chain
    (expected)". Details explain the service-host trust-store
    state is by design. Manual command becomes a reminder
    rather than a fix.
  - leafSubjectEqualsIssuer (heuristic): INFO. Explains the
    heuristic and offers both install-ca (if it is AfterTouch)
    and openssl (if it isn't) as paths.
  - leafForeign (genuinely unexpected): WARN. Unchanged
    semantics; this is the case that actually wants attention.
  - connection failure: ERROR. Unchanged.

Title renamed from "HTTPS endpoint certificate validates" (which
read as a binary assertion the finding contradicted) to
"HTTPS endpoint TLS configuration".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 pkg/service/health/checks_cert_chain.go      | 85 +++++++++++---------
 pkg/service/health/checks_cert_chain_test.go | 65 ++++++++++-----
 2 files changed, 95 insertions(+), 55 deletions(-)

diff --git a/pkg/service/health/checks_cert_chain.go b/pkg/service/health/checks_cert_chain.go
index 274a87d..1cd2006 100644
--- a/pkg/service/health/checks_cert_chain.go
+++ b/pkg/service/health/checks_cert_chain.go
@@ -37,7 +37,7 @@ const CheckIDCertChain = "service_cert_chain"
 func RegisterCertChainCheck(r *Registry, httpsURLFn func() string, caCertFn func() *x509.Certificate) {
 	r.Register(Check{
 		ID:    CheckIDCertChain,
-		Title: "HTTPS endpoint certificate validates",
+		Title: "HTTPS endpoint TLS configuration",
 		Run: func() []Finding {
 			return runCertChainCheck(httpsURLFn(), caCertFn)
 		},
@@ -98,50 +98,63 @@ func runCertChainCheck(httpsURL string, caCertFn func() *x509.Certificate) []Fin
 
 	leaf := peers[0]
 
-	subject := leaf.Subject.String()
-	issuer := leaf.Issuer.String()
-	notAfter := leaf.NotAfter.Format("2006-01-02")
-
 	dnsNames := strings.Join(leaf.DNSNames, ", ")
 	if dnsNames == "" {
 		dnsNames = "(none)"
 	}
 
-	details := fmt.Sprintf(
-		"Verification error: %v. Leaf subject: %s. Issuer: %s. SANs: %s. Expires: %s.",
-		err, subject, issuer, dnsNames, notAfter,
+	chainContext := fmt.Sprintf(
+		"Leaf subject: %s. Issuer: %s. SANs: %s. Expires: %s.",
+		leaf.Subject.String(), leaf.Issuer.String(), dnsNames, leaf.NotAfter.Format("2006-01-02"),
 	)
 
-	var hints []ManualCommand
-
-	classification := classifyLeaf(leaf, caCertFn)
-	switch classification {
+	switch classifyLeaf(leaf, caCertFn) {
 	case leafFromOwnCA:
-		hints = append(hints, ManualCommand{
-			Label:   "Install AfterTouch's CA on each speaker:",
-			Command: "soundtouch-cli --host=<speaker-ip> setup install-ca --service-url=" + httpsURL,
-			Hint:    "The served leaf was issued by AfterTouch's own CA (verified by signature). Requires SSH on the speaker. After install, re-run this check.",
-		})
-	case leafSubjectEqualsIssuer:
-		hints = append(hints, ManualCommand{
-			Label:   "If this is a self-signed cert from AfterTouch, install its CA on each speaker:",
-			Command: "soundtouch-cli --host=<speaker-ip> setup install-ca --service-url=" + httpsURL,
-			Hint:    "Heuristic match (Subject == Issuer) — AfterTouch's own CA wasn't loadable, so this is a best guess. If wrong, treat the chain as foreign.",
-		})
-	default:
-		hints = append(hints, ManualCommand{
-			Label:   "Investigate the chain manually:",
-			Command: fmt.Sprintf("openssl s_client -connect %s -servername %s -showcerts </dev/null", addr, host),
-			Hint:    "Run from the same host as the service. Shows the full chain the peer is serving — likely a reverse proxy or ingress cert.",
-		})
-	}
+		return []Finding{{
+			Severity: SeverityInfo,
+			Message:  fmt.Sprintf("AfterTouch is serving its own self-signed CA chain on %s (expected).", addr),
+			Details: "The service host's system trust store doesn't include AfterTouch's CA — by design. Speakers establish trust via `setup install-ca`, not via system roots. This finding is informational; nothing is wrong with the service. " +
+				chainContext,
+			ManualCommands: []ManualCommand{{
+				Label:   "Reminder — each speaker still needs AfterTouch's CA installed once:",
+				Command: "soundtouch-cli --host=<speaker-ip> setup install-ca --service-url=" + httpsURL,
+				Hint:    "Verified by signature: the leaf was issued by AfterTouch's own CA. Only run install-ca for speakers that haven't been migrated yet.",
+			}},
+		}}
 
-	return []Finding{{
-		Severity:       SeverityWarning,
-		Message:        fmt.Sprintf("HTTPS certificate at %s does not validate against system roots.", addr),
-		Details:        details,
-		ManualCommands: hints,
-	}}
+	case leafSubjectEqualsIssuer:
+		return []Finding{{
+			Severity: SeverityInfo,
+			Message:  fmt.Sprintf("HTTPS endpoint on %s is serving a self-signed certificate.", addr),
+			Details: "AfterTouch's own CA couldn't be loaded to verify the leaf's signature, so this is a heuristic match (Subject == Issuer). If this *is* AfterTouch's self-signed chain, the situation is normal and speakers trust it via `setup install-ca`. If it's some other self-signed cert (custom proxy, etc.), treat the openssl investigation command below as the primary action. " +
+				chainContext,
+			ManualCommands: []ManualCommand{
+				{
+					Label:   "If this is AfterTouch's CA, install it on each speaker:",
+					Command: "soundtouch-cli --host=<speaker-ip> setup install-ca --service-url=" + httpsURL,
+					Hint:    "Heuristic match — verify the served Issuer matches AfterTouch's CA before running.",
+				},
+				{
+					Label:   "Or inspect the served chain manually:",
+					Command: fmt.Sprintf("openssl s_client -connect %s -servername %s -showcerts </dev/null", addr, host),
+					Hint:    "Run from the same host as the service.",
+				},
+			},
+		}}
+
+	default:
+		return []Finding{{
+			Severity: SeverityWarning,
+			Message:  fmt.Sprintf("HTTPS endpoint on %s serves a chain that doesn't validate against system roots and wasn't issued by AfterTouch's CA.", addr),
+			Details: fmt.Sprintf("Unexpected chain — likely a reverse proxy or ingress cert. Verification error: %v. ", err) +
+				chainContext,
+			ManualCommands: []ManualCommand{{
+				Label:   "Inspect the chain manually:",
+				Command: fmt.Sprintf("openssl s_client -connect %s -servername %s -showcerts </dev/null", addr, host),
+				Hint:    "Run from the same host as the service. Shows the full chain the peer is serving.",
+			}},
+		}}
+	}
 }
 
 func splitHTTPSHostPort(raw string) (string, string) {
diff --git a/pkg/service/health/checks_cert_chain_test.go b/pkg/service/health/checks_cert_chain_test.go
index d61e67d..aab0514 100644
--- a/pkg/service/health/checks_cert_chain_test.go
+++ b/pkg/service/health/checks_cert_chain_test.go
@@ -42,34 +42,49 @@ func TestCertChain_SelfSigned_SubjectEqualsIssuerFallback(t *testing.T) {
 	defer srv.Close()
 
 	// No CA provided → fallback to Subject==Issuer heuristic.
+	// This is informational, not a warning — a self-signed
+	// AfterTouch chain is the expected default deployment shape.
 	got := runCertChainCheck(srv.URL, nil)
-	if len(got) != 1 || got[0].Severity != SeverityWarning {
-		t.Fatalf("expected one warning for self-signed cert, got %+v", got)
+	if len(got) != 1 || got[0].Severity != SeverityInfo {
+		t.Fatalf("expected one info finding for self-signed cert, got %+v", got)
 	}
 
 	if !strings.Contains(got[0].Details, "Issuer") {
 		t.Errorf("expected issuer detail, got %q", got[0].Details)
 	}
 
-	if len(got[0].ManualCommands) == 0 {
-		t.Fatalf("expected at least one manual command")
+	if !strings.Contains(got[0].Details, "heuristic") {
+		t.Errorf("expected heuristic disclosure in details, got %q", got[0].Details)
 	}
 
-	cmd := got[0].ManualCommands[0].Command
-	if !strings.Contains(cmd, "install-ca") {
-		t.Errorf("expected install-ca suggestion via Subject==Issuer heuristic, got %q", cmd)
+	if len(got[0].ManualCommands) < 2 {
+		t.Fatalf("expected at least install-ca + openssl commands, got %+v", got[0].ManualCommands)
 	}
 
-	hint := got[0].ManualCommands[0].Hint
-	if !strings.Contains(hint, "Heuristic") {
-		t.Errorf("expected hint to disclose the heuristic match, got %q", hint)
+	var sawInstallCA, sawOpenssl bool
+	for _, c := range got[0].ManualCommands {
+		if strings.Contains(c.Command, "install-ca") {
+			sawInstallCA = true
+		}
+		if strings.Contains(c.Command, "openssl s_client") {
+			sawOpenssl = true
+		}
+	}
+
+	if !sawInstallCA {
+		t.Errorf("expected install-ca suggestion among manual commands")
+	}
+	if !sawOpenssl {
+		t.Errorf("expected openssl investigation command among manual commands")
 	}
 }
 
-func TestCertChain_LeafSignedByOwnCA_PrefersInstallCA(t *testing.T) {
-	// Construct a CA + leaf signed by it. Leaf has SAN 127.0.0.1
-	// so SNI works; Subject != Issuer (different CommonNames),
-	// which would have fooled the old heuristic.
+func TestCertChain_LeafSignedByOwnCA_IsInformationalNotAWarning(t *testing.T) {
+	// AfterTouch's own CA is the *default* deployment shape.
+	// Calling that a warning would mislead non-technical users.
+	// The check should report INFO, explain the situation, and
+	// remind to install-ca on speakers — but not present the
+	// service host's lack of trust as a defect.
 	caTLS, ca := generateInternalCA(t)
 	leafTLS := generateLeafSignedBy(t, ca, caTLS.PrivateKey)
 
@@ -81,20 +96,32 @@ func TestCertChain_LeafSignedByOwnCA_PrefersInstallCA(t *testing.T) {
 	defer srv.Close()
 
 	got := runCertChainCheck(srv.URL, func() *x509.Certificate { return ca })
-	if len(got) != 1 || got[0].Severity != SeverityWarning {
-		t.Fatalf("expected one warning, got %+v", got)
+	if len(got) != 1 {
+		t.Fatalf("expected one finding, got %+v", got)
+	}
+
+	if got[0].Severity != SeverityInfo {
+		t.Errorf("expected SeverityInfo for AfterTouch's own CA chain, got %q", got[0].Severity)
+	}
+
+	if !strings.Contains(got[0].Message, "expected") {
+		t.Errorf("expected message to call this state 'expected', got %q", got[0].Message)
+	}
+
+	if !strings.Contains(got[0].Details, "by design") {
+		t.Errorf("expected details to explain it's by design, got %q", got[0].Details)
 	}
 
 	if len(got[0].ManualCommands) == 0 {
-		t.Fatalf("expected a manual command")
+		t.Fatalf("expected install-ca reminder among manual commands")
 	}
 
 	cmd := got[0].ManualCommands[0]
 	if !strings.Contains(cmd.Command, "install-ca") {
-		t.Errorf("expected install-ca, got %q", cmd.Command)
+		t.Errorf("expected install-ca reminder, got %q", cmd.Command)
 	}
 
-	if !strings.Contains(cmd.Hint, "verified by signature") {
+	if !strings.Contains(cmd.Hint, "Verified by signature") {
 		t.Errorf("expected signature-verified hint, got %q", cmd.Hint)
 	}
 }