From 2c0e43e0d0f3ee494bb0482a5c96d86e4e669828 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 27 Jul 2026 17:54:47 +0200 Subject: [PATCH] =?UTF-8?q?hub=20v0.79.0=20=E2=80=94=20R-97c:=20make=20the?= =?UTF-8?q?=20operator-only=20claim=20true?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.78.0 asserted in a comment that a type with no customerMessages entry cannot reach a customer. It can: templates.go falls back to the raw message when the entry is missing, and the only customer gate is prefs.EnabledEvents — pure configuration. A customer with whole_guest_backup_failed enabled would have been emailed raw English operator text about a backup they cannot act on. The new test proves it against the v0.78.0 shape. operatorOnlyEvents is now an explicit register checked before prefs, logging a skipped/operator_only row so the skip is visible. NOT implemented as 'missing customerMessages blocks delivery' — several types rely on that fallback on purpose. The handler comment now names the real mechanism. --- hub/CHANGELOG.md | 37 ++++++ hub/internal/api/handler.go | 11 +- hub/internal/notify/dispatcher.go | 40 ++++++ hub/internal/notify/operator_only_test.go | 142 ++++++++++++++++++++++ manifests/hub.yaml | 2 +- 5 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 hub/internal/notify/operator_only_test.go diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index f5306dc..7038007 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,5 +1,42 @@ # Felhom Hub — Changelog +## v0.79.0 — R-97c: make the operator-only claim TRUE (2026-07-27) + +v0.78.0 shipped a comment asserting that `whole_guest_backup_failed` / `_recovered` were operator-only +because they have no `customerMessages` entry, "so the dispatcher **structurally cannot** route them +to a customer". **That was false**, and the code says so plainly: + +- `templates.go` treats a missing entry as a **fallback to the raw message**, not a block — + `hunMessage := customerMessages[eventType]; if hunMessage == "" { hunMessage = message }`; +- the only customer gate is `isEventEnabled(prefs.EnabledEvents, ...)` — **configuration**. + +So a customer with `whole_guest_backup_failed` in their enabled list and an email set would have been +sent the raw English operator text about a backup they can take no action on. Proven by running the +new test against the v0.78.0 shape: it emails `customer@example.com`. + +This is the `EffectiveProtected` shape — a doc comment claiming a property the code stopped +providing, which is how the samba false alarm survived. + +**The fix:** an explicit `operatorOnlyEvents` register, checked at the top of `processCustomer` +**before prefs are consulted**, so no customer configuration can opt in. The skip is **logged** +(`status=skipped`, `error_message=operator_only`, `channel=customer`) rather than dropped — a silent +drop is indistinguishable from a delivery that never happened. + +Deliberately **not** implemented as "a missing `customerMessages` entry blocks delivery": several +types rely on the raw-message fallback on purpose (`offbox_enlarge_blocked`'s dynamic Hungarian text +is customer-grade and a template would discard its numbers), so turning the fallback into a gate +would change behaviour well outside this concern. + +The recovery type is listed too, even though its customer leg is pairing-gated on a "sent" row that +cannot exist — relying on that would make one type's safety a consequence of another type's routing, +true today and silently untrue the moment the failed event became customer-visible. + +The `handler.go` comment now states the actual mechanism and warns that allowlisting a type does not +make it operator-only. + +Tests +4, all run under the **breaking** configuration (customer has the event enabled AND an email), +not today's safe one. 17 packages ok. + ## v0.78.0 — R-97a: the whole-guest backup tier gets a voice (operator-only) (2026-07-27) `internal/quiesce` had no route to the hub at all. On 2026-07-27 three failed whole-guest backups and diff --git a/hub/internal/api/handler.go b/hub/internal/api/handler.go index cf1f6bf..9ee92f9 100644 --- a/hub/internal/api/handler.go +++ b/hub/internal/api/handler.go @@ -1562,9 +1562,14 @@ var allowedEventTypes = map[string]bool{ // DELIBERATELY NOT `backup_failed`/`backup_completed`. Those two carry customerMessages entries // AND sit in demo-felhom's live enabled_events, so reusing them would email the CUSTOMER, in // Hungarian, that their backup failed — while it is still retrying behind the R-88 breaker. A - // customer can take no action on a failed whole-guest backup. These follow the R-85 pattern - // instead: allowlisted, with NO customerMessages entry, so the dispatcher structurally cannot - // route them to a customer. Do NOT add customerMessages entries without a copy review. + // customer can take no action on a failed whole-guest backup. + // + // OPERATOR-ONLY IS ENFORCED BY `notify.operatorOnlyEvents`, NOT by the absence of a + // customerMessages entry. v0.78.0 claimed the latter and was WRONG (corrected in v0.79.0/R-97c): + // `FormatCustomerEmail` treats a missing entry as a fallback to the raw message, and the only + // customer gate is `prefs.EnabledEvents` — configuration, which a customer or a future code path + // can change. The register is checked before customer dispatch and logs a `skipped/operator_only` + // row. Adding a type here does NOT make it operator-only; add it to that register too. "whole_guest_backup_failed": true, "whole_guest_backup_recovered": true, diff --git a/hub/internal/notify/dispatcher.go b/hub/internal/notify/dispatcher.go index ee3d40c..d47457a 100644 --- a/hub/internal/notify/dispatcher.go +++ b/hub/internal/notify/dispatcher.go @@ -285,7 +285,47 @@ func (d *Dispatcher) processOperator(customerID, eventType, severity, message, d d.store.LogNotification(customerID, eventType, severity, message, "sent", "", "operator") } +// operatorOnlyEvents are event types that must NEVER reach a customer, whatever their preferences say. +// +// R-97c. This register exists because the guarantee it provides was previously ASSERTED IN A COMMENT +// and not implemented. The claim was that a type with no `customerMessages` entry "structurally +// cannot" be routed to a customer. It cannot: `FormatCustomerEmail` (templates.go) treats a missing +// entry as a **fallback to the raw message**, not a block — +// +// hunMessage := customerMessages[eventType] +// if hunMessage == "" { hunMessage = message } +// +// — and the only customer gate is `isEventEnabled(prefs.EnabledEvents, ...)`, i.e. CONFIGURATION. +// So a customer with `whole_guest_backup_failed` in their enabled list and an email set would have +// received the raw English operator text about a backup they can take no action on. +// +// That is the `EffectiveProtected` shape: a doc comment claiming a property the code stopped +// providing, which is how the samba false alarm survived. The register makes the claim true. +// +// NOT implemented as "a missing customerMessages entry blocks delivery" — several existing types rely +// on the raw-message fallback deliberately (e.g. offbox_enlarge_blocked, whose dynamic Hungarian text +// is customer-grade and would be DISCARDED by a template). Turning the fallback into a gate would +// change behaviour well outside this concern. +var operatorOnlyEvents = map[string]bool{ + // R-97a. A customer can take no action on a failed whole-guest backup, and being told it failed + // while it is still retrying behind the R-88 breaker is alarming without being actionable. + "whole_guest_backup_failed": true, + // The recovery is ALSO listed, even though its customer leg is pairing-gated on a customer-channel + // "sent" row that can never exist for the line above. Relying on that would make this type's safety + // a consequence of another type's routing — true today, and silently untrue the moment the failed + // event becomes customer-visible. Belt, not inference. + "whole_guest_backup_recovered": true, +} + func (d *Dispatcher) processCustomer(customerID, eventType, severity, message, detailsJSON, source string) { + // R-97c: operator-tier events stop here, BEFORE prefs are consulted — the point is that no + // customer configuration can opt in. Logged rather than dropped, so the skip is visible in + // notification_log instead of looking like a delivery that never happened. + if operatorOnlyEvents[eventType] { + d.store.LogNotification(customerID, eventType, severity, message, "skipped", "operator_only", "customer") + return + } + // Check if customer is blocked if d.store.IsCustomerBlocked(customerID) { return diff --git a/hub/internal/notify/operator_only_test.go b/hub/internal/notify/operator_only_test.go new file mode 100644 index 0000000..f231819 --- /dev/null +++ b/hub/internal/notify/operator_only_test.go @@ -0,0 +1,142 @@ +package notify + +import ( + "io" + "log" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-hub/internal/store" +) + +// R-97c — the operator-only claim must be TRUE, not asserted. +// +// v0.78.0 committed a comment saying a type with no `customerMessages` entry "structurally cannot" +// reach a customer. It can: templates.go falls back to the raw message when the entry is missing, and +// the only customer gate is `prefs.EnabledEvents` — configuration. This suite runs under exactly the +// configuration that would break it. + +func opOnlyStore(t *testing.T) *store.Store { + t.Helper() + st, err := store.New(filepath.Join(t.TempDir(), "oo.db"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { st.Close() }) + if err := st.SaveCustomerConfig(&store.CustomerConfig{CustomerID: "c1", APIKey: "k", RetrievalPassword: "p"}); err != nil { + t.Fatal(err) + } + return st +} + +// sentTo records every address the dispatcher actually tried to email. +type sentTo struct{ to []string } + +func opOnlyDispatcher(t *testing.T, st *store.Store, rec *sentTo) *Dispatcher { + t.Helper() + d := NewDispatcher(st, "test-key", "from@felhom.eu", "operator@felhom.eu", true, log.New(io.Discard, "", 0)) + d.sendEmailFn = func(to, subject, body string, headers map[string]string) error { + rec.to = append(rec.to, to) + return nil + } + return d +} + +// SCENARIO E — the breaking configuration: the customer HAS the event enabled and HAS an email. +// +// COMPANION RED-PROOF (observed): delete the operatorOnlyEvents check from processCustomer (the +// v0.78.0 shape) and this fails with +// +// "R-97c: a customer was emailed an OPERATOR-ONLY event (customer@example.com) — enabled_events +// must not be able to opt in" +// +// Restored. +func TestOperatorOnly_CustomerCannotOptIn(t *testing.T) { + st := opOnlyStore(t) + // THE BREAKING CONFIG — not today's safe one. + if err := st.SaveNotificationPrefs("c1", "customer@example.com", + []string{"whole_guest_backup_failed", "backup_failed"}, 6); err != nil { + t.Fatalf("SaveNotificationPrefs: %v", err) + } + + rec := &sentTo{} + d := opOnlyDispatcher(t, st, rec) + d.ProcessEvent("c1", "whole_guest_backup_failed", "error", + "Whole-guest backup FAILED on the felhom-pbs tier", `{"tier":"felhom-pbs"}`, "controller") + + for _, to := range rec.to { + if to == "customer@example.com" { + t.Fatalf("R-97c: a customer was emailed an OPERATOR-ONLY event (%s) — "+ + "enabled_events must not be able to opt in", to) + } + } + // The operator MUST still get it — the guard must not silence the signal entirely. + gotOperator := false + for _, to := range rec.to { + if to == "operator@felhom.eu" { + gotOperator = true + } + } + if !gotOperator { + t.Fatal("the operator must still be notified; the guard is customer-only") + } +} + +// The skip must be VISIBLE — a silent drop is indistinguishable from a delivery that never happened. +func TestOperatorOnly_SkipIsLogged(t *testing.T) { + st := opOnlyStore(t) + if err := st.SaveNotificationPrefs("c1", "customer@example.com", []string{"whole_guest_backup_failed"}, 6); err != nil { + t.Fatal(err) + } + rec := &sentTo{} + d := opOnlyDispatcher(t, st, rec) + d.ProcessEvent("c1", "whole_guest_backup_failed", "error", "boom", `{"tier":"local"}`, "controller") + + logs, err := st.GetRecentNotifications("c1", 20) + if err != nil { + t.Fatalf("GetNotificationLog: %v", err) + } + found := false + for _, l := range logs { + if l.Channel == "customer" && l.Status == "skipped" && strings.Contains(l.ErrorMessage, "operator_only") { + found = true + } + } + if !found { + t.Fatalf("the customer skip must be logged as skipped/operator_only so it is visible; got %d row(s)", len(logs)) + } +} + +// A NORMAL customer event must be unaffected — the register is narrow, not a blanket mute. +func TestOperatorOnly_NormalCustomerEventStillDelivered(t *testing.T) { + st := opOnlyStore(t) + if err := st.SaveNotificationPrefs("c1", "customer@example.com", []string{"backup_failed"}, 6); err != nil { + t.Fatal(err) + } + rec := &sentTo{} + d := opOnlyDispatcher(t, st, rec) + d.ProcessEvent("c1", "backup_failed", "error", "app-data backup failed", "{}", "controller") + + got := false + for _, to := range rec.to { + if to == "customer@example.com" { + got = true + } + } + if !got { + t.Fatal("a normal customer-facing event must still be delivered — the register must not be a blanket mute") + } +} + +// Pin the membership: both R-97a types, by name. +func TestOperatorOnly_RegisterContents(t *testing.T) { + for _, et := range []string{"whole_guest_backup_failed", "whole_guest_backup_recovered"} { + if !operatorOnlyEvents[et] { + t.Fatalf("%s must be operator-only; a customer can take no action on it", et) + } + } + if operatorOnlyEvents["backup_failed"] { + t.Fatal("backup_failed is the APP-DATA tier and IS customer-facing — do not mute it") + } +} diff --git a/manifests/hub.yaml b/manifests/hub.yaml index 10047ad..a8d59d5 100644 --- a/manifests/hub.yaml +++ b/manifests/hub.yaml @@ -125,7 +125,7 @@ spec: spec: containers: - name: hub - image: gitea.dooplex.hu/admin/felhom-hub:0.78.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.79.0 ports: - containerPort: 8080 name: http