diff --git a/internal/app/integration/service.go b/internal/app/integration/service.go index 0557f90c..b72c3a72 100644 --- a/internal/app/integration/service.go +++ b/internal/app/integration/service.go @@ -2020,8 +2020,12 @@ type SendNotificationInput struct { Title string Body string Severity string // critical, high, medium, low - URL string - Fields map[string]string + // EventType decides whether Severity is filterable at all. Empty means + // "unknown", and SeverityFilterApplies treats unknown as filterable — the + // pre-existing behavior for every caller that does not set it. + EventType string + URL string + Fields map[string]string } // SendNotificationResult represents the result of sending a notification. @@ -2072,8 +2076,10 @@ func (s *IntegrationService) SendNotification(ctx context.Context, input SendNot notifExt, _ = s.notificationExtRepo.GetByIntegrationID(ctx, intgID) } - // Check if we should notify for this severity - if notifExt != nil && !notifExt.ShouldNotify(input.Severity) { + // Check if we should notify for this severity. Skipped for event types whose + // Severity is not a finding severity — see integrationdom.SeverityFilterApplies. + if notifExt != nil && integrationdom.SeverityFilterApplies(integrationdom.EventType(input.EventType)) && + !notifExt.ShouldNotify(input.Severity) { return &SendNotificationResult{ Success: false, Error: fmt.Sprintf("notifications disabled for severity: %s", input.Severity), @@ -2161,8 +2167,11 @@ func (s *IntegrationService) BroadcastNotification(ctx context.Context, input Br } if iwn.Notification != nil { - // Check if this integration should receive notifications for this severity - if !iwn.Notification.ShouldNotify(input.Severity) { + // Check if this integration should receive notifications for this + // severity. Not applied to event types whose Severity is a constant + // rather than a finding severity — see SeverityFilterApplies. + if integrationdom.SeverityFilterApplies(input.EventType) && + !iwn.Notification.ShouldNotify(input.Severity) { continue } @@ -2178,6 +2187,7 @@ func (s *IntegrationService) BroadcastNotification(ctx context.Context, input Br Title: input.Title, Body: input.Body, Severity: input.Severity, + EventType: string(input.EventType), URL: input.URL, Fields: input.Fields, }) diff --git a/pkg/domain/integration/notification_extension.go b/pkg/domain/integration/notification_extension.go index 0957bb61..880bf8ce 100644 --- a/pkg/domain/integration/notification_extension.go +++ b/pkg/domain/integration/notification_extension.go @@ -253,6 +253,39 @@ func DefaultEnabledEventTypes() []EventType { } } +// SeverityFilterApplies reports whether the per-integration severity filter is +// meaningful for this event type. +// +// EnqueueParams.Severity carries two different things depending on the event: +// +// - For finding-shaped events (new_finding, sla_breach, ...) it IS the +// finding's severity. An operator who leaves the filter at its default is +// saying "only tell me about critical and high findings", and honoring +// that is the whole point of the filter. +// +// - For approval lifecycle events it is a hardcoded constant chosen by the +// enqueue site ("medium" for requested/rejected, "low" for approved). It +// describes nothing about a finding, and no operator ever asked to +// suppress it. +// +// Running the second kind through a filter built for the first kind silently +// defeats it. That is not hypothetical: DefaultEnabledEventTypes deliberately +// includes EventTypeApprovalRequested, with the comment "if it reaches nobody +// the finding stays blocked indefinitely" — and then the severity gate dropped +// it anyway, because "medium" is not in the default critical+high set. The +// event-type gate was opened on purpose and the severity gate closed it again. +// +// Events exempted here remain fully controllable through the event-type filter, +// which is the switch that actually means "I do not want these". +func SeverityFilterApplies(eventType EventType) bool { + switch MapLegacyEventType(eventType) { + case EventTypeApprovalRequested, EventTypeApprovalApproved, EventTypeApprovalRejected: + return false + default: + return true + } +} + // AllKnownEventTypes returns all known event types (for backward compatibility API). func AllKnownEventTypes() []EventType { types := make([]EventType, 0, len(AllEventTypes())) diff --git a/pkg/domain/integration/severity_filter_test.go b/pkg/domain/integration/severity_filter_test.go new file mode 100644 index 00000000..5cb56e3c --- /dev/null +++ b/pkg/domain/integration/severity_filter_test.go @@ -0,0 +1,145 @@ +package integration + +import "testing" + +// The severity filter was silently defeating the event-type filter. +// +// DefaultEnabledEventTypes deliberately turns EventTypeApprovalRequested ON, +// with the reasoning recorded next to it: "An approval request is addressed to +// a human; if it reaches nobody the finding stays blocked indefinitely." +// +// The enqueue site stamps Severity: "medium" (vulnerability_service.go), and +// IsSeverityEnabled treats an empty enabled_severities list as critical+high. +// So the event-type gate was opened on purpose and the severity gate closed it +// again — the exact outcome that comment exists to prevent. +// +// These tests pin the two halves together so the same defeat cannot reappear. + +func TestSeverityFilter_DoesNotApplyToApprovalEvents(t *testing.T) { + approvals := []EventType{ + EventTypeApprovalRequested, + EventTypeApprovalApproved, + EventTypeApprovalRejected, + } + + for _, et := range approvals { + t.Run(string(et), func(t *testing.T) { + if SeverityFilterApplies(et) { + t.Fatalf("%s is severity-filtered, but its Severity is a constant "+ + "chosen by the enqueue site, not a finding severity", et) + } + }) + } +} + +// The filter must keep working for the events it was built for — a fix that +// delivers everything is not a fix. +func TestSeverityFilter_StillAppliesToFindingEvents(t *testing.T) { + findingShaped := []EventType{ + EventTypeNewFinding, + EventTypeNewExposure, + EventTypeFindingAssigned, + EventTypeFindingPriorityEscalated, + EventTypeSLABreach, + } + + for _, et := range findingShaped { + t.Run(string(et), func(t *testing.T) { + if !SeverityFilterApplies(et) { + t.Fatalf("%s stopped being severity-filtered: an operator who asked "+ + "for critical+high only would start receiving everything", et) + } + }) + } +} + +// The end-to-end statement of the bug: approval_requested is default-ON at the +// event-type gate, and must survive the severity gate on a default (empty) +// configuration. +func TestApprovalRequested_SurvivesBothGatesOnDefaults(t *testing.T) { + ext := &NotificationExtension{} // empty config = platform defaults + + var defaultOn bool + for _, et := range DefaultEnabledEventTypes() { + if et == EventTypeApprovalRequested { + defaultOn = true + break + } + } + if !defaultOn { + t.Fatal("EventTypeApprovalRequested is no longer default-on; if that was " + + "deliberate, this test should be deleted along with the reasoning " + + "recorded in DefaultEnabledEventTypes") + } + + if !ext.ShouldNotifyEventType(EventTypeApprovalRequested) { + t.Fatal("blocked by the event-type gate") + } + + // This is the half that was broken: "medium" is not in the default + // critical+high set, so the severity gate dropped it. + const enqueuedSeverity = "medium" // vulnerability_service.go RequestApproval + if ext.ShouldNotify(enqueuedSeverity) { + t.Fatal("test is not exercising the bug: the default severity set now " + + "includes medium, so this would pass without the fix") + } + if SeverityFilterApplies(EventTypeApprovalRequested) { + t.Fatal("approval_requested is still severity-filtered, so it is still " + + "dropped on a default configuration and the finding stays blocked") + } +} + +// Completeness gate. A new event type must be a deliberate decision about +// whether its Severity is a real severity, not a default inherited by whoever +// adds the constant. This fails the build on any unclassified addition. +func TestSeverityFilter_EveryEventTypeIsClassified(t *testing.T) { + // Severity is a constant at the enqueue site, so filtering it is meaningless. + notFilterable := map[EventType]bool{ + EventTypeApprovalRequested: true, + EventTypeApprovalApproved: true, + EventTypeApprovalRejected: true, + } + + // Severity describes a finding/exposure, so the operator's filter is real. + filterable := map[EventType]bool{ + EventTypeSecurityAlert: true, + EventTypeSystemError: true, + EventTypeNewAsset: true, + EventTypeAssetChanged: true, + EventTypeAssetDeleted: true, + EventTypeScanStarted: true, + EventTypeScanCompleted: true, + EventTypeScanFailed: true, + EventTypeNewFinding: true, + EventTypeFindingConfirmed: true, + EventTypeFindingTriaged: true, + EventTypeFindingFixed: true, + EventTypeFindingReopened: true, + EventTypeFindingPriorityEscalated: true, + EventTypeFindingAssigned: true, + EventTypeSLABreach: true, + EventTypeWorkflowNotification: true, + EventTypeNewExposure: true, + EventTypeExposureResolved: true, + } + + for _, info := range AllEventTypes() { + et := info.Type + switch { + case notFilterable[et]: + if SeverityFilterApplies(et) { + t.Errorf("%s is listed as not-filterable here but SeverityFilterApplies says otherwise", et) + } + case filterable[et]: + if !SeverityFilterApplies(et) { + t.Errorf("%s is listed as filterable here but SeverityFilterApplies says otherwise", et) + } + default: + t.Errorf("event type %q is not classified. Decide whether its "+ + "EnqueueParams.Severity is a real finding severity (add it to "+ + "`filterable`) or a constant picked by the enqueue site (add it to "+ + "`notFilterable` AND to SeverityFilterApplies). Inheriting the "+ + "default silently is how approval_requested became undeliverable.", et) + } + } +}