Skip to content

Feat: GH Issue generation based on severity#74

Merged
tmikula-dev merged 5 commits into
masterfrom
feature/create-issues-based-on-severity
Jun 15, 2026
Merged

Feat: GH Issue generation based on severity#74
tmikula-dev merged 5 commits into
masterfrom
feature/create-issues-based-on-severity

Conversation

@tmikula-dev

@tmikula-dev tmikula-dev commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Overview

This pull request introduces a configurable minimum severity threshold for security issue creation, allowing users to control which findings result in new or reopened GitHub issues. The changes span workflow configuration, backend logic, and documentation, and include comprehensive tests to ensure correct behavior.

Release Notes

  • Feat: GH Issue generation based on severity

Related

Closes #73

Summary by CodeRabbit

  • New Features
    • Added configurable min-severity (default: low) to filter which security findings are promoted to issues, supporting low, medium, high, and critical.
  • Documentation
    • Updated security workflow and examples to document min-severity and the matching CLI flag.
  • Bug Fixes
    • Improved robustness of JSON parsing error handling when reading issue data.
  • Tests
    • Expanded unit tests to verify severity filtering, defaults/normalization, and validation behavior.

@tmikula-dev tmikula-dev self-assigned this Jun 10, 2026
@tmikula-dev tmikula-dev added the enhancement New feature or request label Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8bb4f38b-82fc-4d25-908e-73fcf0f8edb1

📥 Commits

Reviewing files that changed from the base of the PR and between 3ad7780 and 3ef8770.

📒 Files selected for processing (6)
  • .github/workflows/aquasec-scan.yml
  • docs/security/aquasec-night-scan-example.yml
  • src/security/README.md
  • src/security/config.py
  • src/security/issues/sync.py
  • src/security/services/issue_syncer.py
✅ Files skipped from review due to trivial changes (1)
  • src/security/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/aquasec-scan.yml
  • src/security/services/issue_syncer.py
  • src/security/config.py
  • src/security/issues/sync.py

Walkthrough

This PR implements severity-based filtering for AquaSec security alerts in the organizational workflows. It adds a configurable minimum severity threshold (low, medium, high, or critical), allowing teams to generate GitHub issues only for alerts at or above that level while maintaining existing issue lifecycle management for suppressed alerts.

Changes

Security Alert Severity Filtering

Layer / File(s) Summary
Severity Constants and Config Schema
src/security/constants.py, src/security/config.py
Defines MIN_SEVERITY_DEFAULT = "low" and VALID_SEVERITIES frozenset. Adds min_severity field to SecurityConfig dataclass.
Configuration Loading and Validation
src/security/config.py, tests/security/test_config.py
Loads min_severity from CLI args or MIN_SEVERITY environment variable with lowercase normalization. Validates against allowed severity set, raising exit on invalid values. Test helper and six new test cases cover defaulting, normalization, env reading, CLI precedence, and validation.
Alert Filtering Logic
src/security/issues/sync.py, tests/security/issues/test_sync.py
Introduces _meets_min_severity(severity, min_severity) helper using SEVERITY_ORDER for rank-based comparison. Updates sync_alerts_and_issues signature to accept min_severity parameter. Skips ensure_issue for below-threshold alerts in processing loop while preserving adept-to-close behavior. Parametrized tests verify threshold logic and unknown severity handling.
CLI and Integration Wiring
src/security/main.py, src/security/services/issue_syncer.py, .github/workflows/aquasec-scan.yml
Adds --min-severity CLI flag to argument parser. Passes config.min_severity to sync_alerts_and_issues call in IssueSyncer.sync. Declares optional workflow input min-severity and exports it to MIN_SEVERITY environment variable.
Documentation and User-Facing Updates
src/security/README.md, docs/security/aquasec-night-scan-example.yml
Documents new min-severity input parameter (allowed values and default) in workflow configuration and CLI flags tables. Adds feature description of severity-based issue creation filtering. Updates example configuration to demonstrate usage.
Python 3 Exception Syntax Fix
src/core/github/issues.py
Updates exception handlers in gh_issue_get_sub_issue_numbers and gh_issue_list_by_label to use Python 3 tuple-based syntax for json.JSONDecodeError, ValueError, and TypeError.

