diff --git a/CHANGELOG.md b/CHANGELOG.md index c2d7942..88bc047 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ ## Changelog +### v0.137.0 — empty-email notification save guard (data-loss fix) (2026-07-15) + +Fixes a silent alert-delivery wipe demonstrated on the demo customer on 2026-07-15: saving the +Értesítések form with a **blank e-mail box while events were still enabled** dropped an empty +`Email` into the prefs AND pushed it to the hub (`SyncPreferences`), overwriting the customer's +provisioning-seeded alert address — the "Kedves Ügyfél!" delivery path went dark until it was +restored by hand in 6D (P3-DELIVERY). + +- **`web/handlers.go` `settingsNotificationsHandler`:** after computing the trimmed email + enabled + events, a guard refuses the save when `email == "" && len(enabledEvents) > 0` — it returns + **before** `SetNotificationPrefs` and **before** any hub sync, re-rendering the page with a + Hungarian error ("Adj meg egy értesítési e-mail címet – …") and repainting the just-submitted + checkboxes (an overlay on `notificationsPageData`'s `NotificationPrefs`, render-only). Enabled + events with no address is a purely destructive state reachable only via the bug. The legitimate + **empty-email + ZERO events** clear-all still proceeds (the empty hub push is correct there). +- **Deliberately NOT** an HTML `required` attr on the input — `required` is unconditional and would + block the legitimate clear-all case; the server-side guard is the correct, precisely-conditional + floor. `SyncPreferences` / the hub side / the seed-migration are untouched. +- **Tests (`web/notifications_guard_test.go`):** guard-fires (stored email survives — the wipe is + prevented; red-proofed: remove the guard → the email is wiped to `""`), legitimate clear-all + proceeds, normal save persists. Real temp-file `Settings` (non-hollow: asserts stored state). + ### v0.136.0 — `.fab` exclusion scoping: classes in the manual export (Task 4) (2026-07-15) Task 4 — the `.fab` column of the matrix (architecture §2; the SQ5 exclusion-scoping verdict + Viktor diff --git a/controller/README.md b/controller/README.md index caaef85..5bd2cf2 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1436,6 +1436,12 @@ Notification preferences (email, enabled events, cooldown hours) are: - Synced to Hub on save and on controller startup via `POST /api/v1/preferences` - Hub sync failure doesn't block local save +**Empty-email save guard (v0.137.0):** the save handler REFUSES a submit with a blank e-mail box +while any event is still enabled (it would store an empty address AND push it to the hub, wiping the +provisioning-seeded alert delivery). The form re-renders with a Hungarian error and the customer's +ticked events preserved; no save, no sync. Clearing the e-mail with **zero** events enabled is +allowed (an intentional turn-everything-off). + --- ### 7. Update Management diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index d6d9875..cda6973 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -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, diff --git a/controller/internal/web/notifications_guard_test.go b/controller/internal/web/notifications_guard_test.go new file mode 100644 index 0000000..6c05ced --- /dev/null +++ b/controller/internal/web/notifications_guard_test.go @@ -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) + } +}