From d0c1d77491cbbcb91777d0e1420e11caa8bd44f0 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sat, 18 Jul 2026 13:13:51 +0200 Subject: [PATCH] fix(shares): reserved key leaked into the crossdrive_completed hub event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by LIVE VALIDATION, not by a unit test: the first demo tier-2 run pushed 'Masodlagos mentes elkeszult: _shares' — the reserved key reached Hungarian customer/operator copy. Mapped at the SOURCE of the notification (RunSharesTier2's tier2Notify calls) so no future notifier wiring can reintroduce it, plus DisplayStackName at the main.go wiring as idempotent defense in depth. Regression test added with a red-proof. --- controller/cmd/controller/main.go | 8 ++++-- controller/internal/backup/shares_test.go | 30 ++++++++++++++++++++++ controller/internal/backup/tier2_shares.go | 7 +++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 193e39f..ad1173f 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -559,14 +559,18 @@ func main() { // Tier 2: off-drive copy of each HDD app's recovery unit + userdata (auto-enabled, auto-target). // Runs after the DB dump so it copies a fresh unit. backupMgr.SetTier2Notifier(func(stackName, destLabel string, dur time.Duration, err error) { + // DISPLAY BOUNDARY (R-7b): this event's StackName lands in Hungarian operator/customer copy + // ("Másodlagos mentés elkészült: "), so the reserved shares key is mapped here. Caught + // by live validation — the first demo run pushed a literal "_shares" before this line. + display := backup.DisplayStackName(stackName) if err != nil { notifier.NotifyCrossDriveFailed(notify.CrossDriveDetails{ - StackName: stackName, Method: "rsync", DestPath: destLabel, + StackName: display, Method: "rsync", DestPath: destLabel, Duration: dur.Round(time.Second).String(), Error: err.Error(), }) } else { notifier.NotifyCrossDriveCompleted(notify.CrossDriveDetails{ - StackName: stackName, Method: "rsync", DestPath: destLabel, + StackName: display, Method: "rsync", DestPath: destLabel, Duration: dur.Round(time.Second).String(), }) } diff --git a/controller/internal/backup/shares_test.go b/controller/internal/backup/shares_test.go index 9be03f6..85d91c8 100644 --- a/controller/internal/backup/shares_test.go +++ b/controller/internal/backup/shares_test.go @@ -9,6 +9,7 @@ import ( "sort" "strings" "testing" + "time" "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" @@ -453,3 +454,32 @@ func TestSharesDriveKeyIsCollisionFree(t *testing.T) { // runtimeIsPOSIX reports whether file-mode assertions are meaningful on this host (Windows reports // synthesised permissions, so mode checks there are noise rather than signal). func runtimeIsPOSIX() bool { return os.PathSeparator == '/' } + +// REGRESSION (found in live validation, not by a unit test): the first demo run pushed a hub event +// reading „Másodlagos mentés elkészült: _shares" — the reserved key reached Hungarian customer copy. +// The mapping now happens at the SOURCE of the notification rather than at each wiring site, so no +// future notifier wiring can reintroduce the leak. Red-proof: pass SharesPseudoStack to m.tier2Notify +// instead of DisplayStackName(...) and this fails. +func TestSharesTier2NotifierNeverLeaksReservedKey(t *testing.T) { + env := newSharesEnv(t, "hdd_1", "hdd_2") + env.addShare(t, "hdd_1", "dokumentumok", true) + + var notified []string + env.m.tier2Notify = func(stackName, destLabel string, dur time.Duration, err error) { + notified = append(notified, stackName) + } + if err := env.m.RunSharesTier2(); err != nil { + t.Fatal(err) + } + if len(notified) == 0 { + t.Fatal("precondition: the notifier should have fired") + } + for _, n := range notified { + if n == SharesPseudoStack { + t.Errorf("the reserved key reached the notification boundary raw: %q", n) + } + if n != SharesDisplayName { + t.Errorf("notifier got %q, want %q", n, SharesDisplayName) + } + } +} diff --git a/controller/internal/backup/tier2_shares.go b/controller/internal/backup/tier2_shares.go index b9794b5..c342fef 100644 --- a/controller/internal/backup/tier2_shares.go +++ b/controller/internal/backup/tier2_shares.go @@ -206,7 +206,10 @@ func (m *Manager) RunSharesTier2() error { if err := mirror(sh.Path, filepath.Join(destBase, filepath.FromSlash(rel))); err != nil { m.recordTier2Failure(SharesPseudoStack, target, err) if m.tier2Notify != nil { - m.tier2Notify(SharesPseudoStack, target.Label, time.Since(start), err) + // DISPLAY BOUNDARY: the notifier feeds Hungarian customer/operator copy, so the + // reserved key is mapped HERE rather than at each wiring site — a live demo run + // pushed a literal "_shares" event before this was moved to the source. + m.tier2Notify(DisplayStackName(SharesPseudoStack), target.Label, time.Since(start), err) } return fmt.Errorf("tier2 shares mirror %s: %w", sh.Name, err) } @@ -249,7 +252,7 @@ func (m *Manager) RunSharesTier2() error { } m.recordTier2Success(SharesPseudoStack, lastTarget, totalSize, strings.Join(warns, " "), dur) if m.tier2Notify != nil { - m.tier2Notify(SharesPseudoStack, lastTarget.Label, dur, nil) + m.tier2Notify(DisplayStackName(SharesPseudoStack), lastTarget.Label, dur, nil) } m.logger.Printf("[INFO] [shares] tier-2 run complete: %d share leg(s), %s, %s", mirrored, humanizeBytes(totalSize), dur.Round(time.Second))