Sequence Diagram

sequenceDiagram
  participant Workflow as GitHub Workflow
  participant CLI as security/main.py
  participant Config as SecurityConfig
  participant IssueSyncer as IssueSyncer
  participant SyncLogic as sync_alerts_and_issues
  participant FilterCheck as _meets_min_severity

  Workflow->>CLI: parse args with --min-severity
  CLI->>Config: load(args, env)
  Config->>Config: normalize to lowercase
  Config->>Config: validate against VALID_SEVERITIES
  IssueSyncer->>SyncLogic: sync_alerts_and_issues(alerts, min_severity)
  SyncLogic->>SyncLogic: iterate over alerts
  SyncLogic->>FilterCheck: check alert.metadata.severity
  FilterCheck->>FilterCheck: compare rank via SEVERITY_ORDER
  alt Alert meets threshold
    SyncLogic->>SyncLogic: ensure_issue(alert)
  else Alert below threshold
    SyncLogic->>SyncLogic: skip (continue)
  end
  SyncLogic->>SyncLogic: apply adept-to-close logic (all alerts)
Loading

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • AbsaOSS/organizational-workflows#47: Both PRs modify src/security/issues/sync.py in the security issue synchronization flow—main PR adds min_severity gating around ensure_issue, while the related PR refactors issue body/title data used during that same sync path.

Suggested reviewers

  • miroslavpojer

Poem

🐰 A rabbit hops through severity tiers,
Low and medium, high and critical cheer,
Filter the alerts, keep focus sharp bright,
Only what matters gets issues in sight!
Configuration flows, tests pass with delight,
Issues now sorted—a workflow done right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 65.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title accurately describes the main feature addition: severity-based GitHub issue generation for security findings.
Description check ✅ Passed The pull request description follows the template structure with Overview, Release Notes, and Related sections; all required sections are present and contain appropriate information.
Linked Issues check ✅ Passed All acceptance criteria from issue #73 are met: new min-severity input added, GitHub issues generated only for findings >= threshold, comprehensive tests added, and documentation updated.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the severity-based issue creation feature; no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/create-issues-based-on-severity

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/security/README.md (2)

196-196: 💤 Low value

Consider mentioning "unknown" severity behavior in the feature description.

The implementation (context snippet 3, sync.py) includes special handling for findings with "unknown" severity: they pass when min-severity=low (the default) but are filtered at higher thresholds. Documenting this edge case could help users understand the complete filtering behavior.

📝 Optional addition to clarify edge case
-- **Severity filtering**: Configurable minimum severity threshold to limit issue creation to findings at or above the chosen level.
+- **Severity filtering**: Configurable minimum severity threshold to limit issue creation to findings at or above the chosen level. Findings with unknown severity pass at the default threshold (`low`) but are filtered at higher thresholds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/security/README.md` at line 196, Update the "Severity filtering" line to
explicitly describe how findings with "unknown" severity are treated by the
filter: note that with the default min-severity=low unknown findings are allowed
through, but they are excluded when the configured min-severity is raised above
low; reference the config key "min-severity" and the "unknown" severity behavior
implemented in sync.py so users understand this edge case.

155-174: ⚡ Quick win

Consider documenting the MIN_SEVERITY environment variable.

The local running examples document environment variables for AquaSec credentials, but the MIN_SEVERITY environment variable (supported per context snippet 1, config.py) is not mentioned. Users running locally might prefer to set this via environment variable rather than the --min-severity CLI flag.

📝 Example showing MIN_SEVERITY env var

Consider adding an example that shows the MIN_SEVERITY environment variable:

**With minimum severity threshold:**

```bash
AQUA_KEY=... AQUA_SECRET=... AQUA_GROUP_ID=... AQUA_REPOSITORY_ID=... \
MIN_SEVERITY=high \
PYTHONPATH=src python3 src/security/main.py --repo <owner/repo>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @src/security/README.md around lines 155 - 174, Add documentation for the
MIN_SEVERITY environment variable to the local run examples in README.md (the
same section that shows AQUA_KEY/AQUA_SECRET/AQUA_GROUP_ID/AQUA_REPOSITORY_ID
for dry-run/live/verbose). Show a short example that sets MIN_SEVERITY (e.g.,
MIN_SEVERITY=high) in the environment before invoking python3
src/security/main.py so users can see the env-var alternative to the
--min-severity CLI flag; reference the existing config.py behavior
(MIN_SEVERITY) in the text so readers know this env var is supported.


