hub v0.79.0 — R-97c: make the operator-only claim true
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user