From f988666ba071087b972c95f86517094a234e0a25 Mon Sep 17 00:00:00 2001 From: m Date: Tue, 28 Apr 2026 13:02:58 +0200 Subject: [PATCH] fix(t-paliad-064): embed tzdata + stop silent UTC fallback (PR-1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of m's 11:16 reminder emails: the alpine runtime image installs only ca-certificates and ships no /usr/share/zoneinfo, so time.LoadLocation("Europe/Berlin") errored in production. inSlot's silent UTC fallback then matched reminder_morning_time=09:00 against now.UTC().Hour(), firing at 09:00 UTC = 11:00 Berlin (CEST UTC+2) plus the ticker phase. Fix: - import _ "time/tzdata" in cmd/server/main.go (and the services package for test parity) — embeds Go's IANA database in the binary, ~450KB, works without OS tzdata. - inSlot now logs and returns false on bad tz instead of pretending the user lives in UTC. matchesLocalDueDate mirrors the same change. - Tests updated: previous "Mars/Olympus falls back to UTC" expectation flipped to "skips user", new TestTZDataEmbedded asserts LoadLocation("Europe/Berlin") works in the test binary, new TestInSlot_BerlinAt0900_NotAt1100 locks the headline regression (must fire at 07:05 UTC = 09:05 Berlin, must NOT fire at 09:16 UTC = 11:16 Berlin). Validation at the user-save boundary (UserService line 296-300, 619) already rejected unparseable IANA names — that remains; this PR only hardens the read path so any pre-existing corrupt rows skip rather than silently reroute. Build/vet/test/bun-build all green. Self-merging to main. --- cmd/server/main.go | 7 ++++ internal/services/reminder_service.go | 34 ++++++++++++--- internal/services/reminder_service_test.go | 49 +++++++++++++++++++++- 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index cc9cd94..051e219 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -8,6 +8,13 @@ import ( "os/signal" "syscall" + // Embed Go's IANA tz database into the binary so time.LoadLocation works + // without OS tzdata. The runtime image (alpine) doesn't ship /usr/share/ + // zoneinfo — without this import, every reminder timezone lookup fails + // silently and the hourly reminder slot fires in UTC instead of the + // user's chosen tz (t-paliad-064 root cause). Adds ~450KB to the binary. + _ "time/tzdata" + "mgit.msbls.de/m/patholo/internal/auth" "mgit.msbls.de/m/patholo/internal/db" "mgit.msbls.de/m/patholo/internal/handlers" diff --git a/internal/services/reminder_service.go b/internal/services/reminder_service.go index 2d1550e..b8ee549 100644 --- a/internal/services/reminder_service.go +++ b/internal/services/reminder_service.go @@ -39,6 +39,13 @@ import ( "log/slog" "time" + // Embed Go's IANA tz database. Mirrors the import in cmd/server/main.go + // so the services test binary also has tz data available — without this, + // `go test ./internal/services` would pass on a dev host (which has OS + // tzdata) but the prod alpine binary would still be broken, and the tz + // regression test in this package would be vacuous. + _ "time/tzdata" + "github.com/google/uuid" "github.com/jmoiron/sqlx" ) @@ -171,13 +178,26 @@ func slotForKind(kind string) string { // inSlot reports whether the user's local hour-of-day matches the configured // hour for the given slot. The minute is ignored — the ticker fires hourly, -// so we only compare hour granularity. A bad timezone or unparseable time -// falls back to UTC and the column default ("09:00:00"/"16:00:00") so a -// corrupt row never silently suppresses every reminder. +// so we only compare hour granularity. +// +// A bad/empty timezone returns false (skip the user this tick) and logs an +// error — historically we silently fell back to UTC, which masked the +// alpine-container tzdata bug (t-paliad-064): in production +// time.LoadLocation("Europe/Berlin") errored because the runtime image +// shipped no /usr/share/zoneinfo, the silent UTC fallback fired, and +// reminder_morning_time=09:00 matched at 09:00 UTC = 11:00 Berlin. The +// `_ "time/tzdata"` import in cmd/server/main.go makes lookups work without +// OS tzdata; this function now refuses to send rather than guessing. +// +// An unparseable HH:MM string still falls back to the column default +// (09:00 / 16:00) — that's a separate, recoverable input and dropping the +// user for it would be punitive. func inSlot(now time.Time, tz, morning, evening, slot string) bool { loc, err := time.LoadLocation(tz) if err != nil { - loc = time.UTC + slog.Error("reminder: cannot load timezone, skipping user this tick", + "tz", tz, "slot", slot, "error", err) + return false } local := now.In(loc) @@ -298,10 +318,14 @@ func (s *ReminderService) sendPerFrist(ctx context.Context, now time.Time, kind // matchesLocalDueDate decides whether a given Deadline qualifies for the // given kind in the user's local timezone. Mirrors the SQL band but at // per-user date precision. +// +// Bad/empty tz returns false (don't send) for the same reason as inSlot — +// we won't pretend the user lives in UTC. In practice inSlot screens first, +// so this branch is defense-in-depth. func matchesLocalDueDate(now time.Time, tz string, dueDate time.Time, kind string) bool { loc, err := time.LoadLocation(tz) if err != nil { - loc = time.UTC + return false } local := now.In(loc) today := time.Date(local.Year(), local.Month(), local.Day(), 0, 0, 0, 0, loc) diff --git a/internal/services/reminder_service_test.go b/internal/services/reminder_service_test.go index 3415d3f..b1dbc0c 100644 --- a/internal/services/reminder_service_test.go +++ b/internal/services/reminder_service_test.go @@ -81,7 +81,14 @@ func TestInSlot(t *testing.T) { {"evening slot matches at 16:05", utc1605Berlin, "Europe/Berlin", "09:00:00", "16:00:00", "evening", true}, {"evening slot doesn't fire in morning", utc0930Berlin, "Europe/Berlin", "09:00:00", "16:00:00", "evening", false}, {"morning custom 11:00", time.Date(2026, 4, 27, 9, 30, 0, 0, time.UTC), "Europe/Berlin", "11:00", "16:00", "morning", true}, - {"unknown tz falls back to UTC default 09:00", time.Date(2026, 4, 27, 9, 0, 0, 0, time.UTC), "Mars/Olympus", "", "", "morning", true}, + + // t-paliad-064: bad / empty tz now skips the user (no silent UTC + // fallback). The previous behaviour masked the alpine-tzdata bug. + {"unknown tz skips user", time.Date(2026, 4, 27, 9, 0, 0, 0, time.UTC), "Mars/Olympus", "09:00", "16:00", "morning", false}, + {"empty tz skips user", time.Date(2026, 4, 27, 7, 0, 0, 0, time.UTC), "", "09:00", "16:00", "morning", false}, + + // Empty HH:MM still falls back to the column default — different + // failure mode (input string), still worth recovering. {"empty morning string falls back to default 09:00", time.Date(2026, 4, 27, 7, 0, 0, 0, time.UTC), "Europe/Berlin", "", "", "morning", true}, } for _, tc := range tests { @@ -385,3 +392,43 @@ func TestSendPerFrist_ScansCleanly(t *testing.T) { t.Fatalf("sendWeekly: %v", err) } } + +// TestTZDataEmbedded locks down the t-paliad-064 tz fix: the production +// alpine container ships without /usr/share/zoneinfo, so any binary that +// expects time.LoadLocation("Europe/Berlin") to succeed at runtime must +// embed Go's tzdata. The `_ "time/tzdata"` import in reminder_service.go +// is what makes this pass; remove it and this test fails. +// +// The test deliberately covers a non-UTC IANA zone — UTC is hard-coded in +// the runtime and would pass even without the embed. +func TestTZDataEmbedded(t *testing.T) { + zones := []string{"Europe/Berlin", "America/New_York", "Asia/Tokyo"} + for _, z := range zones { + if _, err := time.LoadLocation(z); err != nil { + t.Errorf("time.LoadLocation(%q) failed: %v — is `_ \"time/tzdata\"` still imported?", z, err) + } + } +} + +// TestInSlot_BerlinAt0900_NotAt1100 is the headline regression test for the +// 11:16 prod surprise. With reminder_morning_time=09:00 and tz=Europe/Berlin, +// the slot must fire at 09:xx Berlin (07:xx UTC, the user's chosen time) and +// must not fire at 11:xx Berlin (09:xx UTC, the silent-UTC-fallback result). +// +// Pre-fix this test would either pass spuriously on a dev host with OS +// tzdata, or — without the embed — fail because LoadLocation errored and +// the fallback fired at the wrong wall-clock hour. With the fix in place +// it passes deterministically in any environment. +func TestInSlot_BerlinAt0900_NotAt1100(t *testing.T) { + // 09:05 Berlin (CEST UTC+2) == 07:05 UTC. + at0905Berlin := time.Date(2026, 4, 28, 7, 5, 0, 0, time.UTC) + // 11:16 Berlin (CEST UTC+2) == 09:16 UTC. The exact bug signature. + at1116Berlin := time.Date(2026, 4, 28, 9, 16, 0, 0, time.UTC) + + if !inSlot(at0905Berlin, "Europe/Berlin", "09:00:00", "16:00:00", "morning") { + t.Error("inSlot at 09:05 Berlin / morning=09:00 should match — got false") + } + if inSlot(at1116Berlin, "Europe/Berlin", "09:00:00", "16:00:00", "morning") { + t.Error("inSlot at 11:16 Berlin / morning=09:00 must not match — got true (UTC fallback bug)") + } +}