mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Send built-in webhook template headers on real alerts (#2545)
Fixes #2540. A Telegram webhook passed **Test** while every real alert was rejected with `Bad Request: message text is empty`. **Cause.** The two paths built their headers differently: - The Test path always overlaid the service template's headers (`Content-Type: application/json`). - Real deliveries sent the stored header copy that the UI seeded when the webhook was created. An edit can leave that copy blank, masked or replaced. For example, the edit form shows the stored value as `***REDACTED***`, and clearing it saves an empty `Content-Type`. Telegram's HTTP reader parses body arguments only for JSON, form-urlencoded or multipart content types, and `sendMessage` checks `text` before `chat_id`. So a JSON body without a JSON Content-Type produces exactly that error. **Fix.** When a built-in service renders its own body (no custom template), the template's static headers describe that body: - `prepareWebhookDeliveryContext` applies them on every real firing and recovery, replacing stored values under any spelling of the header name. Unrelated stored headers stay. - The Test path uses the same `withWebhookTemplateHeaders` helper, so a passing Test predicts real delivery. - Custom templates and generic webhooks keep their stored headers. ntfy recoveries still build their own plain-text request. **Tests** (`internal/notifications/webhook_template_headers_test.go`): - A real queued Telegram alert, with blank, masked, replaced, lowercase and duplicate stored Content-Type values, now sends JSON with `text`. Before the fix it sent `""`, `***REDACTED***` or `text/plain`. - For every built-in service, the Test, grouped and resolved requests carry identical template headers and keep unrelated stored headers. - A custom template keeps its stored Content-Type. `go test ./internal/notifications` and `./internal/api/alerting` pass.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user