fix(security): mask alert secrets in the TUI detail panel and table

The alert detail panel dumped a.Settings raw — SMTP passwords, bot tokens, API keys on screen and into any recording or screen share. The table view leaked the PagerDuty routing key, Pushover user key, and full discord/slack/webhook URLs (the URL path is the credential). The redaction allowlist moves from internal/server to models.RedactAlertSettings so the backup export and the TUI render through one policy. Panel keys are sorted so rows stop reshuffling every tick; webhook URLs show scheme+host only; keys show first4…last4.
2026-06-11 12:26:40 -04:00
parent a3711c652c
commit a1ab276bc5
3 changed files with 149 additions and 9 deletions
@@ -0,0 +1,36 @@
+package models
+
+// safeAlertSettingKeys lists, per provider type, the alert settings that are
+// NOT secret and may be shown or exported in the clear. Everything else is
+// redacted. Providers absent from this map (discord, slack, webhook, pushover)
+// carry their secret in a field a denylist would miss — the webhook URL, the
+// pushover token/user — so all of their settings are redacted.
+var safeAlertSettingKeys = map[string]map[string]bool{
+	"email":     {"host": true, "port": true, "to": true, "from": true},
+	"ntfy":      {"topic": true, "priority": true},
+	"telegram":  {"chat_id": true},
+	"pagerduty": {"severity": true},
+	"gotify":    {"priority": true},
+	"opsgenie":  {"priority": true, "eu": true},
+}
+
+// RedactAlertSettings keeps only the known-safe keys for the alert type and
+// redacts everything else. An allowlist fails safe: an unknown or newly added
+// setting is redacted by default instead of leaking. Shared by the backup
+// export path and the TUI alert detail panel so both render through the same
+// policy.
+func RedactAlertSettings(alertType string, settings map[string]string) map[string]string {
+	safe := safeAlertSettingKeys[alertType]
+	redacted := make(map[string]string, len(settings))
+	for k, v := range settings {
+		switch {
+		case v == "":
+			redacted[k] = ""
+		case safe[k]:
+			redacted[k] = v
+		default:
+			redacted[k] = "***REDACTED***"
+		}
+	}
+	return redacted
+}
@@ -2,8 +2,11 @@ package tui

 import (
 	"fmt"
+	neturl "net/url"
+	"sort"
 	"strings"

+	"gitea.lerkolabs.com/lerkolabs/uptop/internal/models"
 	"gitea.lerkolabs.com/lerkolabs/uptop/internal/monitor"
 	tea "github.com/charmbracelet/bubbletea"
 	"github.com/charmbracelet/huh"
@@ -100,15 +103,17 @@ func (m Model) fmtAlertConfig(alert struct {
 		return m.st.subtleStyle.Render("—")
 	case "pagerduty":
 		if key := alert.Settings["routing_key"]; key != "" {
-			return limitStr(key, 34)
+			return limitStr(maskSecret(key), 34)
 		}
 		return m.st.subtleStyle.Render("—")
 	case "pushover":
 		if user := alert.Settings["user"]; user != "" {
-			return limitStr(fmt.Sprintf("user:%s", user), 34)
+			return limitStr(fmt.Sprintf("user:%s", maskSecret(user)), 34)
 		}
 		return m.st.subtleStyle.Render("—")
 	case "gotify":
+		// The gotify server URL identifies the target; the token is the
+		// secret and is never shown here.
 		if url := alert.Settings["url"]; url != "" {
 			return limitStr(url, 34)
 		}
@@ -116,10 +121,7 @@ func (m Model) fmtAlertConfig(alert struct {
 	case "opsgenie":
 		key := alert.Settings["api_key"]
 		if key != "" {
-			masked := key
-			if len(masked) > 8 {
-				masked = masked[:4] + "…" + masked[len(masked)-4:]
-			}
+			masked := maskSecret(key)
 			if alert.Settings["eu"] == "true" {
 				return limitStr(fmt.Sprintf("EU %s", masked), 34)
 			}
@@ -127,13 +129,33 @@ func (m Model) fmtAlertConfig(alert struct {
 		}
 		return m.st.subtleStyle.Render("—")
 	default:
-		if val, ok := alert.Settings["url"]; ok {
-			return limitStr(val, 34)
+		// discord/slack/webhook: the URL path IS the credential — show only
+		// enough to identify the target.
+		if val, ok := alert.Settings["url"]; ok && val != "" {
+			return limitStr(maskWebhookURL(val), 34)
 		}
 		return m.st.subtleStyle.Render("—")
 	}
 }

+// maskSecret keeps just enough of a credential to identify it.
+func maskSecret(s string) string {
+	if len(s) > 8 {
+		return s[:4] + "…" + s[len(s)-4:]
+	}
+	return "●●●●●●●●"
+}
+
+// maskWebhookURL shows scheme and host only. For discord, slack, and generic
+// webhooks the URL path carries the token, so the path is never rendered.
+func maskWebhookURL(raw string) string {
+	u, err := neturl.Parse(raw)
+	if err != nil || u.Host == "" {
+		return "●●●●●●●●"
+	}
+	return u.Scheme + "://" + u.Host + "/…"
+}
+
 func (m Model) fmtAlertHealth(h monitor.AlertHealth) string {
 	if h.LastSendAt.IsZero() {
 		return m.st.subtleStyle.Render("●")
@@ -229,7 +251,21 @@ func (m Model) viewAlertDetailPanel() string {

 	b.WriteString(m.divider() + "\n")
 	b.WriteString(m.st.subtleStyle.Render("  CONFIGURATION") + "\n")
-	for k, v := range a.Settings {
+	// Render through the same allowlist the backup export uses — this panel
+	// ends up in screen shares and asciinema recordings. Keys are sorted so
+	// rows don't reshuffle every render.
+	redacted := models.RedactAlertSettings(a.Type, a.Settings)
+	keys := make([]string, 0, len(redacted))
+	for k := range redacted {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		v := redacted[k]
+		if v == "***REDACTED***" {
+			row(k, m.st.subtleStyle.Render("●●●●●●●●"))
+			continue
+		}
 		row(k, v)
 	}

@@ -0,0 +1,68 @@
+package tui
+
+import (
+	"strings"
+	"testing"
+
+	"gitea.lerkolabs.com/lerkolabs/uptop/internal/models"
+)
+
+func TestAlertDetailPanel_MasksSecretsStableOrder(t *testing.T) {
+	m := newTestModel(&tuiMockStore{})
+	m.termWidth, m.termHeight = 120, 40
+	m.alerts = []models.AlertConfig{{
+		ID: 1, Name: "ops", Type: "email",
+		Settings: map[string]string{
+			"host": "smtp.example.com",
+			"port": "587",
+			"user": "oncall@example.com",
+			"pass": "hunter2-secret",
+			"to":   "team@example.com",
+		},
+	}}
+	m.cursor = 0
+
+	out := m.viewAlertDetailPanel()
+	if strings.Contains(out, "hunter2-secret") {
+		t.Error("SMTP password rendered in alert detail panel")
+	}
+	if strings.Contains(out, "oncall@example.com") {
+		t.Error("SMTP user (not on the allowlist) rendered in alert detail panel")
+	}
+	if !strings.Contains(out, "smtp.example.com") {
+		t.Error("allowlisted setting (host) missing from panel")
+	}
+
+	// Map iteration must not reshuffle rows between renders.
+	for i := 0; i < 5; i++ {
+		if m.viewAlertDetailPanel() != out {
+			t.Fatal("panel output unstable across renders — settings keys not sorted")
+		}
+	}
+}
+
+func TestFmtAlertConfig_MasksSecrets(t *testing.T) {
+	m := newTestModel(&tuiMockStore{})
+
+	webhook := m.fmtAlertConfig(struct {
+		Type     string
+		Settings map[string]string
+	}{"discord", map[string]string{"url": "https://discord.com/api/webhooks/123456/SeCrEtToKeN"}})
+	if strings.Contains(webhook, "SeCrEtToKeN") || strings.Contains(webhook, "123456") {
+		t.Errorf("webhook URL path (the credential) rendered in table: %q", webhook)
+	}
+	if !strings.Contains(webhook, "discord.com") {
+		t.Errorf("webhook host missing from table config: %q", webhook)
+	}
+
+	pd := m.fmtAlertConfig(struct {
+		Type     string
+		Settings map[string]string
+	}{"pagerduty", map[string]string{"routing_key": "R0123456789ABCDEFGHIJ"}})
+	if strings.Contains(pd, "R0123456789ABCDEFGHIJ") {
+		t.Errorf("pagerduty routing key rendered raw in table: %q", pd)
+	}
+	if !strings.Contains(pd, "R012") || !strings.Contains(pd, "GHIJ") {
+		t.Errorf("masked routing key should keep identifying ends: %q", pd)
+	}
+}