From 06cdaa577ac8ab010b116c05efd906e3d2938f11 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Mon, 5 Oct 2026 22:46:47 +0100 Subject: [PATCH] Send built-in webhook template headers on real alerts A Telegram webhook passed Test while every real alert was rejected with "message text is empty" (#2540). The Test path always overlaid the service template's headers, but real deliveries sent the stored header copy that the UI seeded at creation. An edit can leave that copy blank, masked or replaced. Telegram reads arguments from a body only when it is labelled as JSON or form data, and checks text before chat_id, so a JSON body without a JSON Content-Type produces exactly that error. When a built-in service renders its own body (no custom template), the template's static headers describe that body. Apply them on every real firing and recovery through prepareWebhookDeliveryContext, replace stored values under any spelling of the name, and route the Test path through the same helper so Test predicts real delivery. Custom templates and generic webhooks keep their stored headers. --- .../v6/internal/subsystems/notifications.md | 18 ++ internal/notifications/notifications.go | 49 +++++ internal/notifications/webhook_enhanced.go | 9 +- .../webhook_template_headers_test.go | 207 ++++++++++++++++++ 4 files changed, 275 insertions(+), 8 deletions(-) create mode 100644 internal/notifications/webhook_template_headers_test.go diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 8089a5c57b..b119154243 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -730,6 +730,24 @@ must therefore report `deliveryPaused: true` whenever the notification manager is disabled, and the destinations UI must surface that as a warning instead of a plain success toast. +A webhook test must also send the headers a real delivery sends. When a +built-in service has no custom template, Pulse renders that service's body, +so the service template's static headers are part of the wire format and +override stored copies under any spelling of the same name. +`prepareWebhookDeliveryContext` applies them to every firing delivery and to +every recovery except ntfy's, and `BuildEnhancedWebhookTestConfig` uses the same +`withWebhookTemplateHeaders` rule; other stored headers stay. The stored +headers are only a copy the UI seeded at creation, and an edit can blank or +replace them. Before this rule, a Telegram webhook with a blank stored +Content-Type passed Test while Telegram ignored every real alert's JSON body +and answered "message text is empty" (#2540). A custom template means the +user owns the body, so stored headers stand. Generic webhooks have no +built-in body and keep their stored headers. ntfy recoveries build their own +plain-text request and are outside this rule. +`internal/notifications/webhook_template_headers_test.go` drives a real +queued Telegram alert with damaged stored Content-Type values, and compares +Test, grouped and resolved requests for every built-in service. + ### Occurrence-bound delivery receipts The notification owner records successful firing delivery by exact alert ID, diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index 61277858b0..bb576ea871 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -2690,9 +2690,58 @@ func (n *NotificationManager) prepareWebhookDeliveryContext(webhook WebhookConfi } } + // Without a custom template Pulse renders the service's built-in body, so + // the template's headers describe that body. The stored headers are only a + // copy the UI seeded at creation, and an edit can blank or replace them; + // Telegram then ignores the JSON and answers "message text is empty" + // (#2540). Test sends always applied these headers, so they passed while + // every real alert failed. + if strings.TrimSpace(webhook.Template) == "" { + if tmpl, ok := builtInWebhookServiceTemplate(webhook.Service); ok { + webhook = withWebhookTemplateHeaders(webhook, tmpl.Headers) + } + } + return webhook, data, nil } +// builtInWebhookServiceTemplate returns the template Pulse renders for a +// service without a custom template. Generic webhooks send the fallback +// payload instead, so they have none. +func builtInWebhookServiceTemplate(service string) (WebhookTemplate, bool) { + if service == "" || service == "generic" { + return WebhookTemplate{}, false + } + for _, tmpl := range GetWebhookTemplates() { + if tmpl.Service == service { + return tmpl, true + } + } + return WebhookTemplate{}, false +} + +// withWebhookTemplateHeaders returns a copy of webhook whose headers carry the +// template's static headers, replacing stored values under any spelling of the +// same name. Templated values are skipped, as executeWebhookRequest skips them. +func withWebhookTemplateHeaders(webhook WebhookConfig, templateHeaders map[string]string) WebhookConfig { + webhook = copyWebhookConfig(webhook) + for key, value := range templateHeaders { + if strings.Contains(value, "{{") { + continue + } + if webhook.Headers == nil { + webhook.Headers = make(map[string]string, len(templateHeaders)) + } + for stored := range webhook.Headers { + if strings.EqualFold(stored, key) { + delete(webhook.Headers, stored) + } + } + webhook.Headers[key] = value + } + return webhook +} + func webhookServiceTemplate(webhook WebhookConfig, mode webhookRenderMode) (string, bool) { if webhook.Service == "" || webhook.Service == "generic" { return "", false diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index 8580e39b3e..5c390fd1c6 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -124,14 +124,7 @@ func BuildEnhancedWebhookTestConfig(basicWebhook WebhookConfig, requestedService continue } webhook.PayloadTemplate = tmpl.PayloadTemplate - if webhook.Headers == nil { - webhook.Headers = make(map[string]string) - } - for k, v := range tmpl.Headers { - if !strings.Contains(v, "{{") { - webhook.Headers[k] = v - } - } + webhook.WebhookConfig = withWebhookTemplateHeaders(webhook.WebhookConfig, tmpl.Headers) break } } diff --git a/internal/notifications/webhook_template_headers_test.go b/internal/notifications/webhook_template_headers_test.go new file mode 100644 index 0000000000..18928dc8d7 --- /dev/null +++ b/internal/notifications/webhook_template_headers_test.go @@ -0,0 +1,207 @@ +package notifications + +// Built-in service bodies carry their template's headers on every delivery +// path (#2540). A Telegram webhook whose stored Content-Type had been blanked +// passed the Test button, which always applied the template headers, while +// every real alert went out without a JSON content type. Telegram ignores +// such a body and rejects the request with "message text is empty". + +import ( + "encoding/json" + "io" + "net/http" + "strings" + "sync" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" +) + +type capturedWebhookRequest struct { + headers http.Header + body []byte +} + +func newWebhookCaptureServer(t *testing.T) (string, func() []capturedWebhookRequest) { + t.Helper() + var ( + mu sync.Mutex + requests []capturedWebhookRequest + ) + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + mu.Lock() + requests = append(requests, capturedWebhookRequest{headers: r.Header.Clone(), body: body}) + mu.Unlock() + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + return server.URL, func() []capturedWebhookRequest { + mu.Lock() + defer mu.Unlock() + return append([]capturedWebhookRequest(nil), requests...) + } +} + +// Stored header states an edit can leave behind for the template's +// Content-Type: a cleared value, the API's masked value saved as typed, a +// replacement, and the same name in other spellings. +var damagedContentTypeHeaders = map[string]map[string]string{ + "blank": {"Content-Type": ""}, + "masked": {"Content-Type": "***REDACTED***"}, + "replaced": {"Content-Type": "text/plain"}, + "lowercase": {"content-type": ""}, + "duplicate": {"Content-Type": "application/json", "content-type": "text/plain"}, +} + +func TestRealTelegramAlertWithDamagedContentTypeStillSendsText(t *testing.T) { + for name, headers := range damagedContentTypeHeaders { + t.Run(name, func(t *testing.T) { + url, captured := newWebhookCaptureServer(t) + m := NewNotificationManagerWithDataDir("", t.TempDir()) + t.Cleanup(m.Stop) + if err := m.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + m.SetGroupingConfig(true, 1, true, false) + m.AddWebhook(WebhookConfig{ + ID: "telegram", + Name: "Telegram Bot", + URL: url + "/bot123:token/sendMessage?chat_id=-1001234", + Method: "POST", + Enabled: true, + Service: "telegram", + Headers: headers, + }) + + m.SendAlert(&alerts.Alert{ + ID: "pve-cluster-synology-backups::metric-threshold:usage", + Type: "usage", + Level: alerts.AlertLevelWarning, + ResourceID: "pve-cluster-synology-backups", + ResourceName: "synology-backups", + Node: "pve", + Message: "Storage usage at 91%", + Value: 91.2, + Threshold: 90, + StartTime: time.Now().Add(-time.Minute), + LastSeen: time.Now(), + }) + + deadline := time.Now().Add(15 * time.Second) + for len(captured()) == 0 && time.Now().Before(deadline) { + time.Sleep(20 * time.Millisecond) + } + requests := captured() + if len(requests) != 1 { + t.Fatalf("Telegram received %d requests, want 1", len(requests)) + } + got := requests[0] + if contentType := got.headers.Values("Content-Type"); len(contentType) != 1 || contentType[0] != "application/json" { + t.Fatalf("Content-Type = %q, want [application/json]", contentType) + } + var payload map[string]any + if err := json.Unmarshal(got.body, &payload); err != nil { + t.Fatalf("body is not JSON: %v\n%s", err, got.body) + } + text, _ := payload["text"].(string) + if !strings.Contains(text, "synology-backups") || payload["chat_id"] != "-1001234" { + t.Fatalf("Telegram payload lacks the alert text or chat: %s", got.body) + } + }) + } +} + +// Test and real deliveries must agree on headers for every built-in service, +// so a passing Test predicts real delivery. +func TestBuiltInServiceDeliveriesSendTemplateHeadersLikeTest(t *testing.T) { + for _, tmpl := range GetWebhookTemplates() { + if tmpl.Service == "generic" { + continue + } + for name, headers := range damagedContentTypeHeaders { + t.Run(tmpl.Service+"/"+name, func(t *testing.T) { + url, captured := newWebhookCaptureServer(t) + m := NewNotificationManager("") + t.Cleanup(m.Stop) + if err := m.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + webhook := WebhookConfig{ + Name: "contract-" + tmpl.Service, + URL: url + "/hook?chat_id=1234", + Enabled: true, + Service: tmpl.Service, + Headers: map[string]string{"Authorization": "Bearer stored"}, + } + for key, value := range headers { + webhook.Headers[key] = value + } + if tmpl.Service == "pagerduty" { + webhook.Headers["routing_key"] = "routing-key" + } + batch := contractGroupedAlerts() + + if err := m.SendTestWebhook(webhook); err != nil { + t.Fatalf("test send: %v", err) + } + m.webhookClient = nil // real sends below use the delivery client + if err := m.sendGroupedWebhook(webhook, batch); err != nil { + t.Fatalf("grouped send: %v", err) + } + resolvedSends := 0 + if tmpl.Service != "ntfy" { // ntfy recoveries own their plain-text request + if err := m.sendResolvedWebhook(webhook, batch, time.Now()); err != nil { + t.Fatalf("resolved send: %v", err) + } + resolvedSends = 1 + } + + requests := captured() + if len(requests) != 2+resolvedSends { + t.Fatalf("captured %d requests, want %d", len(requests), 2+resolvedSends) + } + for i, req := range requests { + for key, want := range tmpl.Headers { + if got := req.headers.Values(key); len(got) != 1 || got[0] != want { + t.Errorf("request %d %s = %q, want [%s]", i, key, got, want) + } + } + if got := req.headers.Get("Authorization"); got != "Bearer stored" { + t.Errorf("request %d dropped an unrelated stored header: Authorization = %q", i, got) + } + } + }) + } + } +} + +// A custom template means the user owns the body, so their headers stand. +func TestCustomTemplateDeliveryKeepsStoredContentType(t *testing.T) { + url, captured := newWebhookCaptureServer(t) + m := NewNotificationManager("") + t.Cleanup(m.Stop) + if err := m.UpdateAllowedPrivateCIDRs("127.0.0.1/32"); err != nil { + t.Fatal(err) + } + m.webhookClient = nil + webhook := WebhookConfig{ + Name: "custom", + URL: url + "/hook?chat_id=1234", + Enabled: true, + Service: "telegram", + Template: `{"text": "{{.ResourceName | jsonString}}"}`, + Headers: map[string]string{"Content-Type": "application/vnd.custom+json"}, + } + if err := m.sendGroupedWebhook(webhook, contractGroupedAlerts()); err != nil { + t.Fatalf("grouped send: %v", err) + } + requests := captured() + if len(requests) != 1 { + t.Fatalf("captured %d requests, want 1", len(requests)) + } + if got := requests[0].headers.Get("Content-Type"); got != "application/vnd.custom+json" { + t.Fatalf("Content-Type = %q, want the stored custom value", got) + } +}