diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f89c722..2d7d40f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -48,6 +48,7 @@ Patterns - Use private methods (`_method_name`) for internal class helpers - All logs must start with " -" prefix (e.g., "Security -") - Never disable pylint behavior in the code +- External links should link to a new window Testing - Mirror src structure: `src/security/module.py` -> `tests/security/test_module.py` @@ -61,5 +62,6 @@ Testing - Use `@pytest.mark.parametrize` for data-driven tests (negative/failure scenarios with multiple similar cases) Quality gates (run after changes, fix only if below threshold) +- Do all changes at once and run the gates afterward - Run all quality gates at once: `make qa` - Once a quality gate passes, do not re-run it in different scenarios \ No newline at end of file diff --git a/.github/workflows/aquasec-scan.yml b/.github/workflows/aquasec-scan.yml index 0eab25c..e4f4681 100644 --- a/.github/workflows/aquasec-scan.yml +++ b/.github/workflows/aquasec-scan.yml @@ -109,7 +109,7 @@ jobs: uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd with: repository: AbsaOSS/organizational-workflows - ref: ${{ github.job_workflow_sha }} + ref: ${{ job.workflow_sha }} path: org-workflows persist-credentials: false diff --git a/src/core/helpers.py b/src/core/helpers.py index ea1d4a6..920384e 100644 --- a/src/core/helpers.py +++ b/src/core/helpers.py @@ -61,6 +61,23 @@ def normalize_path(path: str | None) -> str: return p +def normalize_bullet_list(text: str) -> str: + """Normalize a Markdown bullet list so all items are at the top level. + + Some sources (e.g. AquaSec) emit the first bullet without indentation + and subsequent bullets with leading whitespace, producing a nested list. + This function strips leading whitespace from any line whose stripped form + starts with ``- ``, so every item renders at the same level. + """ + if not text: + return text + lines = [] + for line in text.splitlines(): + stripped = line.lstrip() + lines.append(f"- {stripped[2:]}" if stripped.startswith("- ") else line) + return "\n".join(lines) + + def sanitize_markdown(text: str) -> str: """Escape block-level Markdown so text renders as plain content in an issue body.""" if not text: diff --git a/src/core/rendering.py b/src/core/rendering.py index be5b40c..95c1de1 100644 --- a/src/core/rendering.py +++ b/src/core/rendering.py @@ -20,6 +20,8 @@ import re from typing import Any +from security.constants import NOT_AVAILABLE + PLACEHOLDER_RE = re.compile(r"\{\{\s*([a-zA-Z0-9_\.]+)\s*\}\}") @@ -49,3 +51,57 @@ def repl(match: re.Match[str]) -> str: return str(v) return PLACEHOLDER_RE.sub(repl, template) + + +def strip_na_sections(body: str) -> str: + """Remove N/A fields and empty sections from a rendered Markdown body. + + - Lines like ``- **Key:** N/A`` (with optional trailing whitespace/italics) are removed. + - Section headers (``## Heading``) with no remaining content are removed. + """ + lines = body.split("\n") + filtered: list[str] = [] + i = 0 + while i < len(lines): + line = lines[i] + + # Remove bullet-list N/A fields: "- **Key:** N/A" with optional trailing content + if re.match(r"^\s*-\s+\*\*[^*]+:\*\*\s*N/A\s*$", line): + i += 1 + # Also remove subsequent italic description lines + while i < len(lines) and re.match(r"^\s+\*\(.*\)\*\s*$", lines[i]): + i += 1 + continue + + filtered.append(line) + i += 1 + + # Second pass: remove ## headings whose section body is empty or just N/A + result: list[str] = [] + i = 0 + while i < len(filtered): + line = filtered[i] + + if re.match(r"^##\s+", line): + # Collect the section body (until next ## heading or end) + section_header = line + section_body_lines: list[str] = [] + j = i + 1 + while j < len(filtered) and not re.match(r"^##\s+", filtered[j]): + section_body_lines.append(filtered[j]) + j += 1 + + # Check if section body is effectively empty + body_text = "\n".join(section_body_lines).strip() + if not body_text or body_text == NOT_AVAILABLE: + i = j + continue + + result.append(section_header) + result.extend(section_body_lines) + i = j + else: + result.append(line) + i += 1 + + return "\n".join(result) diff --git a/src/security/collect_alert.py b/src/security/collect_alert.py index 110fec3..789301b 100644 --- a/src/security/collect_alert.py +++ b/src/security/collect_alert.py @@ -31,7 +31,6 @@ logger = logging.getLogger(__name__) VALID_STATES = {"open", "dismissed", "fixed", "all"} - RULE_DETAIL_KEYS = [ "Type", "Severity", @@ -47,6 +46,7 @@ "OWASP", "References", ] +MULTILINE_KEYS = {"references", "owasp"} def _snake_case(name: str) -> str: @@ -59,12 +59,33 @@ def _help_value(rule_help: str | None, name: str) -> str | None: if not rule_help: return None m = re.search(rf"\*\*{re.escape(name)}:\*\*\s*([^\n\r]+)", rule_help, re.IGNORECASE) - return m.group(1) if m else None + return m.group(1).strip() if m else None + + +def _help_multiline_value(rule_help: str | None, name: str) -> str | None: + """Extract a multi-line value from ``**Name:**`` markup in the rule help text.""" + if not rule_help: + return None + m = re.search( + rf"\*\*{re.escape(name)}:\*\*[ \t]*([^\n\r]*(?:\n(?!\*\*[A-Za-z][\w\s]*?:\*\*)[^\n\r]*)*)", + rule_help, + re.IGNORECASE, + ) + if not m: + return None + return m.group(1).strip() or None def _parse_rule_details(rule_help: str | None) -> dict[str, str | None]: """Extract known rule detail fields from ``**Key:** value`` markup in rule help.""" - return {_snake_case(key): _help_value(rule_help, key) for key in RULE_DETAIL_KEYS} + result: dict[str, str | None] = {} + for key in RULE_DETAIL_KEYS: + snake = _snake_case(key) + if snake in MULTILINE_KEYS: + result[snake] = _help_multiline_value(rule_help, key) + else: + result[snake] = _help_value(rule_help, key) + return result def _parse_alert_details(message_text: str) -> dict[str, str]: diff --git a/src/security/constants.py b/src/security/constants.py index ae85d38..494a008 100644 --- a/src/security/constants.py +++ b/src/security/constants.py @@ -24,4 +24,7 @@ SECMETA_TYPE_PARENT = "parent" SECMETA_TYPE_CHILD = "child" +SECURITY_FINDING_DEFAULT = "Security finding" NOT_AVAILABLE = "N/A" + +GITHUB_BASE_URL = "https://github.com" diff --git a/src/security/issues/builder.py b/src/security/issues/builder.py index 3e7cbac..d09121e 100644 --- a/src/security/issues/builder.py +++ b/src/security/issues/builder.py @@ -16,17 +16,31 @@ """Issue title / body construction from Alert dataclasses.""" +import html from typing import Any -from core.helpers import iso_date, sanitize_markdown -from core.rendering import render_markdown_template +from core.helpers import iso_date, normalize_bullet_list, sanitize_markdown +from core.rendering import render_markdown_template, strip_na_sections + +from security.constants import NOT_AVAILABLE, SECMETA_TYPE_PARENT, SECURITY_FINDING_DEFAULT, GITHUB_BASE_URL -from security.constants import NOT_AVAILABLE, SECMETA_TYPE_PARENT from security.alerts.models import Alert from security.issues.secmeta import render_secmeta from security.issues.templates import CHILD_BODY_TEMPLATE, PARENT_BODY_TEMPLATE +def _new_window_link(text: str, url: str | None) -> str: + """Wrap *text* in an HTML anchor that opens in a new window. + + Returns plain *text* when *url* is empty or ``N/A``. + """ + safe_text = html.escape(text or "", quote=False) + if url and url != NOT_AVAILABLE: + safe_url = html.escape(url, quote=True) + return f'{safe_text}' + return safe_text + + def _synthesize_references(alert: Alert) -> str: """Build a Markdown bullet list from metadata URLs when rule_details.references is absent.""" lines = [] @@ -45,7 +59,6 @@ def _synthesize_owasp(alert: Alert) -> str: def alert_extra_data(alert: Alert) -> dict[str, Any]: """Build the extra-data dict for parent issue templates from nested alert data.""" - rule_id = alert.metadata.rule_id references = alert.rule_details.references if references == NOT_AVAILABLE: references = _synthesize_references(alert) @@ -54,14 +67,15 @@ def alert_extra_data(alert: Alert) -> dict[str, Any]: owasp = _synthesize_owasp(alert) return { - "cve": rule_id if rule_id.upper().startswith("CVE-") else NOT_AVAILABLE, - "owasp": owasp, + "rule_id": alert.metadata.rule_id, + "owasp": sanitize_markdown(normalize_bullet_list(owasp)), "category": alert.metadata.rule_name or NOT_AVAILABLE, - "impact": alert.rule_details.impact, - "likelihood": alert.rule_details.likelihood, - "confidence": alert.rule_details.confidence, + "advisory_url": alert.metadata.help_uri or NOT_AVAILABLE, + "impact": sanitize_markdown(alert.rule_details.impact), + "likelihood": sanitize_markdown(alert.rule_details.likelihood), + "confidence": sanitize_markdown(alert.rule_details.confidence), "remediation": sanitize_markdown(alert.rule_details.remediation), - "references": references, + "references": sanitize_markdown(normalize_bullet_list(references)), } @@ -70,10 +84,9 @@ def classify_category(alert: Alert) -> str: return alert.metadata.rule_name -def build_parent_issue_title(rule_id: str, severity: str = "") -> str: +def build_parent_issue_title(rule_id: str) -> str: """Build the title string for a parent issue.""" - sev_tag = f"[{severity.upper()}] " if severity else "" - return f"{sev_tag}Security Alert – {rule_id}".strip() + return f"Security Alert – {rule_id}".strip() def build_parent_template_values(alert: Alert, *, rule_id: str, severity: str) -> dict[str, Any]: @@ -85,11 +98,11 @@ def build_parent_template_values(alert: Alert, *, rule_id: str, severity: str) - extra = alert_extra_data(alert) return { - "category": alert.metadata.rule_name or NOT_AVAILABLE, - "avd_id": alert.alert_details.vulnerability or rule_id, "title": sanitize_markdown(alert.metadata.rule_description or rule_id), + "category": alert.metadata.rule_name or NOT_AVAILABLE, "severity": severity, "published_date": iso_date(alert.rule_details.published_date or NOT_AVAILABLE), + "short_description": sanitize_markdown(alert.metadata.rule_description or NOT_AVAILABLE), "package_name": alert.rule_details.package_name, "fixed_version": alert.rule_details.fixed_version, "extraData": extra, @@ -110,20 +123,20 @@ def build_parent_issue_body(alert: Alert) -> str: } values = build_parent_template_values(alert, rule_id=rule_id, severity=severity) - human_body = render_markdown_template(PARENT_BODY_TEMPLATE, values).strip() + "\n" + human_body = strip_na_sections(render_markdown_template(PARENT_BODY_TEMPLATE, values)).strip() + "\n" return render_secmeta(secmeta) + "\n\n" + human_body def build_issue_title( rule_description: str | None, - rule_name: str | None, - rule_id: str, fingerprint: str, + severity: str = "", ) -> str: """Build the title string for a child issue.""" prefix = fingerprint[:8] if fingerprint else NOT_AVAILABLE - summary = (rule_description or rule_name or rule_id or "Security finding").strip() - return f"[SEC][FP={prefix}] {summary}" + sev_tag = f":{severity.upper()}" if severity else "" + summary = (rule_description or SECURITY_FINDING_DEFAULT).strip() + return f"[SEC{sev_tag}][FP={prefix}] {summary}" def build_child_issue_body(alert: Alert) -> str: @@ -132,42 +145,58 @@ def build_child_issue_body(alert: Alert) -> str: if not repo_full: repo_full = alert.alert_details.repository - vulnerability = alert.alert_details.vulnerability - avd_id = vulnerability if vulnerability.startswith("AVD-") else NOT_AVAILABLE - title = sanitize_markdown(alert.metadata.rule_description or alert.metadata.rule_id) + # Use artifact as the display file name, fall back to scm_file basename + artifact = alert.alert_details.artifact scm_file = alert.alert_details.scm_file + + if artifact and artifact != NOT_AVAILABLE: + file_display = artifact.rsplit("/", 1)[-1] if "/" in artifact else artifact + elif scm_file and scm_file != NOT_AVAILABLE: + file_display = scm_file.rsplit("/", 1)[-1] + else: + file_display = NOT_AVAILABLE + start_line = alert.metadata.start_line start_line_str = str(start_line) if start_line is not None else "" - # Build a display name (filename only) and permalink with #L anchor - file_name = scm_file.rsplit("/", 1)[-1] if scm_file and scm_file != NOT_AVAILABLE else None if scm_file and scm_file != NOT_AVAILABLE and start_line_str: file_permalink = f"{scm_file}#L{start_line_str}" - file_display = f"{file_name}#L{start_line_str}" else: file_permalink = scm_file if scm_file != NOT_AVAILABLE else "" - file_display = file_name or NOT_AVAILABLE + + # Start/End line logic + end_line = alert.metadata.end_line + if start_line is not None: + start_line_val = str(start_line) + end_line_val = str(end_line) if end_line is not None else start_line_val + else: + start_line_val = NOT_AVAILABLE + end_line_val = NOT_AVAILABLE alert_hash = alert.alert_details.alert_hash message = sanitize_markdown(alert.alert_details.message) category = classify_category(alert) + repository_link = _new_window_link(repo_full, f"{GITHUB_BASE_URL}/{repo_full}") values: dict[str, Any] = { + "severity": alert.metadata.severity, "category": category or NOT_AVAILABLE, - "avd_id": avd_id, + "rule_id": alert.metadata.rule_id, "alert_hash": alert_hash, "title": title, + "first_seen": iso_date(alert.alert_details.first_seen or alert.metadata.created_at), "message": message, - "repository_full_name": repo_full, + "repository_link": repository_link, "file_display": file_display, "file_permalink": file_permalink, + "start_line": start_line_val, + "end_line": end_line_val, "package_name": alert.rule_details.package_name, "installed_version": alert.alert_details.installed_version, "fixed_version": alert.rule_details.fixed_version, "reachable": alert.alert_details.reachable, - "first_seen": iso_date(alert.alert_details.first_seen or alert.metadata.created_at or NOT_AVAILABLE), } - return render_markdown_template(CHILD_BODY_TEMPLATE, values).strip() + "\n" + return strip_na_sections(render_markdown_template(CHILD_BODY_TEMPLATE, values)).strip() + "\n" diff --git a/src/security/issues/models.py b/src/security/issues/models.py index 3bdaa9b..62e2569 100644 --- a/src/security/issues/models.py +++ b/src/security/issues/models.py @@ -97,7 +97,6 @@ class AlertContext: rule_name: str rule_description: str severity: str - cve: str path: str start_line: int | None end_line: int | None diff --git a/src/security/issues/sync.py b/src/security/issues/sync.py index 95ac10e..73b4e05 100644 --- a/src/security/issues/sync.py +++ b/src/security/issues/sync.py @@ -37,7 +37,7 @@ ) from core.github.projects import ProjectPrioritySync, gh_project_get_priority_field from core.models import Issue -from core.rendering import render_markdown_template +from core.rendering import render_markdown_template, strip_na_sections from security.alerts.models import Alert from security.constants import ( @@ -45,7 +45,6 @@ LABEL_SCOPE_SECURITY, LABEL_SEC_ADEPT_TO_CLOSE, LABEL_TYPE_TECH_DEBT, - NOT_AVAILABLE, SECMETA_TYPE_CHILD, SECMETA_TYPE_PARENT, ) @@ -249,9 +248,11 @@ def ensure_parent_issue( rebuilt = ( render_secmeta(existing_secmeta) + "\n\n" - + render_markdown_template( - PARENT_BODY_TEMPLATE, - build_parent_template_values(alert, rule_id=rule_id, severity=severity_stored), + + strip_na_sections( + render_markdown_template( + PARENT_BODY_TEMPLATE, + build_parent_template_values(alert, rule_id=rule_id, severity=severity_stored), + ) ).strip() + "\n" ) @@ -263,7 +264,7 @@ def ensure_parent_issue( existing.body = rebuilt # Detect parent title drift and update when needed. - expected_title = build_parent_issue_title(rule_id, severity_stored) + expected_title = build_parent_issue_title(rule_id) if expected_title != (existing.title or ""): if dry_run: logging.info( @@ -281,7 +282,7 @@ def ensure_parent_issue( return existing - title = build_parent_issue_title(rule_id, alert.metadata.severity) + title = build_parent_issue_title(rule_id) body = build_parent_issue_body(alert) labels = [LABEL_SCOPE_SECURITY, LABEL_TYPE_TECH_DEBT, LABEL_EPIC] if dry_run: @@ -355,7 +356,7 @@ def _handle_new_child_issue( human_body = build_child_issue_body(ctx.alert) body = render_secmeta(secmeta) + "\n\n" + human_body - title = build_issue_title(ctx.rule_description, ctx.rule_name, ctx.rule_id, ctx.fingerprint) + title = build_issue_title(ctx.rule_description, ctx.fingerprint, ctx.severity) if sync.dry_run: labels = [LABEL_SCOPE_SECURITY, LABEL_TYPE_TECH_DEBT] @@ -571,7 +572,7 @@ def _sync_child_title_and_labels( issue: Issue, ) -> None: """Fix title drift and ensure required labels and priority on the child issue.""" - expected_title = build_issue_title(ctx.rule_description, ctx.rule_name, ctx.rule_id, ctx.fingerprint) + expected_title = build_issue_title(ctx.rule_description, ctx.fingerprint, ctx.severity) if expected_title != (issue.title or ""): if sync.dry_run: logging.info( @@ -671,7 +672,6 @@ def ensure_issue( return rule_id = alert.metadata.rule_id - cve = rule_id if rule_id.upper().startswith("CVE-") else NOT_AVAILABLE path = normalize_path(alert.metadata.file) start_line = alert.metadata.start_line @@ -713,7 +713,6 @@ def ensure_issue( rule_name=alert.metadata.rule_name, rule_description=alert.metadata.rule_description, severity=alert.metadata.severity, - cve=cve, path=path, start_line=start_line, end_line=end_line, diff --git a/src/security/issues/templates.py b/src/security/issues/templates.py index d2d7ede..d269f1f 100644 --- a/src/security/issues/templates.py +++ b/src/security/issues/templates.py @@ -16,15 +16,13 @@ """Security-specific Markdown body templates.""" -PARENT_BODY_TEMPLATE = """# Security Alert – {{ avd_id }} +PARENT_BODY_TEMPLATE = """## General Information -## General Information - -- **Category:** {{ category }} -- **AVD ID:** {{ avd_id }} - **Title:** {{ title }} +- **Category:** {{ category }} - **Severity:** {{ severity }} - **Published date:** {{ published_date }} +- **Short Description:** {{ short_description }} ## Affected Package @@ -33,9 +31,9 @@ ## Classification -- **CVE:** {{ extraData.cve }} -- **OWASP:** {{ extraData.owasp }} +- **Rule:** {{ extraData.rule_id }} - **Category:** {{ extraData.category }} +- **Advisory URL:** {{ extraData.advisory_url }} ## Risk Assessment @@ -46,10 +44,14 @@ - **Confidence:** {{ extraData.confidence }} *(How confident the finding is; likelihood of false positive)* -## Recommended Remediation +## Remediation {{ extraData.remediation }} +## OWASP + +{{ extraData.owasp }} + ## References {{ extraData.references }} @@ -58,19 +60,23 @@ CHILD_BODY_TEMPLATE = """## General Information +- **Severity:** {{ severity }} +- **Title:** {{ title }} - **Category:** {{ category }} -- **AVD ID:** {{ avd_id }} +- **Rule:** {{ rule_id }} - **Alert hash:** {{ alert_hash }} -- **Title:** {{ title }} +- **First seen:** {{ first_seen }} -## Vulnerability Description +## Description {{ message }} ## Location -- **Repository:** {{ repository_full_name }} +- **Repository:** {{ repository_link }} - **File:** [{{ file_display }}]({{ file_permalink }}) +- **Start Line:** {{ start_line }} +- **End Line:** {{ end_line }} ## Dependency Details @@ -78,8 +84,4 @@ - **Installed version:** {{ installed_version }} - **Fixed version:** {{ fixed_version }} - **Reachable:** {{ reachable }} - -## Detection Timeline - -- **First seen:** {{ first_seen }} """ diff --git a/tests/core/test_helpers.py b/tests/core/test_helpers.py index 0ae35a7..8e2f245 100644 --- a/tests/core/test_helpers.py +++ b/tests/core/test_helpers.py @@ -16,7 +16,7 @@ import pytest -from core.helpers import sanitize_markdown +from core.helpers import normalize_bullet_list, sanitize_markdown # sanitize_markdown @@ -71,3 +71,28 @@ def test_real_aquasec_message() -> None: result = sanitize_markdown(msg) assert not result.startswith("## ") assert result.startswith(r"\## ") + + +# normalize_bullet_list + + +@pytest.mark.parametrize("text", [None, ""], ids=["none", "empty"]) +def test_normalize_bullet_list_passthrough_empty(text: str | None) -> None: + assert normalize_bullet_list(text) == text + + +def test_normalize_bullet_list_already_flat() -> None: + text = "- https://example.com/a\n- https://example.com/b" + assert normalize_bullet_list(text) == text + + +def test_normalize_bullet_list_normalizes_indented() -> None: + text = "- https://first.example.com\n - https://second.example.com\n - https://third.example.com" + result = normalize_bullet_list(text) + assert result == "- https://first.example.com\n- https://second.example.com\n- https://third.example.com" + + +def test_normalize_bullet_list_preserves_non_bullet_lines() -> None: + text = "Some intro text\n- https://example.com\n - https://other.com\nTrailing text" + result = normalize_bullet_list(text) + assert result == "Some intro text\n- https://example.com\n- https://other.com\nTrailing text" diff --git a/tests/security/issues/test_builder.py b/tests/security/issues/test_builder.py index 7bbcd56..b76eac6 100644 --- a/tests/security/issues/test_builder.py +++ b/tests/security/issues/test_builder.py @@ -48,17 +48,11 @@ def test_extra_data_from_nested() -> None: }) extra = alert_extra_data(alert) assert isinstance(extra, dict) - assert extra["cve"] == "CVE-123" + assert extra["rule_id"] == "CVE-123" assert extra["confidence"] == "error" assert extra["category"] == "sast" -def test_extra_data_non_cve() -> None: - """Non-CVE rule_id results in N/A for cve field.""" - alert = Alert.from_dict({"metadata": {"rule_id": "RULE-1", "rule_name": "sast"}, "rule_details": {}}) - assert alert_extra_data(alert)["cve"] == "N/A" - - # ===================================================================== # classify_category # ===================================================================== @@ -78,12 +72,8 @@ def test_classify_category(raw: dict, expected: str) -> None: # ===================================================================== -@pytest.mark.parametrize("severity, expected", [ - ("high", "[HIGH] Security Alert \u2013 CVE-2026-25755"), - ("", "Security Alert \u2013 CVE-2026-25755"), -], ids=["with_severity", "without_severity"]) -def test_build_parent_issue_title(severity: str, expected: str) -> None: - assert expected == build_parent_issue_title("CVE-2026-25755", severity) +def test_build_parent_issue_title() -> None: + assert "Security Alert \u2013 CVE-2026-25755" == build_parent_issue_title("CVE-2026-25755") # ===================================================================== @@ -101,9 +91,9 @@ def test_vuln_category_from_rule_name(vuln_alert: Alert) -> None: assert vals["category"] == "vulnerabilities" -def test_avd_id_uses_vulnerability(sast_alert: Alert) -> None: +def test_avd_id_removed_from_values(sast_alert: Alert) -> None: vals = build_parent_template_values(sast_alert, rule_id="req-with-very-false-aquasec-python", severity="high") - assert vals["avd_id"] == "req-with-very-false-aquasec-python" + assert "avd_id" not in vals @pytest.mark.parametrize("rule_details", [ @@ -130,13 +120,13 @@ def test_extra_data_references(vuln_alert: Alert) -> None: def test_all_template_keys_present(sast_alert: Alert) -> None: vals = build_parent_template_values(sast_alert, rule_id="test", severity="high") - required = {"category", "avd_id", "title", "severity", "published_date", "package_name", "fixed_version", "extraData"} + required = {"category", "title", "severity", "published_date", "short_description", "package_name", "fixed_version", "extraData"} assert required.issubset(vals.keys()) def test_extra_data_sub_keys(sast_alert: Alert) -> None: extra = build_parent_template_values(sast_alert, rule_id="test", severity="high")["extraData"] - required_extra = {"cve", "owasp", "category", "impact", "likelihood", "confidence", "remediation", "references"} + required_extra = {"rule_id", "owasp", "category", "advisory_url", "impact", "likelihood", "confidence", "remediation", "references"} assert required_extra.issubset(extra.keys()) @@ -180,6 +170,24 @@ def test_references_not_overridden_when_present() -> None: assert "should-not-appear" not in refs +def test_references_indented_bullets_normalized() -> None: + """Indented bullet items from AquaSec are normalized to top-level bullets.""" + alert = Alert.from_dict({ + "metadata": {"rule_id": "CVE-2026-XXXX"}, + "rule_details": { + "references": ( + "- https://first.example.com\n" + " - https://second.example.com\n" + " - https://third.example.com" + ), + }, + }) + refs = alert_extra_data(alert)["references"] + for line in refs.splitlines(): + if line.strip(): + assert line.startswith("- "), f"Expected top-level bullet, got: {line!r}" + + # ===================================================================== # build_parent_issue_body # ===================================================================== @@ -209,17 +217,16 @@ def test_parent_body_confidence(vuln_alert: Alert) -> None: # ===================================================================== -@pytest.mark.parametrize("description, rule_name, rule_id, fingerprint, expected", [ - ("A description", "sast", "rule-123", "a1b2c3d4e5f6", "[SEC][FP=a1b2c3d4] A description"), - (None, "sast", "rule-123", "abcdef12", "sast"), - (None, None, "rule-123", "abcdef12", "rule-123"), - (None, None, "", "abcdef12", "Security finding"), - ("A description", "sast", "rule-123", "", "N/A"), -], ids=["full_format", "fallback_rule_name", "fallback_rule_id", "fallback_default", "empty_fingerprint"]) +@pytest.mark.parametrize("description, fingerprint, severity, expected", [ + ("A description", "a1b2c3d4e5f6", "high", "[SEC:HIGH][FP=a1b2c3d4] A description"), + (None, "abcdef12", "medium", "[SEC:MEDIUM][FP=abcdef12] Security finding"), + (None, "abcdef12", "", "[SEC][FP=abcdef12] Security finding"), + ("A description", "", "critical", "[SEC:CRITICAL][FP=N/A] A description"), +], ids=["full_format", "no_description_fallback", "no_severity", "empty_fingerprint"]) def test_build_issue_title( - description: str | None, rule_name: str | None, rule_id: str, fingerprint: str, expected: str + description: str | None, fingerprint: str, severity: str, expected: str ) -> None: - assert expected in build_issue_title(description, rule_name, rule_id, fingerprint) + assert expected == build_issue_title(description, fingerprint, severity) # ===================================================================== @@ -238,7 +245,10 @@ def test_build_issue_title( "95", "False", "2025-09-17", -], ids=["title", "alert_hash", "message", "repository", "scm_url", "target_line", "reachable", "first_seen"]) + "**Severity:** high", + 'target="_blank"', +], ids=["title", "alert_hash", "message", "repository", "scm_url", "target_line", "reachable", "first_seen", + "severity", "new_window_links"]) def test_sast_child_body_contains(sast_alert: Alert, expected: str) -> None: assert expected in build_child_issue_body(sast_alert) @@ -253,12 +263,11 @@ def test_sast_child_body_contains(sast_alert: Alert, expected: str) -> None: "068f963657211cd416dac1f9b30d606c", "2026-02-20", "## General Information", - "## Vulnerability Description", + "## Description", "## Location", "## Dependency Details", - "## Detection Timeline", ], ids=["title", "installed_version", "reachable", "scm_file", "alert_hash", "first_seen", - "section_general", "section_vuln", "section_location", "section_dependency", "section_timeline"]) + "section_general", "section_description", "section_location", "section_dependency"]) def test_vuln_child_body_contains(vuln_alert: Alert, expected: str) -> None: assert expected in build_child_issue_body(vuln_alert) @@ -267,9 +276,9 @@ def test_vuln_child_body_contains(vuln_alert: Alert, expected: str) -> None: @pytest.mark.parametrize("expected", [ "pipelineMisconfigurations", - "**Installed version:**", + "**Reachable:**", "False", -], ids=["category", "installed_version_label", "reachable"]) +], ids=["category", "reachable_label", "reachable"]) def test_pipeline_child_body_contains(pipeline_alert: Alert, expected: str) -> None: assert expected in build_child_issue_body(pipeline_alert) @@ -323,7 +332,7 @@ def test_message_with_heading_is_escaped() -> None: "rule_details": {}, }) body = build_child_issue_body(alert) - assert not any(l.strip().startswith("## Black") for l in body.split("\n")), "Message heading should be escaped" + assert not any(line.strip().startswith("## Black") for line in body.split("\n")), "Message heading should be escaped" assert r"\## Black" in body @@ -334,5 +343,5 @@ def test_parent_remediation_heading_is_escaped() -> None: "rule_details": {"remediation": "## Step 1\nDo something **important**."}, }, repo="org/repo") body = build_parent_issue_body(alert) - assert not any(l.strip() == "## Step 1" for l in body.split("\n")), "Remediation heading should be escaped" + assert not any(line.strip() == "## Step 1" for line in body.split("\n")), "Remediation heading should be escaped" assert r"\## Step 1" in body diff --git a/tests/security/issues/test_models.py b/tests/security/issues/test_models.py index 9df3447..0b46aa1 100644 --- a/tests/security/issues/test_models.py +++ b/tests/security/issues/test_models.py @@ -112,7 +112,7 @@ def test_alert_context_creation() -> None: repo="org/repo", tool="AquaSec", rule_id="R1", rule_name="sast", rule_description="Test finding description", - severity="high", cve="CVE-79", path="src/f.py", + severity="high", path="src/f.py", start_line=10, end_line=20, commit_sha="abc123", ) assert ctx.alert_number == 1 diff --git a/tests/security/issues/test_secmeta.py b/tests/security/issues/test_secmeta.py index 1ac9017..b90d2b3 100644 --- a/tests/security/issues/test_secmeta.py +++ b/tests/security/issues/test_secmeta.py @@ -116,8 +116,8 @@ def test_preferred_order() -> None: rendered = render_secmeta(data) lines = rendered.strip().split("\n") # type should appear before fingerprint per preferred_order - type_idx = next(i for i, l in enumerate(lines) if "type=" in l) - fp_idx = next(i for i, l in enumerate(lines) if "fingerprint=" in l) + type_idx = next(i for i, line in enumerate(lines) if "type=" in line) + fp_idx = next(i for i, line in enumerate(lines) if "fingerprint=" in line) assert type_idx < fp_idx def test_secmeta_roundtrip() -> None: diff --git a/tests/security/issues/test_sync.py b/tests/security/issues/test_sync.py index affc440..128390b 100644 --- a/tests/security/issues/test_sync.py +++ b/tests/security/issues/test_sync.py @@ -78,7 +78,6 @@ def _make_alert_context(**overrides: Any) -> AlertContext: rule_name="sast", rule_description="Test finding description", severity="high", - cve="CVE-2026-1234", path="src/main.py", start_line=10, end_line=20, @@ -484,7 +483,7 @@ def test_sync_title_already_correct(mocker: MockerFixture) -> None: """Title is not updated when it matches the expected format.""" mock_labels = mocker.patch("security.issues.sync.gh_issue_add_labels") from security.issues.builder import build_issue_title - title = build_issue_title("Test finding description", "sast", "CVE-2026-1234", "fp_test_123") + title = build_issue_title("Test finding description", "fp_test_123", "high") issue = Issue(number=1, state="open", title=title, body="b") ctx = _make_alert_context(rule_name="sast", rule_id="CVE-2026-1234", fingerprint="fp_test_123") sync = _make_sync_context() diff --git a/tests/security/issues/test_templates.py b/tests/security/issues/test_templates.py index 9399980..470b3cb 100644 --- a/tests/security/issues/test_templates.py +++ b/tests/security/issues/test_templates.py @@ -30,16 +30,18 @@ def test_contains_required_sections() -> None: assert "## Affected Package" in PARENT_BODY_TEMPLATE assert "## Classification" in PARENT_BODY_TEMPLATE assert "## Risk Assessment" in PARENT_BODY_TEMPLATE - assert "## Recommended Remediation" in PARENT_BODY_TEMPLATE + assert "## Remediation" in PARENT_BODY_TEMPLATE assert "## References" in PARENT_BODY_TEMPLATE + assert "## OWASP" in PARENT_BODY_TEMPLATE def test_contains_all_placeholders() -> None: """All documented placeholders are present.""" expected_placeholders = [ - "{{ avd_id }}", "{{ category }}", "{{ title }}", "{{ severity }}", - "{{ published_date }}", "{{ package_name }}", - "{{ fixed_version }}", "{{ extraData.cve }}", "{{ extraData.owasp }}", - "{{ extraData.category }}", "{{ extraData.impact }}", + "{{ category }}", "{{ title }}", "{{ severity }}", + "{{ published_date }}", "{{ short_description }}", "{{ package_name }}", + "{{ fixed_version }}", "{{ extraData.rule_id }}", "{{ extraData.owasp }}", + "{{ extraData.category }}", "{{ extraData.advisory_url }}", + "{{ extraData.impact }}", "{{ extraData.likelihood }}", "{{ extraData.confidence }}", "{{ extraData.remediation }}", "{{ extraData.references }}", ] @@ -49,17 +51,18 @@ def test_contains_all_placeholders() -> None: def test_renders_without_error() -> None: """Template renders successfully with minimal values.""" values = { - "avd_id": "CVE-123", "category": "sast", "title": "Test", "severity": "high", "published_date": "2026-01-01", + "short_description": "Test short desc", "package_name": "pkg", "fixed_version": "2.0", "extraData": { - "cve": "CVE-123", + "rule_id": "CVE-123", "owasp": "A07", "category": "sast", + "advisory_url": "https://example.com/advisory", "impact": "high", "likelihood": "medium", "confidence": "high", @@ -74,11 +77,13 @@ def test_renders_without_error() -> None: def test_renders_impact_and_likelihood() -> None: """impact and likelihood values appear in the rendered parent template.""" values = { - "avd_id": "CVE-1", "category": "sast", "title": "T", + "category": "sast", "title": "T", "severity": "high", "published_date": "2026-01-01", + "short_description": "desc", "package_name": "pkg", "fixed_version": "2.0", "extraData": { - "cve": "N/A", "owasp": "N/A", "category": "sast", + "rule_id": "N/A", "owasp": "N/A", "category": "sast", + "advisory_url": "N/A", "impact": "medium", "likelihood": "medium", "confidence": "error", "remediation": "upgrade", "references": "https://example.com", @@ -92,35 +97,37 @@ def test_renders_impact_and_likelihood() -> None: def test_child_contains_required_sections() -> None: assert "## General Information" in CHILD_BODY_TEMPLATE - assert "## Vulnerability Description" in CHILD_BODY_TEMPLATE + assert "## Description" in CHILD_BODY_TEMPLATE assert "## Location" in CHILD_BODY_TEMPLATE assert "## Dependency Details" in CHILD_BODY_TEMPLATE - assert "## Detection Timeline" in CHILD_BODY_TEMPLATE def test_child_contains_all_placeholders() -> None: expected_placeholders = [ - "{{ avd_id }}", "{{ alert_hash }}", "{{ title }}", "{{ message }}", - "{{ repository_full_name }}", "{{ file_display }}", "{{ file_permalink }}", + "{{ severity }}", "{{ rule_id }}", "{{ alert_hash }}", "{{ title }}", "{{ message }}", + "{{ repository_link }}", "{{ file_display }}", "{{ file_permalink }}", "{{ package_name }}", "{{ installed_version }}", "{{ fixed_version }}", - "{{ reachable }}", "{{ first_seen }}", + "{{ reachable }}", "{{ first_seen }}", "{{ start_line }}", "{{ end_line }}", ] for ph in expected_placeholders: assert ph in CHILD_BODY_TEMPLATE, f"Missing placeholder: {ph}" def test_child_renders_without_error() -> None: values = { - "avd_id": "CVE-123", + "severity": "high", + "rule_id": "CVE-123", "alert_hash": "abc", "title": "Test", "message": "msg", - "repository_full_name": "org/repo", - "file_display": "file.py#L10", + "first_seen": "2026-01-01", + "repository_link": 'org/repo', + "file_display": "file.py", "file_permalink": "https://example.com/file.py#L10", + "start_line": "10", + "end_line": "10", "package_name": "pkg", "installed_version": "1.0", "fixed_version": "2.0", "reachable": "True", - "first_seen": "2026-01-01", } result = render_markdown_template(CHILD_BODY_TEMPLATE, values) assert "CVE-123" in result diff --git a/tests/security/test_collect_alert.py b/tests/security/test_collect_alert.py index bc701b3..88c157f 100644 --- a/tests/security/test_collect_alert.py +++ b/tests/security/test_collect_alert.py @@ -32,6 +32,7 @@ _parse_rule_details, _snake_case, _help_value, + _help_multiline_value, main, parse_args, ) @@ -143,6 +144,52 @@ def test_help_value_stops_at_newline() -> None: assert _help_value(text, "Type") == "sast" +# _help_multiline_value + + +def test_help_multiline_value_single_line() -> None: + text = "**References:** https://example.com" + assert _help_multiline_value(text, "References") == "https://example.com" + + +def test_help_multiline_value_bullet_list() -> None: + text = ( + "**References:**\n" + "- https://example.com/ref1\n" + "- https://example.com/ref2\n" + "**Type:** sast" + ) + result = _help_multiline_value(text, "References") + assert "https://example.com/ref1" in result + assert "https://example.com/ref2" in result + + +def test_help_multiline_value_not_found() -> None: + assert _help_multiline_value("no match here", "References") is None + + +def test_help_multiline_value_none_input() -> None: + assert _help_multiline_value(None, "References") is None + + +def test_help_multiline_value_empty_value() -> None: + text = "**References:**\n**Type:** sast" + assert _help_multiline_value(text, "References") is None + + +def test_parse_rule_details_multiline_references() -> None: + rule_help = ( + "**Type:** sast\n" + "**References:**\n" + "- https://example.com/ref1\n" + "- https://example.com/ref2\n" + "**Severity:** HIGH" + ) + details = _parse_rule_details(rule_help) + assert "https://example.com/ref1" in details["references"] + assert "https://example.com/ref2" in details["references"] + + def test_parse_rule_details_extracts_fields() -> None: rule_help = ( "**Type:** sast\n" diff --git a/tests/security/test_constants.py b/tests/security/test_constants.py deleted file mode 100644 index 91560d2..0000000 --- a/tests/security/test_constants.py +++ /dev/null @@ -1,46 +0,0 @@ -# -# Copyright 2026 ABSA Group Limited -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -"""Unit tests for ``utils.constants``.""" - -from security.constants import ( - LABEL_EPIC, - LABEL_SCOPE_SECURITY, - LABEL_SEC_ADEPT_TO_CLOSE, - LABEL_TYPE_TECH_DEBT, - SECMETA_TYPE_CHILD, - SECMETA_TYPE_PARENT, -) - - -def test_scope_security() -> None: - assert LABEL_SCOPE_SECURITY == "scope:security" - -def test_type_tech_debt() -> None: - assert LABEL_TYPE_TECH_DEBT == "type:tech-debt" - -def test_epic() -> None: - assert LABEL_EPIC == "epic" - -def test_adept_to_close() -> None: - assert LABEL_SEC_ADEPT_TO_CLOSE == "sec:adept-to-close" - - -def test_parent() -> None: - assert SECMETA_TYPE_PARENT == "parent" - -def test_child() -> None: - assert SECMETA_TYPE_CHILD == "child"