From f279dc0b07aa8fce03ad4c3072e1c16e00735dd3 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 6 Oct 2026 09:19:18 +0100 Subject: [PATCH] Resolve masked webhook values per key and keep responses masked The webhook editor sends back the ***REDACTED*** values the list returns for every header, custom field and signing secret the user did not retype. Three paths mishandled them: - Update restored the whole saved header or custom-field map whenever any value was masked, silently dropping a header or field added in the same edit. - Update echoed the restored saved custom fields, so a Pushover token and user key came back in the response even though the list masks them, in breach of the api-contracts webhook response clause. Create echoed the request, and could store a masked value verbatim. - The editor's Test button posts the form, so testing an edit after a reload sent the literal mask as the token or header. Resolve each masked value to the saved value of the same key on update and on a form test that carries a saved webhook's id, keep values typed in the same request, and drop masks with no saved value. Build list, create and update responses from one masked shape. Normalization now drops blank headers, so clearing a header removes it instead of sending an empty Content-Type that generic receivers cannot parse. --- .../v6/internal/subsystems/agent-lifecycle.md | 7 + .../v6/internal/subsystems/api-contracts.md | 14 ++ .../v6/internal/subsystems/notifications.md | 10 + .../internal/subsystems/storage-recovery.md | 12 + internal/api/alerting/notifications.go | 231 +++++++++--------- internal/api/alerting/notifications_test.go | 177 +++++++++++++- internal/notifications/notifications.go | 17 ++ .../webhook_template_headers_test.go | 37 +++ 8 files changed, 390 insertions(+), 115 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index a32223caff..fb5f448aab 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8745,3 +8745,10 @@ not claim host re-enrolment or installed-agent acceptance. changed only to carry prompt-cache token buckets through the AI usage export and the Patrol stream response. No agent registration, enrolment, install, update or removal path changed. + +### Webhook masked-value resolution only + +`internal/api/alerting/notifications.go` changed only to resolve masked +webhook header, custom field and signing secret values per key on update and +form test, and to mask create and update responses. No agent registration, +enrolment, install, update or removal path changed. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 905bf26451..e410966406 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -7697,6 +7697,20 @@ routing. Webhook list/create/update responses must return the normalized routing state without exposing masked headers, custom fields, or signing secrets, and the frontend client must retain the old payload shape when older servers omit these optional fields. +All three responses come from `maskedWebhookResponse`, so create and update +mask header values, custom field values and the signing secret exactly as +the list does; they no longer echo the request or the restored saved values. +A `***REDACTED***` value sent back resolves per key: `PUT +/api/notifications/webhooks/{id}` and a `POST /api/notifications/webhooks/test` +whose body carries a saved webhook's `id` take the saved value for that one +header, custom field or signing secret, keep every value typed in the same +request, and drop a masked key that has no saved value. Create drops masked +values. The mask is never stored or sent. Both write routes require +`settings:write`, so the test route reaches no secret an editor cannot +already keep. Header entries with a blank name or value are dropped at +normalization, so a cleared header is removed rather than sent empty. +`TestWebhookMaskedValuesResolvePerKey` in +`internal/api/alerting/notifications_test.go` pins these rules. `GET /api/notifications/health` must fail closed around the persistent queue. Its queue object reports `healthy`, `degraded`, or `unavailable`; includes retained status counts, `attention_required`, fixed `reason_codes`, and the diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index b119154243..637c689ced 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -748,6 +748,16 @@ plain-text request and are outside this rule. queued Telegram alert with damaged stored Content-Type values, and compares Test, grouped and resolved requests for every built-in service. +`NormalizeWebhookConfig` drops header entries with a blank name or value at +every ingress and on load, so a header cleared in the editor is removed +rather than sent empty. An empty Content-Type would otherwise replace the +default `application/json` on generic webhooks, which have no template +headers to restore it. The webhook API resolves the editor's masked values +per key before normalization, so the `***REDACTED***` mask is never stored or +sent (see the api-contracts webhook clause). +`TestGenericWebhookBlankStoredHeaderKeepsDefaultContentType` pins the +delivery side. + ### Occurrence-bound delivery receipts The notification owner records successful firing delivery by exact alert ID, diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 42eb4c900a..ebbc81311a 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6714,3 +6714,15 @@ Pulse already writes, with `omitempty` encoding so existing records load unchanged and older builds ignore the fields. `internal/api/ai_handlers.go` reads them for the usage export only. No new file, directory, retention window, backup, migration or recovery authority is introduced. + +### Webhook configuration never persists the API mask + +`internal/api/alerting/notifications.go` resolves each `***REDACTED***` +value the webhook editor sends back to the saved value of the same key, or +drops it, before the encrypted webhook file is written, so the mask is never +persisted as a header, custom field or signing secret. Webhook normalization +also drops blank header entries when the file is loaded or saved; a stored +blank header is removed on the next save. Export and import pass through the +same normalization. No new file, retention window, backup, migration or +recovery authority is introduced; `TestWebhookMaskedValuesResolvePerKey` in +`internal/api/alerting/notifications_test.go` covers the write path. diff --git a/internal/api/alerting/notifications.go b/internal/api/alerting/notifications.go index 3cde888f05..ff931beffe 100644 --- a/internal/api/alerting/notifications.go +++ b/internal/api/alerting/notifications.go @@ -352,64 +352,113 @@ func (h *NotificationHandlers) UpdateAppriseConfig(w http.ResponseWriter, r *htt func (h *NotificationHandlers) GetWebhooks(w http.ResponseWriter, r *http.Request) { webhooks := h.getMonitor(r.Context()).GetNotificationManager().GetWebhooks() - // Mask sensitive fields in headers and customFields maskedWebhooks := make([]map[string]interface{}, len(webhooks)) for i, webhook := range webhooks { - whMap := map[string]interface{}{ - "id": webhook.ID, - "name": webhook.Name, - "url": webhook.URL, - "method": webhook.Method, - "enabled": webhook.Enabled, - "service": webhook.Service, - } - - // Mask headers - only show keys, not values - if len(webhook.Headers) > 0 { - maskedHeaders := make(map[string]string) - for key := range webhook.Headers { - maskedHeaders[key] = "***REDACTED***" - } - whMap["headers"] = maskedHeaders - } - - // Mask custom fields - only show keys, not values - if len(webhook.CustomFields) > 0 { - maskedFields := make(map[string]string) - for key := range webhook.CustomFields { - maskedFields[key] = "***REDACTED***" - } - whMap["customFields"] = maskedFields - } - - // Include template if present - if webhook.Template != "" { - whMap["template"] = webhook.Template - } - - // Include the configured mention so the UI can render it after reload (#1118) - if webhook.Mention != "" { - whMap["mention"] = webhook.Mention - } - - if len(webhook.TagFilter) > 0 { - whMap["tagFilter"] = append([]string(nil), webhook.TagFilter...) - whMap["tagFilterMode"] = webhook.TagMode - } - whMap["minimumSeverity"] = webhook.MinimumSeverity - - // Signal that a signing secret is configured without revealing it - if webhook.SigningSecret != "" { - whMap["signingSecret"] = "***REDACTED***" - } - - maskedWebhooks[i] = whMap + maskedWebhooks[i] = maskedWebhookResponse(webhook) } w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(maskedWebhooks) } +// maskedWebhookSecret stands in for every stored header, custom field and +// signing secret value the webhook API returns. A request that sends it back +// for a saved webhook means "keep the saved value"; see +// restoreMaskedWebhookSecrets. +const maskedWebhookSecret = "***REDACTED***" + +// maskedWebhookResponse is the single response shape for webhook list, create +// and update: header and custom field values and the signing secret are +// masked, so no response returns a stored secret. +func maskedWebhookResponse(webhook notifications.WebhookConfig) map[string]interface{} { + whMap := map[string]interface{}{ + "id": webhook.ID, + "name": webhook.Name, + "url": webhook.URL, + "method": webhook.Method, + "enabled": webhook.Enabled, + "service": webhook.Service, + } + + // Mask headers - only show keys, not values + if len(webhook.Headers) > 0 { + maskedHeaders := make(map[string]string) + for key := range webhook.Headers { + maskedHeaders[key] = maskedWebhookSecret + } + whMap["headers"] = maskedHeaders + } + + // Mask custom fields - only show keys, not values + if len(webhook.CustomFields) > 0 { + maskedFields := make(map[string]string) + for key := range webhook.CustomFields { + maskedFields[key] = maskedWebhookSecret + } + whMap["customFields"] = maskedFields + } + + // Include template if present + if webhook.Template != "" { + whMap["template"] = webhook.Template + } + + // Include the configured mention so the UI can render it after reload (#1118) + if webhook.Mention != "" { + whMap["mention"] = webhook.Mention + } + + if len(webhook.TagFilter) > 0 { + whMap["tagFilter"] = append([]string(nil), webhook.TagFilter...) + whMap["tagFilterMode"] = webhook.TagMode + } + whMap["minimumSeverity"] = webhook.MinimumSeverity + + // Signal that a signing secret is configured without revealing it + if webhook.SigningSecret != "" { + whMap["signingSecret"] = maskedWebhookSecret + } + + return whMap +} + +// restoreMaskedWebhookSecrets replaces each masked value in an incoming +// webhook with the saved value of the same header, custom field or signing +// secret. Restoring per key keeps any value the user typed in the same edit; +// a masked key with no saved value is dropped, so the mask is never stored or +// sent as a real value. +func restoreMaskedWebhookSecrets(incoming, saved notifications.WebhookConfig) notifications.WebhookConfig { + incoming.Headers = restoreMaskedWebhookValues(incoming.Headers, saved.Headers) + incoming.CustomFields = restoreMaskedWebhookValues(incoming.CustomFields, saved.CustomFields) + if incoming.SigningSecret == maskedWebhookSecret { + incoming.SigningSecret = saved.SigningSecret + } + return incoming +} + +func restoreMaskedWebhookValues(incoming, saved map[string]string) map[string]string { + if len(incoming) == 0 { + return incoming + } + restored := make(map[string]string, len(incoming)) + for key, value := range incoming { + if value != maskedWebhookSecret { + restored[key] = value + continue + } + if savedValue, ok := saved[key]; ok { + restored[key] = savedValue + } + } + return restored +} + +// stripMaskedWebhookSecrets drops masked values from a webhook that has no +// saved counterpart, such as a create or an unsaved form test. +func stripMaskedWebhookSecrets(webhook notifications.WebhookConfig) notifications.WebhookConfig { + return restoreMaskedWebhookSecrets(webhook, notifications.WebhookConfig{}) +} + // CreateWebhook creates a new webhook func (h *NotificationHandlers) CreateWebhook(w http.ResponseWriter, r *http.Request) { h.configMu.Lock() @@ -430,7 +479,7 @@ func (h *NotificationHandlers) CreateWebhook(w http.ResponseWriter, r *http.Requ http.Error(w, err.Error(), http.StatusBadRequest) return } - webhook = notifications.NormalizeWebhookConfig(webhook) + webhook = notifications.NormalizeWebhookConfig(stripMaskedWebhookSecrets(webhook)) monitor := h.getMonitor(r.Context()) manager := monitor.GetNotificationManager() @@ -456,19 +505,7 @@ func (h *NotificationHandlers) CreateWebhook(w http.ResponseWriter, r *http.Requ } manager.AddWebhook(webhook) - // Return the full webhook data including any extra fields like 'service' - var responseData map[string]interface{} - if err := json.Unmarshal(bodyBytes, &responseData); err != nil { - log.Warn().Err(err).Msg("Failed to unmarshal webhook response data") - responseData = make(map[string]interface{}) - } - responseData["id"] = webhook.ID - responseData["customFields"] = webhook.CustomFields - responseData["tagFilter"] = webhook.TagFilter - responseData["tagFilterMode"] = webhook.TagMode - responseData["minimumSeverity"] = webhook.MinimumSeverity - - if err := utils.WriteJSONResponse(w, responseData); err != nil { + if err := utils.WriteJSONResponse(w, maskedWebhookResponse(webhook)); err != nil { log.Error().Err(err).Msg("Failed to write webhook creation response") } } @@ -509,13 +546,12 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ MinimumSeverity *string `json:"minimumSeverity"` } _ = json.Unmarshal(bodyBytes, &routingPresence) - webhook = notifications.NormalizeWebhookConfig(webhook) monitor := h.getMonitor(r.Context()) manager := monitor.GetNotificationManager() - // Preserve original headers/customFields if the incoming values are redacted - // This happens when the frontend sends back masked values from GetWebhooks + // The edit form sends back the masked values GetWebhooks returned for + // anything the user did not retype; those keep their saved values. existingWebhooks := manager.GetWebhooks() found := false for _, existing := range existingWebhooks { @@ -530,36 +566,7 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ if routingPresence.MinimumSeverity == nil { webhook.MinimumSeverity = existing.MinimumSeverity } - // Preserve headers if incoming contains redacted values - if len(webhook.Headers) > 0 && len(existing.Headers) > 0 { - hasRedacted := false - for _, v := range webhook.Headers { - if v == "***REDACTED***" { - hasRedacted = true - break - } - } - if hasRedacted { - webhook.Headers = existing.Headers - } - } - // Preserve customFields if incoming contains redacted values - if len(webhook.CustomFields) > 0 && len(existing.CustomFields) > 0 { - hasRedacted := false - for _, v := range webhook.CustomFields { - if v == "***REDACTED***" { - hasRedacted = true - break - } - } - if hasRedacted { - webhook.CustomFields = existing.CustomFields - } - } - // Preserve the signing secret if the incoming value is redacted - if webhook.SigningSecret == "***REDACTED***" { - webhook.SigningSecret = existing.SigningSecret - } + webhook = restoreMaskedWebhookSecrets(webhook, existing) break } } @@ -567,6 +574,7 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ http.Error(w, fmt.Sprintf("webhook not found: %s", webhookID), http.StatusNotFound) return } + webhook = notifications.NormalizeWebhookConfig(webhook) // Validate webhook URL if err := manager.ValidateWebhookURL(webhook.URL); err != nil { @@ -599,19 +607,7 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ return } - // Return the full webhook data including any extra fields like 'service' - var responseData map[string]interface{} - if err := json.Unmarshal(bodyBytes, &responseData); err != nil { - log.Warn().Err(err).Msg("Failed to unmarshal webhook response data") - responseData = make(map[string]interface{}) - } - responseData["id"] = webhookID - responseData["customFields"] = webhook.CustomFields - responseData["tagFilter"] = webhook.TagFilter - responseData["tagFilterMode"] = webhook.TagMode - responseData["minimumSeverity"] = webhook.MinimumSeverity - - if err := utils.WriteJSONResponse(w, responseData); err != nil { + if err := utils.WriteJSONResponse(w, maskedWebhookResponse(webhook)); err != nil { log.Error().Err(err).Str("webhookID", webhookID).Msg("Failed to write webhook update response") } } @@ -908,7 +904,20 @@ func (h *NotificationHandlers) TestWebhook(w http.ResponseWriter, r *http.Reques http.Error(w, err.Error(), http.StatusBadRequest) return } - basicWebhook = notifications.NormalizeWebhookConfig(basicWebhook) + // Testing an edit of a saved webhook sends the form, whose untouched + // secrets are the masked values from GetWebhooks. Test with the saved + // values, as the saved webhook would send; this route requires the same + // settings:write scope as UpdateWebhook, which keeps them the same way. + saved := notifications.WebhookConfig{} + if basicWebhook.ID != "" { + for _, candidate := range h.getMonitor(r.Context()).GetNotificationManager().GetWebhooks() { + if candidate.ID == basicWebhook.ID { + saved = candidate + break + } + } + } + basicWebhook = notifications.NormalizeWebhookConfig(restoreMaskedWebhookSecrets(basicWebhook, saved)) // Try to extract service from body if present var serviceCheck struct { diff --git a/internal/api/alerting/notifications_test.go b/internal/api/alerting/notifications_test.go index 4207845704..d86fadfe46 100644 --- a/internal/api/alerting/notifications_test.go +++ b/internal/api/alerting/notifications_test.go @@ -895,8 +895,11 @@ func TestNotificationHandlers(t *testing.T) { assert.Equal(t, 200, w.Code) assert.NotContains(t, w.Body.String(), "app_token") assert.NotContains(t, w.Body.String(), "user_token") - assert.Contains(t, w.Body.String(), "\"token\":\"legacy-app\"") - assert.Contains(t, w.Body.String(), "\"user\":\"legacy-user\"") + // Responses name the canonical fields but, like the list, mask their values. + assert.Contains(t, w.Body.String(), "\"token\":\"***REDACTED***\"") + assert.Contains(t, w.Body.String(), "\"user\":\"***REDACTED***\"") + assert.NotContains(t, w.Body.String(), "legacy-app") + assert.NotContains(t, w.Body.String(), "legacy-user") }) t.Run("TestWebhook_UsesNotificationsOwnedTemplateSynthesis", func(t *testing.T) { @@ -1054,8 +1057,11 @@ func TestNotificationHandlers(t *testing.T) { assert.Equal(t, 200, w.Code) assert.NotContains(t, w.Body.String(), "app_token") assert.NotContains(t, w.Body.String(), "user_token") - assert.Contains(t, w.Body.String(), "\"token\":\"legacy-app\"") - assert.Contains(t, w.Body.String(), "\"user\":\"legacy-user\"") + // Responses name the canonical fields but, like the list, mask their values. + assert.Contains(t, w.Body.String(), "\"token\":\"***REDACTED***\"") + assert.Contains(t, w.Body.String(), "\"user\":\"***REDACTED***\"") + assert.NotContains(t, w.Body.String(), "legacy-app") + assert.NotContains(t, w.Body.String(), "legacy-user") }) t.Run("TestNotification_InvalidJSON", func(t *testing.T) { @@ -1384,3 +1390,166 @@ func TestNotificationHandlersWebhookSigningSecretLifecycle(t *testing.T) { mockManager.AssertExpectations(t) }) } + +// The webhook editor sends back the masked values GetWebhooks returned for +// every header, custom field and signing secret the user did not retype. Each +// masked value must resolve to its own saved value: never to the mask itself, +// never by discarding values typed in the same edit, and never back out in a +// response. +func TestWebhookMaskedValuesResolvePerKey(t *testing.T) { + newHandlers := func() (*NotificationHandlers, *MockNotificationManager, *MockNotificationConfigPersistence) { + monitor := new(MockNotificationMonitor) + manager := new(MockNotificationManager) + persistence := new(MockNotificationConfigPersistence) + monitor.On("GetNotificationManager").Return(manager) + monitor.On("GetConfigPersistence").Return(persistence) + return NewNotificationHandlers(nil, monitor), manager, persistence + } + saved := notifications.WebhookConfig{ + ID: "wh-ops", + Name: "Ops", + URL: "https://hooks.example.com/ops", + Enabled: true, + Service: "pushover", + Headers: map[string]string{"Authorization": "Bearer saved-token", "Content-Type": "application/json"}, + CustomFields: map[string]string{"token": "saved-app-token", "user": "saved-user-key"}, + SigningSecret: "saved-signing-secret", + } + savedSecrets := []string{"saved-token", "saved-app-token", "saved-user-key", "saved-signing-secret"} + + t.Run("update keeps values typed in the same edit", func(t *testing.T) { + h, manager, persistence := newHandlers() + var published notifications.WebhookConfig + manager.On("GetWebhooks").Return([]notifications.WebhookConfig{saved}).Once() + manager.On("ValidateWebhookURL", saved.URL).Return(nil).Once() + manager.On("UpdateWebhook", "wh-ops", mock.Anything).Run(func(args mock.Arguments) { + published = args.Get(1).(notifications.WebhookConfig) + }).Return(nil).Once() + persistence.On("SaveWebhooks", mock.Anything).Return(nil).Once() + + body, _ := json.Marshal(map[string]interface{}{ + "name": "Ops", "url": saved.URL, "enabled": true, "service": "pushover", + "headers": map[string]string{ + "Authorization": "***REDACTED***", + "Content-Type": "***REDACTED***", + "X-Team": "platform", + }, + "customFields": map[string]string{"token": "***REDACTED***", "user": "new-user-key"}, + "signingSecret": "***REDACTED***", + }) + rec := httptest.NewRecorder() + h.UpdateWebhook(rec, httptest.NewRequest(http.MethodPut, "/api/notifications/webhooks/wh-ops", bytes.NewReader(body))) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, map[string]string{ + "Authorization": "Bearer saved-token", + "Content-Type": "application/json", + "X-Team": "platform", + }, published.Headers) + assert.Equal(t, map[string]string{"token": "saved-app-token", "user": "new-user-key"}, published.CustomFields) + assert.Equal(t, "saved-signing-secret", published.SigningSecret) + for _, secret := range append(savedSecrets, "new-user-key", "platform") { + assert.NotContains(t, rec.Body.String(), secret, "update response must stay masked") + } + assert.Contains(t, rec.Body.String(), `"X-Team":"***REDACTED***"`) + }) + + t.Run("update never stores the mask or a cleared header", func(t *testing.T) { + h, manager, persistence := newHandlers() + var published notifications.WebhookConfig + manager.On("GetWebhooks").Return([]notifications.WebhookConfig{saved}).Once() + manager.On("ValidateWebhookURL", saved.URL).Return(nil).Once() + manager.On("UpdateWebhook", "wh-ops", mock.Anything).Run(func(args mock.Arguments) { + published = args.Get(1).(notifications.WebhookConfig) + }).Return(nil).Once() + persistence.On("SaveWebhooks", mock.Anything).Return(nil).Once() + + body, _ := json.Marshal(map[string]interface{}{ + "name": "Ops", "url": saved.URL, "enabled": true, "service": "pushover", + // Content-Type cleared in the editor; X-Unknown masked but never saved. + "headers": map[string]string{"Authorization": "***REDACTED***", "Content-Type": "", "X-Unknown": "***REDACTED***"}, + "customFields": map[string]string{"token": "***REDACTED***", "user": "***REDACTED***"}, + }) + rec := httptest.NewRecorder() + h.UpdateWebhook(rec, httptest.NewRequest(http.MethodPut, "/api/notifications/webhooks/wh-ops", bytes.NewReader(body))) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, map[string]string{"Authorization": "Bearer saved-token"}, published.Headers) + assert.Equal(t, "", published.SigningSecret, "an omitted signing secret is removed, as before") + }) + + t.Run("create drops masked values and responds masked", func(t *testing.T) { + h, manager, persistence := newHandlers() + var added notifications.WebhookConfig + manager.On("ValidateWebhookURL", "https://hooks.example.com/new").Return(nil).Once() + manager.On("GetWebhooks").Return([]notifications.WebhookConfig{}).Once() + manager.On("AddWebhook", mock.Anything).Run(func(args mock.Arguments) { + added = args.Get(0).(notifications.WebhookConfig) + }).Return().Once() + persistence.On("SaveWebhooks", mock.Anything).Return(nil).Once() + + body, _ := json.Marshal(map[string]interface{}{ + "name": "New", "url": "https://hooks.example.com/new", "enabled": true, + "headers": map[string]string{"Authorization": "***REDACTED***", "X-Api-Key": "typed-key"}, + "signingSecret": "typed-signing-secret", + }) + rec := httptest.NewRecorder() + h.CreateWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks", bytes.NewReader(body))) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, map[string]string{"X-Api-Key": "typed-key"}, added.Headers) + assert.Equal(t, "typed-signing-secret", added.SigningSecret) + assert.NotContains(t, rec.Body.String(), "typed-key") + assert.NotContains(t, rec.Body.String(), "typed-signing-secret") + assert.Contains(t, rec.Body.String(), `"signingSecret":"***REDACTED***"`) + }) + + t.Run("form test of a saved webhook sends its saved values", func(t *testing.T) { + h, manager, _ := newHandlers() + var tested notifications.EnhancedWebhookConfig + manager.On("GetWebhooks").Return([]notifications.WebhookConfig{saved}).Once() + manager.On("TestEnhancedWebhook", mock.Anything).Run(func(args mock.Arguments) { + tested = args.Get(0).(notifications.EnhancedWebhookConfig) + }).Return(200, "OK", nil).Once() + manager.On("IsEnabled").Return(true).Once() + + // The edit form posts its whole state, including the saved id. + body, _ := json.Marshal(map[string]interface{}{ + "id": "wh-ops", "name": "Ops", "url": saved.URL, "enabled": true, "service": "pushover", + "headers": map[string]string{"Authorization": "***REDACTED***", "X-Team": "platform"}, + "customFields": map[string]string{"token": "***REDACTED***", "user": "***REDACTED***"}, + "signingSecret": "***REDACTED***", + }) + rec := httptest.NewRecorder() + h.TestWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks/test", bytes.NewReader(body))) + + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "Bearer saved-token", tested.Headers["Authorization"]) + assert.Equal(t, "platform", tested.Headers["X-Team"]) + assert.Equal(t, "saved-app-token", tested.CustomFields["token"]) + assert.Equal(t, "saved-user-key", tested.CustomFields["user"]) + assert.Equal(t, "saved-signing-secret", tested.SigningSecret) + }) + + t.Run("form test of an unsaved webhook never sends the mask", func(t *testing.T) { + h, manager, _ := newHandlers() + var tested notifications.EnhancedWebhookConfig + manager.On("TestEnhancedWebhook", mock.Anything).Run(func(args mock.Arguments) { + tested = args.Get(0).(notifications.EnhancedWebhookConfig) + }).Return(200, "OK", nil).Once() + manager.On("IsEnabled").Return(true).Once() + + body, _ := json.Marshal(map[string]interface{}{ + "url": "https://hooks.example.com/new", + "headers": map[string]string{"Authorization": "***REDACTED***", "X-Team": "platform"}, + }) + rec := httptest.NewRecorder() + h.TestWebhook(rec, httptest.NewRequest(http.MethodPost, "/api/notifications/webhooks/test", bytes.NewReader(body))) + + assert.Equal(t, http.StatusOK, rec.Code) + _, hasAuthorization := tested.Headers["Authorization"] + assert.False(t, hasAuthorization, "a mask with no saved value must not be sent") + assert.Equal(t, "platform", tested.Headers["X-Team"]) + manager.AssertNotCalled(t, "GetWebhooks") + }) +} diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index bb576ea871..adda188023 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -360,6 +360,23 @@ func NormalizeWebhookConfig(webhook WebhookConfig) WebhookConfig { if strings.EqualFold(strings.TrimSpace(normalized.Service), "pushover") { normalized.CustomFields = normalizePushoverWebhookCustomFields(normalized.CustomFields) } + normalized.Headers = normalizeWebhookHeaders(normalized.Headers) + return normalized +} + +// normalizeWebhookHeaders drops headers with a blank name or value. A cleared +// header field in the editor means "no header"; sending it empty would replace +// the default Content-Type and leave receivers unable to parse the body. +func normalizeWebhookHeaders(headers map[string]string) map[string]string { + if len(headers) == 0 { + return headers + } + normalized := make(map[string]string, len(headers)) + for key, value := range headers { + if strings.TrimSpace(key) != "" && strings.TrimSpace(value) != "" { + normalized[key] = value + } + } return normalized } diff --git a/internal/notifications/webhook_template_headers_test.go b/internal/notifications/webhook_template_headers_test.go index 18928dc8d7..2f7aaae197 100644 --- a/internal/notifications/webhook_template_headers_test.go +++ b/internal/notifications/webhook_template_headers_test.go @@ -205,3 +205,40 @@ func TestCustomTemplateDeliveryKeepsStoredContentType(t *testing.T) { t.Fatalf("Content-Type = %q, want the stored custom value", got) } } + +// A cleared header in the editor means "no header". Generic webhooks have no +// template headers to fall back on, so an empty stored Content-Type must not +// replace the default and leave the receiver unable to parse the JSON body. +func TestGenericWebhookBlankStoredHeaderKeepsDefaultContentType(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 + m.AddWebhook(WebhookConfig{ + ID: "generic", + Name: "generic", + URL: url + "/hook", + Enabled: true, + Headers: map[string]string{"Content-Type": "", " ": "orphan", "X-Team": "platform"}, + }) + webhook := m.GetWebhooks()[0] + if _, ok := webhook.Headers["Content-Type"]; ok || len(webhook.Headers) != 1 { + t.Fatalf("blank headers survived normalization: %v", webhook.Headers) + } + 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.Values("Content-Type"); len(got) != 1 || got[0] != "application/json" { + t.Fatalf("Content-Type = %q, want [application/json]", got) + } + if got := requests[0].headers.Get("X-Team"); got != "platform" { + t.Fatalf("X-Team = %q, want platform", got) + } +}