v0.137.0: guard empty-email notification save (prevents alert-delivery wipe)

Saving the notifications form with a blank email box while events are enabled wiped the customer's hub-side alert address (SyncPreferences pushed empty) — the 2026-07-15 demo incident. settingsNotificationsHandler now refuses that save before SetNotificationPrefs + hub sync, re-renders a Hungarian error, and repaints the submitted checkboxes. Empty email + zero events (clear-all) still proceeds. Tests + red-proof (remove guard -> stored email wiped to empty).
2026-07-15 19:40:01 +02:00
parent 0ef6648e12
commit c124d0adf2
4 changed files with 201 additions and 0 deletions
@@ -1423,6 +1423,29 @@ func (s *Server) settingsNotificationsHandler(w http.ResponseWriter, r *http.Req
 		enabledEvents = append(enabledEvents, "expected_backup_missed", "expected_dbdump_missed")
 	}

+	// EMPTY-EMAIL WIPE GUARD (2026-07-15 demo incident): a blank email box saved while events are
+	// still enabled would store an empty Email AND push it to the hub via SyncPreferences, wiping
+	// the customer's provisioning-seeded alert address — enabled events with nowhere to send them.
+	// The only way to reach this state is the bug, so refuse the save outright (BEFORE
+	// SetNotificationPrefs and BEFORE any hub sync), leaving the stored email untouched, and ask for
+	// an address. The intentional "turn everything off" case (empty email + ZERO events) falls
+	// through below — clearing the email is legitimate there and the empty hub push is correct.
+	if email == "" && len(enabledEvents) > 0 {
+		s.logger.Printf("[WARN] [web] Refused notification save: empty email with %d enabled event(s) — would wipe hub-side alert delivery", len(enabledEvents))
+		data := s.notificationsPageData()
+		// Repaint the customer's just-submitted intent (their ticked events + chosen cooldown, empty
+		// email) so they only need to add an address, not re-tick everything. Overlay the stored
+		// prefs — do NOT persist this; it is render-only.
+		data["NotificationPrefs"] = &settings.NotificationPrefs{
+			Email:         email,
+			EnabledEvents: enabledEvents,
+			CooldownHours: cooldownHours,
+		}
+		data["NotificationError"] = "Adj meg egy értesítési e-mail címet – bekapcsolt értesítésekhez szükséges egy cím, ahova küldhetjük őket."
+		s.executeTemplate(w, r, "settings_notifications", data)
+		return
+	}
+
 	prefs := &settings.NotificationPrefs{
 		Email:         email,
 		EnabledEvents: enabledEvents,
@@ -0,0 +1,150 @@
+package web
+
+// The empty-email wipe guard (Part 1, 2026-07-15 demo incident): saving notification prefs with a
+// blank email box while events are still enabled must be REFUSED before it can store an empty email
+// and push it to the hub (wiping the customer's provisioning-seeded alert address). Non-hollow: the
+// guard is asserted by the REAL stored state (a temp-file Settings) — the email survives the refused
+// save — plus the rendered Hungarian error and the repainted (overlaid) submission.
+
+import (
+	"io"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/stacks"
+)
+
+const guardErrSubstr = "bekapcsolt értesítésekhez szükséges" // distinctive slice of the exact Hungarian error
+
+func notifyGuardServer(t *testing.T) (*Server, *settings.Settings) {
+	t.Helper()
+	lg := log.New(io.Discard, "", 0)
+	dir := t.TempDir()
+	cfg := &config.Config{}
+	cfg.Customer.ID = "c1"
+	cfg.Customer.Name = "Teszt"
+	cfg.Customer.Domain = "example.hu"
+	cfg.Paths.StacksDir = filepath.Join(dir, "stacks")
+	cfg.Paths.DataDir = filepath.Join(dir, "data")
+	cfg.Stacks.ComposeCommand = "docker compose"
+	cfg.Web.SessionSecret = "test-session-secret-abcdef"
+	cfg.Hub.Enabled = true // the notifications form (+ error/checkbox render) is gated on HubEnabled
+
+	sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg)
+	if err != nil {
+		t.Fatalf("settings: %v", err)
+	}
+	mgr, err := stacks.NewManager(cfg, lg)
+	if err != nil {
+		t.Fatalf("stacks: %v", err)
+	}
+	// notifier is left nil: the handler's sync path is guarded by `s.notifier != nil` — the guard
+	// under test returns long before it anyway.
+	s := &Server{cfg: cfg, settings: sett, stackMgr: mgr, logger: lg, version: "test"}
+	s.loadTemplates()
+	return s, sett
+}
+
+func postNotifications(t *testing.T, s *Server, form url.Values) *httptest.ResponseRecorder {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/settings/notifications", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	rr := httptest.NewRecorder()
+	s.settingsNotificationsHandler(rr, req)
+	return rr
+}
+
+// Guard fires: empty email + >=1 enabled event → refused; stored email UNCHANGED (not wiped), the
+// Hungarian error rendered, and the just-submitted (not the stored) events repainted.
+// Red-proof: delete the guard block → the stored email becomes "" (the wipe) and this test fails.
+func TestNotificationsGuard_RefusesEmptyEmailWithEvents(t *testing.T) {
+	s, sett := notifyGuardServer(t)
+	// Seed a provisioning-set email + a DIFFERENT event than we submit (so the repaint assertion
+	// distinguishes the overlaid submission from the stored prefs).
+	if err := sett.SetNotificationPrefs(&settings.NotificationPrefs{
+		Email: "seed@felhom.eu", EnabledEvents: []string{"node_down"}, CooldownHours: 6,
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	rr := postNotifications(t, s, url.Values{
+		"notification_email":  {""},
+		"event_backup_failed": {"on"},
+		"cooldown_hours":      {"6"},
+	})
+
+	// The wipe was prevented: stored email + events are untouched (SetNotificationPrefs not called).
+	got := sett.GetNotificationPrefs()
+	if got.Email != "seed@felhom.eu" {
+		t.Fatalf("stored email = %q, want seed@felhom.eu — the guard must not wipe it", got.Email)
+	}
+	if len(got.EnabledEvents) != 1 || got.EnabledEvents[0] != "node_down" {
+		t.Errorf("stored events = %v, want [node_down] unchanged", got.EnabledEvents)
+	}
+	body := rr.Body.String()
+	if !strings.Contains(body, guardErrSubstr) {
+		t.Errorf("response missing the Hungarian guard error (len=%d)", len(body))
+	}
+	// The overlay repaints the SUBMITTED intent: backup_failed checked, seeded node_down NOT.
+	if !strings.Contains(body, `event_backup_failed" checked`) {
+		t.Errorf("submitted event backup_failed not repainted as checked")
+	}
+	if strings.Contains(body, `event_node_down" checked`) {
+		t.Errorf("stored node_down leaked into the re-render — overlay not applied")
+	}
+}
+
+// Legitimate clear-all: empty email + ZERO events → the save proceeds and intentionally clears the
+// email (the hub push of an empty address is correct there).
+func TestNotificationsGuard_AllowsEmptyEmailZeroEvents(t *testing.T) {
+	s, sett := notifyGuardServer(t)
+	if err := sett.SetNotificationPrefs(&settings.NotificationPrefs{
+		Email: "seed@felhom.eu", EnabledEvents: []string{"backup_failed"}, CooldownHours: 6,
+	}); err != nil {
+		t.Fatal(err)
+	}
+
+	rr := postNotifications(t, s, url.Values{
+		"notification_email": {""},
+		"cooldown_hours":     {"6"},
+	}) // no event_* checkboxes
+
+	// The save PROCEEDED (guard did not fire): the email is intentionally cleared. (Note:
+	// GetNotificationPrefs re-defaults an empty EnabledEvents list on read, so the stored-empty
+	// events are not asserted here — the email clear is the observable proof the save ran.)
+	if p := sett.GetNotificationPrefs(); p.Email != "" {
+		t.Errorf("email = %q, want cleared (legitimate turn-everything-off)", p.Email)
+	}
+	if strings.Contains(rr.Body.String(), guardErrSubstr) {
+		t.Errorf("guard wrongly fired on a legitimate clear-all")
+	}
+}
+
+// Normal save (regression guard): non-empty email + events persist unchanged.
+func TestNotificationsGuard_NormalSavePersists(t *testing.T) {
+	s, sett := notifyGuardServer(t)
+
+	postNotifications(t, s, url.Values{
+		"notification_email":  {"new@felhom.eu"},
+		"event_backup_failed": {"on"},
+		"cooldown_hours":      {"12"},
+	})
+
+	p := sett.GetNotificationPrefs()
+	if p.Email != "new@felhom.eu" {
+		t.Errorf("email = %q, want new@felhom.eu", p.Email)
+	}
+	if len(p.EnabledEvents) != 1 || p.EnabledEvents[0] != "backup_failed" {
+		t.Errorf("events = %v, want [backup_failed]", p.EnabledEvents)
+	}
+	if p.CooldownHours != 12 {
+		t.Errorf("cooldown = %d, want 12", p.CooldownHours)
+	}
+}