fix(t-paliad-064): embed tzdata + stop silent UTC fallback (PR-1)

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.
This commit is contained in:
m
2026-04-28 13:02:58 +02:00
parent 93fdf10537
commit f988666ba0
3 changed files with 84 additions and 6 deletions

View File

@@ -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"

View File

@@ -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)

View File

@@ -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)")
}
}