From 4d5116644f51b669a003001fb2149284ddef922c Mon Sep 17 00:00:00 2001 From: Tyler Koenig Date: Fri, 15 May 2026 00:00:02 -0400 Subject: [PATCH] fix(core): correctness and robustness fixes across all subsystems - Move status page template to package-level template.Must (panic on parse error at init instead of nil deref at runtime) - Fix XSS in import error responses (log detail server-side, return generic message to client) - Handle ListenAndServe errors in HTTP and SSH servers - Use defer resp.Body.Close() in all alert providers, check json.Marshal errors - Share HTTP clients across checks instead of creating per-request - Use http.NewRequestWithContext for per-site timeout control - Support HTTP method field (was always GET despite DB storing method) - Implement AcceptedCodes validation (was hardcoded >= 400 despite DB storing accepted code ranges) - Add defer tx.Rollback() to ImportData for transaction safety --- .gitignore | 3 +- cmd/goupkeep/main.go | 6 +- internal/alert/alert.go | 23 ++- internal/monitor/monitor.go | 63 +++++++-- internal/server/server.go | 273 ++++++++++++++++++------------------ internal/store/postgres.go | 1 + internal/store/sqlite.go | 2 +- 7 files changed, 218 insertions(+), 153 deletions(-) diff --git a/.gitignore b/.gitignore index e1f6a8e..1fd01d6 100644 --- a/.gitignore +++ b/.gitignore @@ -38,4 +38,5 @@ tmp # Old repo /go-upkeep/ -*.local.json \ No newline at end of file +*.local.json +*.local.md \ No newline at end of file diff --git a/cmd/goupkeep/main.go b/cmd/goupkeep/main.go index 26c01b3..3b7d85d 100644 --- a/cmd/goupkeep/main.go +++ b/cmd/goupkeep/main.go @@ -161,7 +161,11 @@ func startSSHServer(port int) { fmt.Printf("SSH server error: %v\n", err) return } - go func() { s.ListenAndServe() }() + go func() { + if err := s.ListenAndServe(); err != nil { + log.Fatalf("SSH server failed: %v", err) + } + }() } func seedDemoData(s store.Store) { diff --git a/internal/alert/alert.go b/internal/alert/alert.go index 71b7570..16af56c 100644 --- a/internal/alert/alert.go +++ b/internal/alert/alert.go @@ -61,12 +61,15 @@ type DiscordProvider struct{ URL string } func (d *DiscordProvider) Send(title, message string) error { payload := map[string]string{"content": fmt.Sprintf("**%s**\n%s", title, message)} - jsonValue, _ := json.Marshal(payload) + jsonValue, err := json.Marshal(payload) + if err != nil { + return err + } resp, err := alertClient.Post(d.URL, "application/json", bytes.NewBuffer(jsonValue)) if err != nil { return err } - resp.Body.Close() + defer resp.Body.Close() return nil } @@ -75,12 +78,15 @@ type SlackProvider struct{ URL string } func (s *SlackProvider) Send(title, message string) error { payload := map[string]string{"text": fmt.Sprintf("*%s*\n%s", title, message)} - jsonValue, _ := json.Marshal(payload) + jsonValue, err := json.Marshal(payload) + if err != nil { + return err + } resp, err := alertClient.Post(s.URL, "application/json", bytes.NewBuffer(jsonValue)) if err != nil { return err } - resp.Body.Close() + defer resp.Body.Close() return nil } @@ -93,12 +99,15 @@ func (w *WebhookProvider) Send(title, message string) error { "message": message, "status": "alert", } - jsonValue, _ := json.Marshal(payload) + jsonValue, err := json.Marshal(payload) + if err != nil { + return err + } resp, err := alertClient.Post(w.URL, "application/json", bytes.NewBuffer(jsonValue)) if err != nil { return err } - resp.Body.Close() + defer resp.Body.Close() return nil } @@ -139,6 +148,6 @@ func (n *NtfyProvider) Send(title, message string) error { if err != nil { return err } - resp.Body.Close() + defer resp.Body.Close() return nil } diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go index 1bb02fe..3f2a869 100644 --- a/internal/monitor/monitor.go +++ b/internal/monitor/monitor.go @@ -1,6 +1,7 @@ package monitor import ( + "context" "crypto/tls" "fmt" "go-upkeep/internal/alert" @@ -9,6 +10,7 @@ import ( "net" "net/http" "strconv" + "strings" "sync" "time" @@ -52,6 +54,13 @@ var ( activeMutex sync.RWMutex insecureSkipVerify bool + + strictClient = &http.Client{ + Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: false}}, + } + insecureClient = &http.Client{ + Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}, + } ) func SetInsecureSkipVerify(skip bool) { @@ -258,15 +267,51 @@ func checkPush(site models.Site) { } } -func checkHTTP(site models.Site) { - start := time.Now() - timeout := time.Duration(site.Timeout) * time.Second - if timeout <= 0 { - timeout = 5 * time.Second +func isCodeAccepted(code int, accepted string) bool { + if accepted == "" { + return code >= 200 && code < 300 } - skipTLS := insecureSkipVerify || site.IgnoreTLS - client := &http.Client{Timeout: timeout, Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: skipTLS}}} - resp, err := client.Get(site.URL) + for _, part := range strings.Split(accepted, ",") { + part = strings.TrimSpace(part) + if strings.Contains(part, "-") { + bounds := strings.SplitN(part, "-", 2) + lo, err1 := strconv.Atoi(strings.TrimSpace(bounds[0])) + hi, err2 := strconv.Atoi(strings.TrimSpace(bounds[1])) + if err1 == nil && err2 == nil && code >= lo && code <= hi { + return true + } + } else { + if v, err := strconv.Atoi(part); err == nil && code == v { + return true + } + } + } + return false +} + +func checkHTTP(site models.Site) { + method := site.Method + if method == "" { + method = "GET" + } + + timeout := siteTimeout(site) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, method, site.URL, nil) + if err != nil { + handleStatusChange(site, "DOWN", 0, 0) + return + } + + client := strictClient + if insecureSkipVerify || site.IgnoreTLS { + client = insecureClient + } + + start := time.Now() + resp, err := client.Do(req) latency := time.Since(start) rawStatus := "UP" @@ -279,7 +324,7 @@ func checkHTTP(site models.Site) { } else { defer resp.Body.Close() rawCode = resp.StatusCode - if resp.StatusCode >= 400 { + if !isCodeAccepted(rawCode, site.AcceptedCodes) { rawStatus = "DOWN" } if site.CheckSSL && resp.TLS != nil && len(resp.TLS.PeerCertificates) > 0 { diff --git a/internal/server/server.go b/internal/server/server.go index f814cc3..3cf6228 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -8,10 +8,139 @@ import ( "go-upkeep/internal/monitor" "go-upkeep/internal/store" "html/template" + "log" "net/http" "sort" ) +var statusTpl = template.Must(template.New("status").Parse(` + + + + {{.Title}} + + + + +
+

