From ae141c62ba155eeef9cf64b8e0f37845d6bc8a83 Mon Sep 17 00:00:00 2001 From: Tyler Koenig Date: Sat, 23 May 2026 13:15:39 -0400 Subject: [PATCH] fix(store): replace panic with error return, handle unmarshal errors generateToken() now returns (string, error) instead of panicking on crypto/rand failure. All json.Unmarshal calls for alert settings now check and propagate errors instead of silently ignoring them. Adds Close() to Store interface for graceful shutdown support. Skips malformed notification entries during Kuma import. --- internal/importer/kuma.go | 4 +++- internal/metrics/prometheus_test.go | 1 + internal/store/sqlstore.go | 34 ++++++++++++++++++++++------- internal/store/store.go | 3 +++ 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/internal/importer/kuma.go b/internal/importer/kuma.go index d2fd2aa..8226d3f 100644 --- a/internal/importer/kuma.go +++ b/internal/importer/kuma.go @@ -96,7 +96,9 @@ func convertKumaNotifications(entries []KumaNotifEntry) map[int]models.AlertConf result := make(map[int]models.AlertConfig) for _, entry := range entries { var cfg KumaNotifConfig - json.Unmarshal([]byte(entry.Config), &cfg) + if err := json.Unmarshal([]byte(entry.Config), &cfg); err != nil { + continue + } alert := models.AlertConfig{ ID: entry.ID, diff --git a/internal/metrics/prometheus_test.go b/internal/metrics/prometheus_test.go index d16723b..847d7cf 100644 --- a/internal/metrics/prometheus_test.go +++ b/internal/metrics/prometheus_test.go @@ -64,6 +64,7 @@ func (m *mockStore) DeleteMaintenanceWindow(int) error { retur func (m *mockStore) IsMonitorInMaintenance(int) (bool, error) { return false, nil } func (m *mockStore) GetPreference(string) (string, error) { return "", nil } func (m *mockStore) SetPreference(string, string) error { return nil } +func (m *mockStore) Close() error { return nil } func TestMetricsHandler(t *testing.T) { ms := &mockStore{ diff --git a/internal/store/sqlstore.go b/internal/store/sqlstore.go index ebb0aa1..63bc096 100644 --- a/internal/store/sqlstore.go +++ b/internal/store/sqlstore.go @@ -29,12 +29,16 @@ func (s *SQLStore) q(query string) string { return rewritePlaceholders(query, s.dollar) } -func generateToken() string { +func generateToken() (string, error) { b := make([]byte, 16) if _, err := rand.Read(b); err != nil { - panic("crypto/rand failed: " + err.Error()) + return "", fmt.Errorf("crypto/rand failed: %w", err) } - return hex.EncodeToString(b) + return hex.EncodeToString(b), nil +} + +func (s *SQLStore) Close() error { + return s.db.Close() } func (s *SQLStore) Init() error { @@ -77,7 +81,11 @@ func (s *SQLStore) GetSites() ([]models.Site, error) { func (s *SQLStore) AddSite(site models.Site) error { token := "" if site.Type == "push" { - token = generateToken() + var err error + token, err = generateToken() + if err != nil { + return fmt.Errorf("generate push token: %w", err) + } } _, err := s.db.Exec(s.q("INSERT INTO sites (name, url, type, token, interval, alert_id, check_ssl, threshold, max_retries, hostname, port, timeout, method, description, parent_id, accepted_codes, dns_resolve_type, dns_server, ignore_tls, paused, regions) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"), site.Name, site.URL, site.Type, token, site.Interval, site.AlertID, site.CheckSSL, site.ExpiryThreshold, site.MaxRetries, @@ -89,7 +97,11 @@ func (s *SQLStore) UpdateSite(site models.Site) error { var existingToken string s.db.QueryRow(s.q("SELECT token FROM sites WHERE id=?"), site.ID).Scan(&existingToken) if site.Type == "push" && existingToken == "" { - existingToken = generateToken() + var err error + existingToken, err = generateToken() + if err != nil { + return fmt.Errorf("generate push token: %w", err) + } } _, err := s.db.Exec(s.q("UPDATE sites SET name=?, url=?, type=?, token=?, interval=?, alert_id=?, check_ssl=?, threshold=?, max_retries=?, hostname=?, port=?, timeout=?, method=?, description=?, parent_id=?, accepted_codes=?, dns_resolve_type=?, dns_server=?, ignore_tls=?, paused=?, regions=? WHERE id=?"), site.Name, site.URL, site.Type, existingToken, site.Interval, site.AlertID, site.CheckSSL, site.ExpiryThreshold, site.MaxRetries, @@ -132,7 +144,9 @@ func (s *SQLStore) GetAlertByName(name string) (models.AlertConfig, error) { if err != nil { return a, err } - json.Unmarshal([]byte(settingsJSON), &a.Settings) + if err := json.Unmarshal([]byte(settingsJSON), &a.Settings); err != nil { + return a, fmt.Errorf("unmarshal alert settings: %w", err) + } return a, nil } @@ -171,7 +185,9 @@ func (s *SQLStore) GetAllAlerts() ([]models.AlertConfig, error) { if err := rows.Scan(&a.ID, &a.Name, &a.Type, &settingsJSON); err != nil { return alerts, err } - json.Unmarshal([]byte(settingsJSON), &a.Settings) + if err := json.Unmarshal([]byte(settingsJSON), &a.Settings); err != nil { + return alerts, fmt.Errorf("unmarshal alert settings for %q: %w", a.Name, err) + } alerts = append(alerts, a) } return alerts, rows.Err() @@ -184,7 +200,9 @@ func (s *SQLStore) GetAlert(id int) (models.AlertConfig, error) { if err != nil { return a, err } - json.Unmarshal([]byte(settingsJSON), &a.Settings) + if err := json.Unmarshal([]byte(settingsJSON), &a.Settings); err != nil { + return a, fmt.Errorf("unmarshal alert settings: %w", err) + } return a, nil } diff --git a/internal/store/store.go b/internal/store/store.go index d83ca72..be562b0 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -64,4 +64,7 @@ type Store interface { // Backup & Restore ExportData() (models.Backup, error) ImportData(data models.Backup) error + + // Lifecycle + Close() error }