fix: honor action 'ignore' when generating notifications#83
Merged
Conversation
aba2ba5 to
0358db9
Compare
Findings suppressed via *_disabled_rules or a local SAST ignore override are
forced to action 'ignore' and tagged with an actionReason by the normalizer.
OpenGrepScanner.generate_notifications() filtered by severity only, so a
suppressed critical/high finding still posted to the PR comment, Slack, Jira,
and the other notifiers even though the dashboard treats it as ignored.
Skip alerts carrying an actionReason ('disabled_rule' or 'sast_ignore_override')
when building notification groups. Gate on the explicit reason rather than
action == 'ignore', because 'ignore' is also the default action the normalizer
derives for low-severity findings -- those must still notify when a user opts in
to low severities. Suppressed alerts still ship in the uploaded facts; only
notifications are gated.
Adds a regression test for the OpenGrep PR-comment path: a suppressed finding is
excluded while an active finding survives, a fully-suppressed component yields no
notifications, and an opted-in low-severity finding still notifies.
Fixes CE-285
1b75971 to
72a863e
Compare
lelia
approved these changes
Jun 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Findings suppressed via a
*_disabled_rulesconfig (or otherwise resolved toaction: ignore) are excluded from the dashboard and the uploaded facts, but they still appear in generated notifications: the GitHub PR comment, Slack, Jira, and the other notifiers.OpenGrepScanner.generate_notifications()groups alerts for the notifiers and filters by severity only. A finding's severity is independent of its suppression, so a disabledcritical/highrule (for example a SQL-injection rule a team has turned off) still posts to the PR comment even though the dashboard treats it as ignored.Fix
Skip alerts whose
actionisignorewhen building the notification groups. This gates every notifier consistently with how the dashboard handles suppressions. Suppressed alerts are still included in the uploaded facts, so the dashboard continues to show them as ignored; only notifications are affected.Test
tests/test_notification_action_filter.pycovers the OpenGrep PR-comment path:action: ignorecritical finding is excluded while a non-ignored critical finding survives, and the summary counts reflect only the active findingFull suite: 141 passed.
Follow-up
The same severity-only gate exists in the
generate_notificationspaths for the trufflehog, trivy, and socket_tier1 connectors. This PR scopes the change to the OpenGrep (SAST) path where it surfaced; extending the sameaction: ignoreskip to those connectors is a sensible next step.Fixes CE-285