diff --git a/.github/workflows/aquasec-scan.yml b/.github/workflows/aquasec-scan.yml index 9abafc8..1f7914a 100644 --- a/.github/workflows/aquasec-scan.yml +++ b/.github/workflows/aquasec-scan.yml @@ -64,6 +64,16 @@ on: type: string default: '' + min-severity: + description: > + Minimum severity level for issue creation. Accepted values: low, medium, high, critical + (case-insensitive). Findings below this threshold are still fetched and still drive + adept-to-close lifecycle for previously created issues, but will not create or reopen + issues. Defaults to 'low', which creates issues for all findings. + required: false + type: string + default: '' + secrets: AQUA_KEY: required: true @@ -116,6 +126,7 @@ jobs: SEVERITY_PRIORITY_MAP: ${{ inputs.severity-priority-map }} PROJECT_NUMBER: ${{ inputs.project-number }} PROJECT_ORG: ${{ inputs.project-org }} + MIN_SEVERITY: ${{ inputs.min-severity }} run: | python3 org-workflows/src/security/main.py \ ${{ inputs.dry-run && '--dry-run' || '' }} \ diff --git a/docs/security/aquasec-night-scan-example.yml b/docs/security/aquasec-night-scan-example.yml index 4b74984..06f317f 100644 --- a/docs/security/aquasec-night-scan-example.yml +++ b/docs/security/aquasec-night-scan-example.yml @@ -43,7 +43,7 @@ jobs: uses: AbsaOSS/organizational-workflows/.github/workflows/aquasec-scan.yml@v1.0.0 # Should be changed to release SHA with: dry-run: ${{ inputs.dry-run || false }} - severity-priority-map: 'Critical=Blocker,High=Urgent,Medium=Normal,Low=Minor' + min-severity: 'high' # Only high and critical severity alerts will be promoted to issues project-number: 42 project-org: 'my-org' secrets: diff --git a/src/core/github/issues.py b/src/core/github/issues.py index f63aa4c..bbc35e4 100644 --- a/src/core/github/issues.py +++ b/src/core/github/issues.py @@ -142,7 +142,7 @@ def gh_issue_get_sub_issue_numbers(repo: str, parent_number: int) -> set[int]: try: numbers = json.loads((res.stdout or "").strip() or "[]") return {int(n) for n in numbers} - except json.JSONDecodeError, ValueError: + except (json.JSONDecodeError, ValueError): # fmt: skip logging.error("Failed to parse sub-issues for parent #%d: %r", parent_number, res.stdout) return set() @@ -187,7 +187,7 @@ def gh_issue_list_by_label(repo: str, label: str) -> dict[int, Issue]: for obj in items or []: try: number = int(obj.get("number")) - except TypeError, ValueError: + except (TypeError, ValueError): # fmt: skip continue raw_labels = obj.get("labels") or [] label_names = [str(lbl.get("name") or lbl) if isinstance(lbl, dict) else str(lbl) for lbl in raw_labels] diff --git a/src/security/README.md b/src/security/README.md index 0f7bed8..177b502 100644 --- a/src/security/README.md +++ b/src/security/README.md @@ -71,13 +71,14 @@ jobs: ### Input Parameters -| Name | Description | Required | Default | -|-------------------------|--------------------------------------------------------------------------------------------------------------------|----------|---------| -| `dry-run` | Simulate issue management without making changes. | No | false | -| `verbose-logging` | Enable verbose logging for the AquaSec scan step. | No | false | -| `severity-priority-map` | Comma-separated severity=priority pairs. Only listed severities get a priority. When not set, priority is skipped. | No | '' | -| `project-number` | GitHub ProjectV2 number (org-level) for priority sync. Required together with `severity-priority-map`. | No | 0 | -| `project-org` | GitHub organisation that owns the ProjectV2 board. | No | '' | +| Name | Description | Required | Default | +|-------------------------|----------------------------------------------------------------------------------------------------------------------|----------|---------| +| `dry-run` | Simulate issue management without making changes. | No | false | +| `verbose-logging` | Enable verbose logging for the AquaSec scan step. | No | false | +| `severity-priority-map` | Comma-separated severity=priority pairs. Only listed severities get a priority. When not set, priority is skipped. | No | '' | +| `project-number` | GitHub ProjectV2 number (org-level) for priority sync. Required together with `severity-priority-map`. | No | 0 | +| `project-org` | GitHub organisation that owns the ProjectV2 board. | No | '' | +| `min-severity` | Minimum severity level for issue creation. Accepted values: `low`, `medium`, `high`, `critical` (case-insensitive). | No | 'low' | ### Secrets @@ -174,16 +175,17 @@ PYTHONPATH=src python3 src/security/main.py --repo --dry-run --verb ### CLI Flags -| Flag | Description | -|---------------------------|------------------------------------------------------------------------------| -| `--repo` | Target repository (owner/repo). | -| `--dry-run` | Simulate without writing issues. All intended actions are logged. | -| `--verbose` | Enable verbose logging. | -| `--issue-label` | Label used to discover existing security issues (default: `scope:security`). | -| `--severity-priority-map` | Severity-to-priority mapping (default: `$SEVERITY_PRIORITY_MAP`). | -| `--project-number` | ProjectV2 number for priority sync (default: `$PROJECT_NUMBER`). | -| `--project-org` | Org that owns the ProjectV2 board (default: `$PROJECT_ORG`). | -| `--teams-webhook-url` | Teams webhook URL (default: `$TEAMS_WEBHOOK_URL`). | +| Flag | Description | +|---------------------------|--------------------------------------------------------------------------------------------| +| `--repo` | Target repository (owner/repo). | +| `--dry-run` | Simulate without writing issues. All intended actions are logged. | +| `--verbose` | Enable verbose logging. | +| `--issue-label` | Label used to discover existing security issues (default: `scope:security`). | +| `--severity-priority-map` | Severity-to-priority mapping (default: `$SEVERITY_PRIORITY_MAP`). | +| `--project-number` | ProjectV2 number for priority sync (default: `$PROJECT_NUMBER`). | +| `--project-org` | Org that owns the ProjectV2 board (default: `$PROJECT_ORG`). | +| `--teams-webhook-url` | Teams webhook URL (default: `$TEAMS_WEBHOOK_URL`). | +| `--min-severity` | Minimum severity for issue creation: `low`, `medium`, `high`, `critical` (default: `low`). | --- @@ -191,6 +193,7 @@ PYTHONPATH=src python3 src/security/main.py --repo --dry-run --verb - **Dry-run mode**: Safe preview of all actions without making changes. - **Verbose logging**: Detailed output for debugging and audit. +- **Severity filtering**: Configurable minimum severity threshold to limit issue creation to findings at or above the chosen level. - **Priority mapping**: Configurable severity-to-priority mapping for ProjectV2 boards. - **Teams notifications**: Real-time alerts for new and reopened findings. - **Parent/child issue structure**: Findings grouped by rule with automatic lifecycle management. diff --git a/src/security/config.py b/src/security/config.py index a8ab753..3b4b69d 100644 --- a/src/security/config.py +++ b/src/security/config.py @@ -22,7 +22,7 @@ import uuid from dataclasses import dataclass, field -from security.constants import LABEL_SCOPE_SECURITY, LOGGING_PREFIX +from security.constants import LABEL_SCOPE_SECURITY, LOGGING_PREFIX, MIN_SEVERITY_DEFAULT, VALID_SEVERITIES logger = logging.getLogger(__name__) @@ -46,6 +46,7 @@ class SecurityConfig: project_number: int | None = None project_org: str = "" teams_webhook_url: str = "" + min_severity: str = MIN_SEVERITY_DEFAULT @classmethod def load(cls, args: argparse.Namespace) -> "SecurityConfig": @@ -59,6 +60,9 @@ def load(cls, args: argparse.Namespace) -> "SecurityConfig": except (ValueError, TypeError): # fmt: skip project_number = None + raw_min_severity = args.min_severity or os.environ.get("MIN_SEVERITY", "") + min_severity = raw_min_severity.lower() if raw_min_severity else MIN_SEVERITY_DEFAULT + return cls( aqua_key=os.environ.get("AQUA_KEY", ""), aqua_secret=os.environ.get("AQUA_SECRET", ""), @@ -72,6 +76,7 @@ def load(cls, args: argparse.Namespace) -> "SecurityConfig": project_number=project_number, project_org=args.project_org or os.environ.get("PROJECT_ORG", ""), teams_webhook_url=args.teams_webhook_url or os.environ.get("TEAMS_WEBHOOK_URL", ""), + min_severity=min_severity, ) def validate(self) -> None: @@ -94,6 +99,9 @@ def validate(self) -> None: if not self.repo or "/" not in self.repo: errors.append("repo: not specified or invalid. Use --repo owner/repo.") + if self.min_severity not in VALID_SEVERITIES: + errors.append(f"Only allowed values for MIN_SEVERITY input: {', '.join(sorted(VALID_SEVERITIES))}.") + if errors: for err in errors: logger.error("%sConfig validation failed: %s", LOGGING_PREFIX, err) diff --git a/src/security/constants.py b/src/security/constants.py index 2fda893..d3d4134 100644 --- a/src/security/constants.py +++ b/src/security/constants.py @@ -50,3 +50,6 @@ # Severity mapping (AquaSec numeric → lowercase string) SEVERITY_MAP: dict[int, str] = {1: "low", 2: "medium", 3: "high", 4: "critical"} + +MIN_SEVERITY_DEFAULT = "low" +VALID_SEVERITIES: frozenset[str] = frozenset({"low", "medium", "high", "critical"}) diff --git a/src/security/issues/sync.py b/src/security/issues/sync.py index cbe2f39..3ffae44 100644 --- a/src/security/issues/sync.py +++ b/src/security/issues/sync.py @@ -65,6 +65,7 @@ IssueIndex, NotifiedIssue, ParentOriginalBodies, + SEVERITY_ORDER, SeverityChange, SyncContext, SyncResult, @@ -814,6 +815,18 @@ def _label_adept_to_close_issues( stats.children_marked_for_closure += 1 +def _meets_min_severity(severity: str, min_severity: str) -> bool: + """Return True if *severity* is at or above *min_severity*. + + When min_severity is 'low' (the default) every finding passes, including + those with 'unknown' severity. For any higher threshold, 'unknown' (rank 0) + is always filtered out because it cannot be confirmed to meet the bar. + """ + if min_severity == "low": + return True + return SEVERITY_ORDER.get(severity.lower(), 0) >= SEVERITY_ORDER[min_severity] + + def sync_alerts_and_issues( alerts: dict[int, Alert], issues: dict[int, Issue], @@ -822,6 +835,7 @@ def sync_alerts_and_issues( severity_priority_map: dict[str, str] | None = None, project_number: int | None = None, project_org: str = "", + min_severity: str = "low", ) -> SyncResult: """Sync open alerts into issues.""" @@ -847,6 +861,11 @@ def sync_alerts_and_issues( ) for alert in alerts.values(): + if not _meets_min_severity(alert.metadata.severity, min_severity): + # Below threshold: skip issue creation, update, and reopen for this alert. + # Existing open issues for this finding are intentionally left frozen — they + # will only be touched by adept-to-close logic if the finding later disappears. + continue ensure_issue(alert, sync) _flush_parent_body_updates(sync.parent_original_bodies, issues, dry_run=dry_run, stats=sync.stats) diff --git a/src/security/main.py b/src/security/main.py index 6bf7806..98801a7 100644 --- a/src/security/main.py +++ b/src/security/main.py @@ -85,12 +85,9 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: default="", help="Teams Incoming Webhook URL. Falls back to $TEAMS_WEBHOOK_URL env var", ) + p.add_argument("--min-severity", default="", help="Minimum severity level for issue creation.") p.add_argument("--dry-run", action="store_true", help="Do not write issues; only print intended actions") - p.add_argument( - "--verbose", - action="store_true", - help="Verbose logs (also enabled by RUNNER_DEBUG=1)", - ) + p.add_argument("--verbose", action="store_true", help="Verbose logs (also enabled by RUNNER_DEBUG=1)") return p.parse_args(argv) diff --git a/src/security/services/issue_syncer.py b/src/security/services/issue_syncer.py index 292c860..95156be 100644 --- a/src/security/services/issue_syncer.py +++ b/src/security/services/issue_syncer.py @@ -22,7 +22,7 @@ from core.priority import parse_severity_priority_map from security.alerts.models import Alert -from security.constants import LOGGING_PREFIX +from security.constants import LOGGING_PREFIX, MIN_SEVERITY_DEFAULT from security.issues.sync import SyncResult, sync_alerts_and_issues from security.config import SecurityConfig @@ -53,6 +53,15 @@ def sync(self, open_alerts: dict[int, Alert], *, dry_run: bool) -> SyncResult: spm = parse_severity_priority_map(config.severity_priority_map) + if config.min_severity != MIN_SEVERITY_DEFAULT: + logger.info( + "%sStarting promotion of alerts to GitHub issues (severity >= %s)", + LOGGING_PREFIX, + config.min_severity, + ) + else: + logger.info("%sStarting promotion of alerts to GitHub issues", LOGGING_PREFIX) + result = sync_alerts_and_issues( open_alerts, issues, @@ -60,6 +69,7 @@ def sync(self, open_alerts: dict[int, Alert], *, dry_run: bool) -> SyncResult: severity_priority_map=spm, project_number=config.project_number, project_org=config.project_org, + min_severity=config.min_severity, ) logger.info("%sCompleted promotion of alerts to GitHub issues", LOGGING_PREFIX) diff --git a/tests/security/issues/test_sync.py b/tests/security/issues/test_sync.py index 02b7aa1..3922e75 100644 --- a/tests/security/issues/test_sync.py +++ b/tests/security/issues/test_sync.py @@ -36,6 +36,7 @@ _label_adept_to_close_issues, _log_sync_summary, _maybe_reopen_child, + _meets_min_severity, _merge_child_secmeta, _rebuild_and_apply_child_body, _remove_adept_to_close_label, @@ -1192,3 +1193,133 @@ def test_log_sync_summary(caplog: pytest.LogCaptureFixture, dry_run: bool, prefi assert any("Parent issues" in m and "created: 2" in m and "title updated: 1" in m for m in messages) assert any("Child issues" in m and "created: 15" in m and "reopened: 1" in m and "title updated: 2" in m and "body updated: 3" in m for m in messages) assert not any("linked" in m for m in messages) + + +# ===================================================================== +# _meets_min_severity +# ===================================================================== + + +@pytest.mark.parametrize("severity,min_severity,expected", [ + # min=low: everything passes, including unknown + ("critical", "low", True), + ("high", "low", True), + ("medium", "low", True), + ("low", "low", True), + ("unknown", "low", True), + # min=medium + ("critical", "medium", True), + ("high", "medium", True), + ("medium", "medium", True), + ("low", "medium", False), + ("unknown", "medium", False), + # min=high + ("critical", "high", True), + ("high", "high", True), + ("medium", "high", False), + ("low", "high", False), + ("unknown", "high", False), + # min=critical + ("critical", "critical", True), + ("high", "critical", False), + ("medium", "critical", False), + ("low", "critical", False), + ("unknown", "critical", False), +]) +def test_meets_min_severity(severity: str, min_severity: str, expected: bool) -> None: + assert _meets_min_severity(severity, min_severity) is expected + + +def test_meets_min_severity_case_insensitive() -> None: + assert _meets_min_severity("HIGH", "medium") is True + assert _meets_min_severity("LOW", "medium") is False + + +# ===================================================================== +# sync_alerts_and_issues – min_severity filtering +# ===================================================================== + + +def test_sync_skips_creation_for_below_threshold_alerts( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """Alerts below min_severity do not create issues.""" + sast_alert.metadata.severity = "low" + mock_ensure = mocker.patch("security.issues.sync.ensure_issue") + + sync_alerts_and_issues({1: sast_alert}, {}, dry_run=True, min_severity="high") + + mock_ensure.assert_not_called() + + +def test_sync_creates_issue_at_exact_threshold( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """An alert exactly at min_severity is processed.""" + sast_alert.metadata.severity = "high" + mock_ensure = mocker.patch("security.issues.sync.ensure_issue") + + sync_alerts_and_issues({1: sast_alert}, {}, dry_run=True, min_severity="high") + + mock_ensure.assert_called_once() + + +def test_sync_creates_issue_above_threshold( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """An alert above min_severity is processed.""" + sast_alert.metadata.severity = "critical" + mock_ensure = mocker.patch("security.issues.sync.ensure_issue") + + sync_alerts_and_issues({1: sast_alert}, {}, dry_run=True, min_severity="high") + + mock_ensure.assert_called_once() + + +def test_sync_adept_to_close_still_runs_for_below_threshold_alerts( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """Adept-to-close logic uses all alerts regardless of min_severity threshold. + + An existing open issue whose alert is below the threshold must NOT be + incorrectly marked adept-to-close just because it was filtered from creation. + """ + sast_alert.metadata.severity = "low" + fp = sast_alert.alert_details.alert_hash + + existing_child = _issue_with_secmeta( + 99, + {"type": "child", "fingerprint": fp, "repo": "test-org/test-repo", "rule_id": "r1", "severity": "low"}, + ) + mock_label = mocker.patch("security.issues.sync.gh_issue_add_labels") + + sync_alerts_and_issues( + {1: sast_alert}, {99: existing_child}, dry_run=False, min_severity="high" + ) + + # The alert is still active, so adept-to-close must NOT have been applied. + mock_label.assert_not_called() + + +def test_sync_unknown_severity_passes_at_min_low( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """Unknown severity is created when min_severity=low (the default).""" + sast_alert.metadata.severity = "unknown" + mock_ensure = mocker.patch("security.issues.sync.ensure_issue") + + sync_alerts_and_issues({1: sast_alert}, {}, dry_run=True, min_severity="low") + + mock_ensure.assert_called_once() + + +def test_sync_unknown_severity_filtered_above_min_low( + mocker: MockerFixture, sast_alert: Alert +) -> None: + """Unknown severity is skipped when min_severity is above low.""" + sast_alert.metadata.severity = "unknown" + mock_ensure = mocker.patch("security.issues.sync.ensure_issue") + + sync_alerts_and_issues({1: sast_alert}, {}, dry_run=True, min_severity="medium") + + mock_ensure.assert_not_called() diff --git a/tests/security/test_config.py b/tests/security/test_config.py index a53bec4..461274e 100644 --- a/tests/security/test_config.py +++ b/tests/security/test_config.py @@ -34,6 +34,7 @@ def _make_args(**kwargs) -> argparse.Namespace: "project_number": "", "project_org": "", "teams_webhook_url": "", + "min_severity": "", } defaults.update(kwargs) return argparse.Namespace(**defaults) @@ -154,3 +155,42 @@ def test_load_handles_invalid_project_number(): config = SecurityConfig.load(_make_args(project_number="not-a-number")) assert config.project_number is None + + +def test_load_min_severity_defaults_to_low(): + config = SecurityConfig.load(_make_args()) + + assert "low" == config.min_severity + + +def test_load_min_severity_normalizes_to_lowercase(): + config = SecurityConfig.load(_make_args(min_severity="HIGH")) + + assert "high" == config.min_severity + + +def test_load_min_severity_reads_from_env(monkeypatch): + monkeypatch.setenv("MIN_SEVERITY", "Critical") + config = SecurityConfig.load(_make_args()) + + assert "critical" == config.min_severity + + +def test_load_min_severity_arg_takes_precedence_over_env(monkeypatch): + monkeypatch.setenv("MIN_SEVERITY", "low") + config = SecurityConfig.load(_make_args(min_severity="high")) + + assert "high" == config.min_severity + + +def test_validate_raises_system_exit_when_min_severity_invalid(): + config = _make_config(min_severity="extreme") + + with pytest.raises(SystemExit): + config.validate() + + +def test_validate_passes_for_all_valid_min_severity_values(): + for level in ("low", "medium", "high", "critical"): + config = _make_config(min_severity=level) + config.validate()