</details>

<!-- cr-comment:v1:7003ae75b0a69179e4b110ff -->

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @src/security/config.py:

  • Around line 102-103: The validation error message for self.min_severity is
    missing the f-string prefix so the allowed values aren't interpolated; update
    the error append call (the block that checks "if self.min_severity not in
    VALID_SEVERITIES") to use an f-string (e.g., f"...{',
    '.join(sorted(VALID_SEVERITIES))}...") so VALID_SEVERITIES is formatted into the
    message, keeping the same wording and reference to VALID_SEVERITIES and errors.

In @src/security/README.md:

  • Line 188: Update the CLI flags table entry for --min-severity to match the
    workflow input docs: state the default value is 'low' and note that the flag
    is case-insensitive (accepts low/medium/high/critical in any letter case).
    Locate the --min-severity row in the CLI flags table (the entry currently only
    lists allowed severities) and extend its description to include the default
    'low' and the case-insensitivity behavior so both tables are consistent.

Nitpick comments:
In @src/security/README.md:

  • Line 196: Update the "Severity filtering" line to explicitly describe how
    findings with "unknown" severity are treated by the filter: note that with the
    default min-severity=low unknown findings are allowed through, but they are
    excluded when the configured min-severity is raised above low; reference the
    config key "min-severity" and the "unknown" severity behavior implemented in
    sync.py so users understand this edge case.
  • Around line 155-174: Add documentation for the MIN_SEVERITY environment
    variable to the local run examples in README.md (the same section that shows
    AQUA_KEY/AQUA_SECRET/AQUA_GROUP_ID/AQUA_REPOSITORY_ID for dry-run/live/verbose).
    Show a short example that sets MIN_SEVERITY (e.g., MIN_SEVERITY=high) in the
    environment before invoking python3 src/security/main.py so users can see the
    env-var alternative to the --min-severity CLI flag; reference the existing
    config.py behavior (MIN_SEVERITY) in the text so readers know this env var is
    supported.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Organization UI

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `2bccaff2-4da2-4c88-acc7-b1ce5d44814b`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 8737f561167222e0ef72bc18487017dac4533a31 and 3ad778071294df46a00b941abfa5a63029d1837c.

</details>

<details>
<summary>📒 Files selected for processing (10)</summary>

* `.github/workflows/aquasec-scan.yml`
* `src/core/github/issues.py`
* `src/security/README.md`
* `src/security/config.py`
* `src/security/constants.py`
* `src/security/issues/sync.py`
* `src/security/main.py`
* `src/security/services/issue_syncer.py`
* `tests/security/issues/test_sync.py`
* `tests/security/test_config.py`

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread src/security/config.py Outdated
Comment thread src/security/README.md Outdated

@miroslavpojer miroslavpojer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments, pls.

Comment thread .github/workflows/aquasec-scan.yml Outdated
Comment thread src/security/issues/sync.py Outdated
@tmikula-dev tmikula-dev merged commit 1bd7305 into master Jun 15, 2026
9 checks passed
@tmikula-dev tmikula-dev deleted the feature/create-issues-based-on-severity branch June 15, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat: Issue Creation Based on Alert Severity

2 participants