diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 0f9553545..99a8df606 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -1204,6 +1204,12 @@ profile and assignment columns, but embedded table framing must route through ## Current State +Notification webhook management changes on shared `internal/api/` handlers are +likewise adjacent only: the webhook `signingSecret` payload field, its masked +list representation, and masked-echo preservation on update are +notifications/API-contract owned and grant no agent install, enrollment, +setup-token, or fleet command authority. + The router projection-builder (`internal/api/router.go`) that wires the operator-state provider into the findings runtime now also populates `NeverAutoRemediate` on the projection. The investigation diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 070395271..6beca591a 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -2024,6 +2024,14 @@ the canonical monitored-system blocked payload. ## Current State +That same notifications boundary also carries the webhook signing-secret +payload contract: webhook management payloads may include an optional +`signingSecret` that enables HMAC-signed deliveries, the list API must mask a +configured secret with the shared redaction placeholder instead of returning +it, and an update that echoes the masked placeholder must preserve the stored +secret rather than overwriting it, matching the header and custom-field +secret-handling rules above. + Proxmox setup bootstrap now includes a non-destructive Audit/Repair path in the generated PVE script and existing-source setup guide. That path audits token presence, token expiry, Pulse-managed token drift, and expected ACLs, then diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index df685e34f..770675c70 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -133,6 +133,17 @@ its tenant block only when an identity is present so single-tenant payloads keep their existing shape. PSA/ticket-bridge receivers must get tenant routing identity from this payload boundary, not by inferring it from webhook endpoint configuration. +That same transport boundary also owns outbound delivery integrity. A webhook +config may carry an optional signing secret; when present, every JSON delivery +through the canonical webhook transport must send `X-Pulse-Timestamp` and +`X-Pulse-Signature` (`v1=` + hex HMAC-SHA256 over `timestamp + "." + body`), +computed at the single request-construction choke point and set after custom +headers so user-provided header maps cannot shadow them. Alert deliveries also +send `X-Pulse-Event-ID` (`alertID:event`) as the idempotency token; it must be +stable across both transport-layer and queue-layer retries of the same alert +occurrence. The management API must mask a configured signing secret on read +and preserve the stored secret when an update echoes the masked placeholder, +the same ownership rule it applies to header and custom-field secrets. That same transport boundary also owns webhook request normalization. Rendered webhook URLs must reject userinfo during validation, and request construction must route through a validated absolute URL object instead of reparsing raw URL diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 30a0ff1c4..6cb77a38e 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -1180,6 +1180,11 @@ recovery scope, or a storage/recovery-owned secret source. ## Current State +Notification webhook management changes on shared `internal/api/` handlers are +likewise adjacent only: the webhook `signingSecret` payload field and its +masking semantics are notifications/API-contract owned and create no storage, +recovery-point, or backup-surface semantics. + Kubernetes pod metadata decoded by `frontend-modern/src/hooks/useUnifiedResources.ts` is shared inventory context for storage/recovery handoffs only; Pod phase, container readiness, owner, image, and restart fields do not become protection diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index e238674bc..804132d56 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -37,6 +37,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" + "github.com/rcourtman/pulse-go-rewrite/internal/notifications" "github.com/rcourtman/pulse-go-rewrite/internal/recovery" "github.com/rcourtman/pulse-go-rewrite/internal/relay" "github.com/rcourtman/pulse-go-rewrite/internal/truenas" @@ -53,6 +54,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/pkg/metrics" "github.com/rcourtman/pulse-go-rewrite/pkg/reporting" "github.com/rs/zerolog" + tmock "github.com/stretchr/testify/mock" ) type resourceContractSnapshot struct { @@ -15369,3 +15371,112 @@ func TestContract_UpdateReadinessIncludesV5AgentMigrationSecurityGuidance(t *tes } } } + +func TestContract_MetadataGetPayloadsUseZeroRecordsInsteadOf404(t *testing.T) { + mtp := config.NewMultiTenantPersistence(t.TempDir()) + guestHandler := NewGuestMetadataHandler(mtp) + dockerHandler := NewDockerMetadataHandler(mtp) + + // An empty store must serialize as an empty JSON object, never null. + rec := httptest.NewRecorder() + guestHandler.HandleGetMetadata(rec, httptest.NewRequest(http.MethodGet, "/api/guests/metadata", nil)) + if body := strings.TrimSpace(rec.Body.String()); body != "{}" { + t.Fatalf("empty guest metadata map must serialize as {}, got %q", body) + } + rec = httptest.NewRecorder() + dockerHandler.HandleGetMetadata(rec, httptest.NewRequest(http.MethodGet, "/api/docker/metadata", nil)) + if body := strings.TrimSpace(rec.Body.String()); body != "{}" { + t.Fatalf("empty docker metadata map must serialize as {}, got %q", body) + } + + // A missing resource must return a zero record carrying the requested ID, + // never a 404 — clients rely on this to render empty metadata forms. + rec = httptest.NewRecorder() + guestHandler.HandleGetMetadata(rec, httptest.NewRequest(http.MethodGet, "/api/guests/metadata/qemu-100", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("missing guest metadata must respond 200, got %d", rec.Code) + } + var guestMeta config.GuestMetadata + if err := json.Unmarshal(rec.Body.Bytes(), &guestMeta); err != nil { + t.Fatalf("decode guest metadata response: %v", err) + } + if guestMeta.ID != "qemu-100" { + t.Fatalf("missing guest metadata must echo the requested ID, got %q", guestMeta.ID) + } + + rec = httptest.NewRecorder() + dockerHandler.HandleGetMetadata(rec, httptest.NewRequest(http.MethodGet, "/api/docker/metadata/host-1", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("missing docker metadata must respond 200, got %d", rec.Code) + } + var dockerMeta config.DockerMetadata + if err := json.Unmarshal(rec.Body.Bytes(), &dockerMeta); err != nil { + t.Fatalf("decode docker metadata response: %v", err) + } + if dockerMeta.ID != "host-1" { + t.Fatalf("missing docker metadata must echo the requested ID, got %q", dockerMeta.ID) + } +} + +// TestContract_WebhookSigningSecretMaskedAndPreserved pins the webhook +// management payload contract for delivery signing secrets: the list API must +// mask a configured secret with the shared redaction placeholder, and an +// update that echoes the placeholder must preserve the stored secret instead +// of overwriting it with the literal placeholder string. +func TestContract_WebhookSigningSecretMaskedAndPreserved(t *testing.T) { + mockMonitor := new(MockNotificationMonitor) + mockManager := new(MockNotificationManager) + mockPersistence := new(MockNotificationConfigPersistence) + mockMonitor.On("GetNotificationManager").Return(mockManager) + mockMonitor.On("GetConfigPersistence").Return(mockPersistence) + h := NewNotificationHandlers(nil, mockMonitor) + + stored := notifications.WebhookConfig{ + ID: "wh-signed", + Name: "Signed Webhook", + URL: "https://psa.example.com/inbound/pulse", + Enabled: true, + Service: "generic", + SigningSecret: "stored-secret", + } + + // List must mask the configured secret. + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + rec := httptest.NewRecorder() + h.GetWebhooks(rec, httptest.NewRequest(http.MethodGet, "/api/notifications/webhooks", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("GetWebhooks responded %d", rec.Code) + } + var listed []map[string]interface{} + if err := json.Unmarshal(rec.Body.Bytes(), &listed); err != nil { + t.Fatalf("decode webhook list: %v", err) + } + if len(listed) != 1 { + t.Fatalf("expected one webhook in list, got %d", len(listed)) + } + if got := listed[0]["signingSecret"]; got != "***REDACTED***" { + t.Fatalf("list signingSecret = %v, want masked placeholder", got) + } + + // Update echoing the masked placeholder must preserve the stored secret. + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + mockManager.On("ValidateWebhookURL", stored.URL).Return(nil).Once() + mockManager.On("UpdateWebhook", "wh-signed", tmock.MatchedBy(func(w notifications.WebhookConfig) bool { + return w.SigningSecret == "stored-secret" + })).Return(nil).Once() + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + mockPersistence.On("SaveWebhooks", tmock.Anything).Return(nil).Once() + + update := stored + update.SigningSecret = "***REDACTED***" + body, err := json.Marshal(update) + if err != nil { + t.Fatalf("marshal update: %v", err) + } + rec = httptest.NewRecorder() + h.UpdateWebhook(rec, httptest.NewRequest(http.MethodPut, "/api/notifications/webhooks/wh-signed", bytes.NewReader(body))) + if rec.Code != http.StatusOK { + t.Fatalf("UpdateWebhook responded %d: %s", rec.Code, rec.Body.String()) + } + mockManager.AssertExpectations(t) +} diff --git a/internal/api/notifications.go b/internal/api/notifications.go index a0b2c9556..edb154db6 100644 --- a/internal/api/notifications.go +++ b/internal/api/notifications.go @@ -323,6 +323,11 @@ func (h *NotificationHandlers) GetWebhooks(w http.ResponseWriter, r *http.Reques whMap["mention"] = webhook.Mention } + // Signal that a signing secret is configured without revealing it + if webhook.SigningSecret != "" { + whMap["signingSecret"] = "***REDACTED***" + } + maskedWebhooks[i] = whMap } @@ -442,6 +447,10 @@ func (h *NotificationHandlers) UpdateWebhook(w http.ResponseWriter, r *http.Requ webhook.CustomFields = existing.CustomFields } } + // Preserve the signing secret if the incoming value is redacted + if webhook.SigningSecret == "***REDACTED***" { + webhook.SigningSecret = existing.SigningSecret + } break } } diff --git a/internal/api/notifications_test.go b/internal/api/notifications_test.go index da1b86935..447fc2894 100644 --- a/internal/api/notifications_test.go +++ b/internal/api/notifications_test.go @@ -980,3 +980,55 @@ func TestNotificationHandlers_SetMonitorConcurrentAccess(t *testing.T) { wg.Wait() assert.NotNil(t, h.getMonitor(context.Background())) } + +// Webhook signing secrets must be masked on list reads and preserved when an +// update echoes the masked placeholder back. +func TestNotificationHandlersWebhookSigningSecretLifecycle(t *testing.T) { + mockMonitor := new(MockNotificationMonitor) + mockManager := new(MockNotificationManager) + mockPersistence := new(MockNotificationConfigPersistence) + mockMonitor.On("GetNotificationManager").Return(mockManager) + mockMonitor.On("GetConfigPersistence").Return(mockPersistence) + h := NewNotificationHandlers(nil, mockMonitor) + + stored := notifications.WebhookConfig{ + ID: "wh-signed", + Name: "Signed Webhook", + URL: "https://psa.example.com/inbound/pulse", + Enabled: true, + Service: "generic", + SigningSecret: "stored-secret", + } + + t.Run("GetWebhooksMasksSigningSecret", func(t *testing.T) { + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + req := httptest.NewRequest("GET", "/api/notifications/webhooks", nil) + w := httptest.NewRecorder() + h.GetWebhooks(w, req) + + assert.Equal(t, 200, w.Code) + var resp []map[string]interface{} + _ = json.Unmarshal(w.Body.Bytes(), &resp) + assert.Equal(t, 1, len(resp)) + assert.Equal(t, "***REDACTED***", resp[0]["signingSecret"]) + }) + + t.Run("UpdateWebhookPreservesRedactedSigningSecret", func(t *testing.T) { + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + mockManager.On("ValidateWebhookURL", stored.URL).Return(nil).Once() + mockManager.On("UpdateWebhook", "wh-signed", mock.MatchedBy(func(w notifications.WebhookConfig) bool { + return w.SigningSecret == "stored-secret" + })).Return(nil).Once() + mockManager.On("GetWebhooks").Return([]notifications.WebhookConfig{stored}).Once() + mockPersistence.On("SaveWebhooks", mock.Anything).Return(nil).Once() + + update := stored + update.SigningSecret = "***REDACTED***" + body, _ := json.Marshal(update) + req := httptest.NewRequest("PUT", "/api/notifications/webhooks/wh-signed", bytes.NewReader(body)) + w := httptest.NewRecorder() + h.UpdateWebhook(w, req) + assert.Equal(t, 200, w.Code) + mockManager.AssertExpectations(t) + }) +} diff --git a/internal/notifications/notifications.go b/internal/notifications/notifications.go index ccebbd75d..3cee584b5 100644 --- a/internal/notifications/notifications.go +++ b/internal/notifications/notifications.go @@ -3,7 +3,10 @@ package notifications import ( "bytes" "context" + "crypto/hmac" + "crypto/sha256" "crypto/tls" + "encoding/hex" "encoding/json" "fmt" "html" @@ -14,6 +17,7 @@ import ( "net/url" "os" "os/exec" + "strconv" "strings" "sync" "text/template" @@ -248,6 +252,7 @@ type webhookHTTPResult struct { type webhookRequestOptions struct { alertType string + eventID string // Idempotency token sent as X-Pulse-Event-ID (stable across retries) timeout time.Duration userAgent string responseLogging bool @@ -551,6 +556,11 @@ type WebhookConfig struct { Template string `json:"template"` // Custom payload template CustomFields map[string]string `json:"customFields,omitempty"` Mention string `json:"mention,omitempty"` // Platform-specific mention (e.g., @everyone, @channel, <@USER_ID>) + // SigningSecret enables HMAC-SHA256 signing of outbound deliveries. + // When set, requests carry X-Pulse-Timestamp and X-Pulse-Signature + // (v1=hex(hmac_sha256(secret, timestamp + "." + body))) so receivers + // can verify origin and integrity. + SigningSecret string `json:"signingSecret,omitempty"` } // AppriseMode identifies how Pulse should deliver notifications through Apprise. @@ -2173,7 +2183,7 @@ func (n *NotificationManager) sendGroupedWebhook(webhook WebhookConfig, alertLis } // Send using same request logic - return n.sendWebhookRequest(webhook, jsonData, "grouped") + return n.sendWebhookRequest(webhook, jsonData, "grouped", webhookEventID(data.ID, data.Event)) } func (n *NotificationManager) sendResolvedWebhook(webhook WebhookConfig, alertList []*alerts.Alert, resolvedAt time.Time) error { @@ -2251,7 +2261,7 @@ func (n *NotificationManager) sendResolvedWebhook(webhook WebhookConfig, alertLi return fmt.Errorf("render resolved webhook payload: %w", err) } - return n.sendWebhookRequest(webhook, jsonData, "resolved") + return n.sendWebhookRequest(webhook, jsonData, "resolved", webhookEventID(data.ID, data.Event)) } // sendResolvedWebhookNtfy sends a resolved webhook formatted for ntfy (plain text + headers) @@ -2466,6 +2476,17 @@ func (n *NotificationManager) executeWebhookRequest(webhook WebhookConfig, paylo req.Header.Set(key, value) } + // Delivery integrity headers are set after custom headers so user-provided + // header maps cannot shadow them. + if opts.eventID != "" { + req.Header.Set("X-Pulse-Event-ID", opts.eventID) + } + if secret := strings.TrimSpace(webhook.SigningSecret); secret != "" { + timestamp := strconv.FormatInt(time.Now().Unix(), 10) + req.Header.Set("X-Pulse-Timestamp", timestamp) + req.Header.Set("X-Pulse-Signature", "v1="+signWebhookPayload(secret, timestamp, payload)) + } + client := n.webhookClient if client == nil || (opts.timeout > 0 && opts.timeout != WebhookTimeout) { client = n.createSecureWebhookClient(opts.timeout) @@ -2512,7 +2533,7 @@ func (n *NotificationManager) executeWebhookRequest(webhook WebhookConfig, paylo return result, nil } -func (n *NotificationManager) sendWebhookRequest(webhook WebhookConfig, jsonData []byte, alertType string) error { +func (n *NotificationManager) sendWebhookRequest(webhook WebhookConfig, jsonData []byte, alertType string, eventID string) error { // Re-validate webhook URL to prevent DNS rebinding attacks if err := n.ValidateWebhookURL(webhook.URL); err != nil { log.Error(). @@ -2534,6 +2555,7 @@ func (n *NotificationManager) sendWebhookRequest(webhook WebhookConfig, jsonData result, err := n.executeWebhookRequest(webhook, jsonData, webhookRequestOptions{ alertType: alertType, + eventID: eventID, timeout: WebhookTimeout, userAgent: "Pulse-Monitoring/2.0", responseLogging: false, @@ -2602,6 +2624,30 @@ func isEmptyInterface(value interface{}) bool { } } +// signWebhookPayload computes the hex HMAC-SHA256 of "timestamp.body" with +// the webhook's signing secret. Binding the timestamp into the MAC lets +// receivers reject replayed deliveries. +func signWebhookPayload(secret, timestamp string, payload []byte) string { + mac := hmac.New(sha256.New, []byte(secret)) + mac.Write([]byte(timestamp)) + mac.Write([]byte(".")) + mac.Write(payload) + return hex.EncodeToString(mac.Sum(nil)) +} + +// webhookEventID builds the idempotency token for a delivery: stable for the +// same alert occurrence and event so receivers can deduplicate retries at +// either retry layer (transport or queue). +func webhookEventID(alertID, event string) string { + if alertID == "" { + return "" + } + if event == "" { + event = "alert" + } + return alertID + ":" + event +} + // prepareWebhookData prepares data for template rendering func (n *NotificationManager) prepareWebhookData(alert *alerts.Alert, customFields map[string]interface{}) WebhookPayloadData { duration := time.Since(alert.StartTime) diff --git a/internal/notifications/webhook_enhanced.go b/internal/notifications/webhook_enhanced.go index eca381535..43cff3c54 100644 --- a/internal/notifications/webhook_enhanced.go +++ b/internal/notifications/webhook_enhanced.go @@ -215,12 +215,14 @@ func (n *NotificationManager) SendEnhancedWebhook(webhook EnhancedWebhookConfig, return fmt.Errorf("rate limit exceeded for webhook %s", webhook.Name) } + eventID := webhookEventID(alert.ID, "alert") + // Send with retry logic if webhook.RetryEnabled { - return n.sendWebhookWithRetry(webhook, payload) + return n.sendWebhookWithRetry(webhook, payload, eventID) } - _, err = n.executeEnhancedWebhookRequest(webhook, payload, WebhookTimeout, "Pulse-Monitoring/2.0") + _, err = n.executeEnhancedWebhookRequest(webhook, payload, WebhookTimeout, "Pulse-Monitoring/2.0", eventID) if err != nil { return fmt.Errorf("send webhook %q once: %w", webhook.Name, err) } @@ -303,7 +305,7 @@ func (n *NotificationManager) shouldSendWebhook(webhook EnhancedWebhookConfig, a // - Queue retries: up to MaxAttempts (default 3) with exponential backoff // Total attempts = RetryCount * MaxAttempts (e.g., 3 * 3 = 9 HTTP calls for a single notification) // This ensures delivery even during transient failures at either layer. -func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig, payload []byte) error { +func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig, payload []byte, eventID string) error { maxRetries := webhook.RetryCount if maxRetries <= 0 { maxRetries = WebhookDefaultRetries @@ -364,7 +366,7 @@ func (n *NotificationManager) sendWebhookWithRetry(webhook EnhancedWebhookConfig } } - resp, err := n.executeEnhancedWebhookRequest(webhook, payload, WebhookTimeout, "Pulse-Monitoring/2.0") + resp, err := n.executeEnhancedWebhookRequest(webhook, payload, WebhookTimeout, "Pulse-Monitoring/2.0", eventID) totalAttempts = attempt + 1 lastResp = resp if err == nil { @@ -538,9 +540,10 @@ func webhookErrorStatusCode(err error) (int, bool) { } // executeEnhancedWebhookRequest sends a single enhanced webhook request and returns response metadata. -func (n *NotificationManager) executeEnhancedWebhookRequest(webhook EnhancedWebhookConfig, payload []byte, timeout time.Duration, userAgent string) (*webhookHTTPResult, error) { +func (n *NotificationManager) executeEnhancedWebhookRequest(webhook EnhancedWebhookConfig, payload []byte, timeout time.Duration, userAgent string, eventID string) (*webhookHTTPResult, error) { canonical := canonicalWebhookConfigForEnhanced(webhook) return n.executeWebhookRequest(canonical, payload, webhookRequestOptions{ + eventID: eventID, timeout: timeout, userAgent: userAgent, responseLogging: webhook.ResponseLogging, @@ -618,7 +621,7 @@ func (n *NotificationManager) TestEnhancedWebhook(webhook EnhancedWebhookConfig) webhook.Headers["Tags"] = tags } - result, err := n.executeEnhancedWebhookRequest(webhook, payload, WebhookTestTimeout, "Pulse-Monitoring/2.0 (Test)") + result, err := n.executeEnhancedWebhookRequest(webhook, payload, WebhookTestTimeout, "Pulse-Monitoring/2.0 (Test)", webhookEventID(testAlert.ID, "alert")) if err != nil { if result != nil { return result.statusCode, result.body, err diff --git a/internal/notifications/webhook_enhanced_test.go b/internal/notifications/webhook_enhanced_test.go index 9ff5c4c45..cccfac116 100644 --- a/internal/notifications/webhook_enhanced_test.go +++ b/internal/notifications/webhook_enhanced_test.go @@ -265,7 +265,7 @@ func TestSendWebhookWithRetry_429RetryAfter(t *testing.T) { RetryCount: 2, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) assert.Equal(t, 2, attempts) } @@ -298,7 +298,7 @@ func TestSendWebhookWithRetry_InvalidRetryAfterFallsBackToExponentialBackoff(t * } start := time.Now() - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") elapsed := time.Since(start) assert.NoError(t, err) assert.Equal(t, 2, attempts) @@ -332,7 +332,7 @@ func TestSendWebhookWithRetry_RetriesRequestTimeoutResponses(t *testing.T) { RetryCount: 1, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) assert.Equal(t, 2, attempts) } @@ -361,7 +361,7 @@ func TestSendWebhookWithRetry_RetriesMisdirectedRequestResponses(t *testing.T) { RetryCount: 1, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) assert.Equal(t, 2, attempts) } @@ -390,7 +390,7 @@ func TestSendWebhookWithRetry_RetriesLockedResponses(t *testing.T) { RetryCount: 1, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) assert.Equal(t, 2, attempts) } @@ -419,7 +419,7 @@ func TestSendWebhookWithRetry_RetriesTooEarlyResponses(t *testing.T) { RetryCount: 1, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) assert.Equal(t, 2, attempts) } @@ -453,7 +453,7 @@ func TestSendWebhookWithRetry_StopsOnNonRetryableErrorAfterRetryable(t *testing. RetryCount: 5, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.Error(t, err) assert.Contains(t, err.Error(), "HTTP 400") assert.Contains(t, err.Error(), "after 2 attempts") @@ -478,7 +478,7 @@ func TestSendWebhookWithRetry_PreservesSuccessfulStatusCodeInHistory(t *testing. RetryCount: 1, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.NoError(t, err) history := nm.GetWebhookHistory() @@ -508,7 +508,7 @@ func TestSendWebhookWithRetry_PreservesFailedStatusCodeInHistory(t *testing.T) { RetryCount: 0, } - err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`)) + err := nm.sendWebhookWithRetry(webhook, []byte(`{"test":true}`), "test-alert:alert") assert.Error(t, err) assert.Contains(t, err.Error(), "HTTP 400") diff --git a/internal/notifications/webhook_security_test.go b/internal/notifications/webhook_security_test.go index 135e20233..40ba26f74 100644 --- a/internal/notifications/webhook_security_test.go +++ b/internal/notifications/webhook_security_test.go @@ -14,7 +14,7 @@ func TestSendWebhookRequestRevalidatesURL(t *testing.T) { URL: "http://127.0.0.1/webhook", } - err := nm.sendWebhookRequest(webhook, []byte(`{}`), "alert") + err := nm.sendWebhookRequest(webhook, []byte(`{}`), "alert", "test-alert:alert") if err == nil { t.Fatalf("expected validation error for localhost webhook URL") } diff --git a/internal/notifications/webhook_signing_test.go b/internal/notifications/webhook_signing_test.go new file mode 100644 index 000000000..80b87baf9 --- /dev/null +++ b/internal/notifications/webhook_signing_test.go @@ -0,0 +1,114 @@ +package notifications + +import ( + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "io" + "net/http" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSignWebhookPayloadDeterministic(t *testing.T) { + sig := signWebhookPayload("secret", "1700000000", []byte(`{"a":1}`)) + + mac := hmac.New(sha256.New, []byte("secret")) + mac.Write([]byte("1700000000.{\"a\":1}")) + assert.Equal(t, hex.EncodeToString(mac.Sum(nil)), sig) +} + +func TestWebhookEventID(t *testing.T) { + assert.Equal(t, "alert-1:alert", webhookEventID("alert-1", "alert")) + assert.Equal(t, "alert-1:resolved", webhookEventID("alert-1", "resolved")) + assert.Equal(t, "alert-1:alert", webhookEventID("alert-1", "")) + assert.Equal(t, "", webhookEventID("", "alert")) +} + +func TestWebhookDeliveryCarriesSignatureAndEventID(t *testing.T) { + nm := NewNotificationManager("http://pulse.local") + _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1") + + var gotSignature, gotTimestamp, gotEventID string + var gotBody []byte + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotSignature = r.Header.Get("X-Pulse-Signature") + gotTimestamp = r.Header.Get("X-Pulse-Timestamp") + gotEventID = r.Header.Get("X-Pulse-Event-ID") + gotBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + webhook := WebhookConfig{ + Name: "Signed Webhook", + URL: server.URL, + Enabled: true, + Service: "generic", + SigningSecret: "topsecret", + } + + err := nm.sendWebhookRequest(webhook, []byte(`{"hello":"world"}`), "alert", "alert-42:alert") + require.NoError(t, err) + + assert.Equal(t, "alert-42:alert", gotEventID) + require.NotEmpty(t, gotTimestamp) + require.True(t, strings.HasPrefix(gotSignature, "v1="), "signature %q missing v1= prefix", gotSignature) + expected := signWebhookPayload("topsecret", gotTimestamp, gotBody) + assert.Equal(t, "v1="+expected, gotSignature) +} + +func TestWebhookDeliveryWithoutSecretOmitsSignature(t *testing.T) { + nm := NewNotificationManager("http://pulse.local") + _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1") + + var sawSignature, sawTimestamp bool + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, sawSignature = r.Header["X-Pulse-Signature"] + _, sawTimestamp = r.Header["X-Pulse-Timestamp"] + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + webhook := WebhookConfig{ + Name: "Unsigned Webhook", + URL: server.URL, + Enabled: true, + Service: "generic", + } + + err := nm.sendWebhookRequest(webhook, []byte(`{}`), "alert", "") + require.NoError(t, err) + assert.False(t, sawSignature) + assert.False(t, sawTimestamp) +} + +// Custom header maps must not be able to shadow the integrity headers. +func TestWebhookSigningHeadersWinOverCustomHeaders(t *testing.T) { + nm := NewNotificationManager("http://pulse.local") + _ = nm.UpdateAllowedPrivateCIDRs("127.0.0.1") + + var gotSignature string + server := newIPv4HTTPServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotSignature = r.Header.Get("X-Pulse-Signature") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + webhook := WebhookConfig{ + Name: "Shadow Attempt", + URL: server.URL, + Enabled: true, + Service: "generic", + Headers: map[string]string{"X-Pulse-Signature": "v1=spoofed"}, + SigningSecret: "topsecret", + } + + err := nm.sendWebhookRequest(webhook, []byte(`{}`), "alert", "") + require.NoError(t, err) + assert.NotEqual(t, "v1=spoofed", gotSignature) + assert.True(t, strings.HasPrefix(gotSignature, "v1=")) +}