{{.Title}}

+
+
+
+
Powered by Go-Upkeep
+
+ + +`)) + type ServerConfig struct { Port int EnableStatus bool @@ -76,7 +205,8 @@ func Start(cfg ServerConfig) { return } if err := store.Get().ImportData(data); err != nil { - http.Error(w, "Import Failed: "+err.Error(), 500) + log.Printf("Import failed: %v", err) + http.Error(w, "Import failed", 500) return } w.Write([]byte("Import Successful")) @@ -94,12 +224,14 @@ func Start(cfg ServerConfig) { } var kb importer.KumaBackup if err := json.NewDecoder(r.Body).Decode(&kb); err != nil { - http.Error(w, "Invalid Kuma JSON: "+err.Error(), 400) + log.Printf("Invalid Kuma JSON: %v", err) + http.Error(w, "Invalid Kuma JSON", 400) return } backup := importer.ConvertKuma(&kb) if err := store.Get().ImportData(backup); err != nil { - http.Error(w, "Import Failed: "+err.Error(), 500) + log.Printf("Kuma import failed: %v", err) + http.Error(w, "Import failed", 500) return } w.Write([]byte(fmt.Sprintf("Imported %d monitors, %d alerts from Kuma v%s", len(backup.Sites), len(backup.Alerts), kb.Version))) @@ -119,7 +251,9 @@ func Start(cfg ServerConfig) { go func() { addr := fmt.Sprintf(":%d", cfg.Port) fmt.Printf("HTTP Server listening on %s\n", addr) - http.ListenAndServe(addr, mux) + if err := http.ListenAndServe(addr, mux); err != nil { + log.Fatalf("HTTP server failed: %v", err) + } }() } @@ -143,138 +277,9 @@ func renderStatusPage(w http.ResponseWriter, title string) { return sites[i].Name < sites[j].Name }) - const tpl = ` - - - - {{.Title}} - - - - -
-

{{.Title}}

-
-
-
-
Powered by Go-Upkeep
-
- - - ` - - t, _ := template.New("status").Parse(tpl) data := struct { Title string Sites []models.Site }{Title: title, Sites: sites} - t.Execute(w, data) + statusTpl.Execute(w, data) } diff --git a/internal/store/postgres.go b/internal/store/postgres.go index c5201d0..94c046b 100644 --- a/internal/store/postgres.go +++ b/internal/store/postgres.go @@ -239,6 +239,7 @@ func (p *PostgresStore) ImportData(data models.Backup) error { if err != nil { return err } + defer tx.Rollback() tx.Exec("TRUNCATE TABLE sites RESTART IDENTITY CASCADE") tx.Exec("TRUNCATE TABLE alerts RESTART IDENTITY CASCADE") diff --git a/internal/store/sqlite.go b/internal/store/sqlite.go index e83d96e..1b1d5fd 100644 --- a/internal/store/sqlite.go +++ b/internal/store/sqlite.go @@ -258,8 +258,8 @@ func (s *SQLiteStore) ImportData(data models.Backup) error { if err != nil { return err } + defer tx.Rollback() - // Wipe Existing tx.Exec("DELETE FROM sites") tx.Exec("DELETE FROM sqlite_sequence WHERE name='sites'") tx.Exec("DELETE FROM alerts")