From 802d967afd4dc950a048ceebff2bfb48cc542c0b Mon Sep 17 00:00:00 2001 From: David Larsen Date: Tue, 23 Jun 2026 21:15:07 -0400 Subject: [PATCH] fix: honor action 'ignore' when generating notifications Suppressed findings are normalized to action 'ignore' and the dashboard respects that, but generate_notifications() filtered alerts by severity only. Critical/high findings suppressed via *_disabled_rules still posted to the PR comment and other notifiers, since their severity is immutable. Skip alerts whose action is 'ignore' when building notification groups, so suppressions are honored consistently across every notifier. Suppressed alerts still ship in the uploaded facts; only notifications are gated. Adds a regression test for the OpenGrep PR-comment path. --- .../core/connector/opengrep/__init__.py | 9 ++ tests/test_notification_action_filter.py | 97 +++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 tests/test_notification_action_filter.py diff --git a/socket_basics/core/connector/opengrep/__init__.py b/socket_basics/core/connector/opengrep/__init__.py index 0cf4135..bb5f22e 100644 --- a/socket_basics/core/connector/opengrep/__init__.py +++ b/socket_basics/core/connector/opengrep/__init__.py @@ -720,6 +720,15 @@ def generate_notifications(self, components: List[Dict[str, Any]]) -> Dict[str, groups: Dict[str, List[Dict[str, Any]]] = {} for c in comps_map.values(): for a in c.get('alerts', []): + # Skip suppressed alerts. A rule disabled via *_disabled_rules + # (or otherwise suppressed) is normalized to action 'ignore'. + # Honor that here so ignored findings are excluded from every + # notifier (PR comment, Slack, Jira, ...), matching how the + # dashboard treats them. Suppressed alerts still ship in the + # uploaded facts; this only gates notifications. + if (a.get('action') or '').strip().lower() == 'ignore': + continue + # Filter by severity - only include alerts that match allowed severities alert_severity = (a.get('severity') or '').strip().lower() if alert_severity and hasattr(self, 'allowed_severities') and alert_severity not in self.allowed_severities: diff --git a/tests/test_notification_action_filter.py b/tests/test_notification_action_filter.py new file mode 100644 index 0000000..bd4a427 --- /dev/null +++ b/tests/test_notification_action_filter.py @@ -0,0 +1,97 @@ +"""Suppressed findings (action == 'ignore') must be excluded from notifications. + +Regression test for a rule suppressed via *_disabled_rules (or any dashboard / +triage suppression): the alert is normalized to action 'ignore'. The dashboard +honors that, but notification generation previously keyed off severity alone, so +suppressed critical/high findings still posted to the PR comment, Slack, etc. + +generate_notifications() now drops ignored alerts before building any notifier +output. Suppressed alerts still ship in the uploaded facts; only notifications +are gated. +""" + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +from scripts.preview_pr_comments import make_mock_config + +from socket_basics.core.connector.opengrep import OpenGrepScanner + + +def _make_scanner(config): + # generate_notifications() only needs .config and .allowed_severities; + # bypass __init__, which shells out to build the opengrep rule set. + scanner = OpenGrepScanner.__new__(OpenGrepScanner) + scanner.config = config + scanner.allowed_severities = {"critical", "high"} + return scanner + + +def _critical_alert(rule_id, action, snippet, line): + return { + "title": rule_id, + "severity": "critical", + "action": action, + "subType": "sast-generic", + "location": {"path": "slik/domain/GetActionablePlanStatusesUseCase.kt"}, + "props": { + "ruleId": rule_id, + "filePath": "slik/domain/GetActionablePlanStatusesUseCase.kt", + "startLine": line, + "endLine": line, + "codeSnippet": snippet, + }, + } + + +@pytest.fixture +def config(): + return make_mock_config() + + +def test_ignored_alert_excluded_from_pr_notifications(config): + scanner = _make_scanner(config) + components = [ + { + "id": "GetActionablePlanStatusesUseCase.kt", + "name": "GetActionablePlanStatusesUseCase.kt", + "type": "generic", + "alerts": [ + _critical_alert("kotlin-sql-injection", "ignore", "SUPPRESSED_SNIPPET_XYZ", 66), + _critical_alert("kotlin-weak-hash", "error", "ACTIVE_SNIPPET_ABC", 70), + ], + } + ] + + result = scanner.generate_notifications(components) + content = "\n".join(item["content"] for item in result.get("github_pr", [])) + + # The active finding survives. + assert "kotlin-weak-hash" in content + assert "ACTIVE_SNIPPET_ABC" in content + + # The suppressed finding is gone, even though it is critical. + assert "kotlin-sql-injection" not in content + assert "SUPPRESSED_SNIPPET_XYZ" not in content + + # Summary counts only the non-ignored critical. + assert "Critical: 1" in content + assert "Critical: 2" not in content + + +def test_all_ignored_yields_no_notifications(config): + scanner = _make_scanner(config) + components = [ + { + "id": "f.kt", + "name": "f.kt", + "type": "generic", + "alerts": [_critical_alert("kotlin-sql-injection", "ignore", "X", 66)], + } + ] + + # Every alert suppressed -> no groups -> empty per-notifier mapping. + assert scanner.generate_notifications(components) == {}