From dbbdb64f29b4d1d49ea85e0db430069d6e816231 Mon Sep 17 00:00:00 2001 From: mimran-khan Date: Tue, 23 Jun 2026 02:27:42 +0530 Subject: [PATCH] fix(report): filter empty LLM findings and add SARIF rules[] array Two SARIF output issues fixed: 1. Empty findings (missing rule_id or message) from LLM meta-analyzer failures are now filtered before SARIF serialization. Previously these could produce invalid SARIF results with empty ruleId. 2. SARIF output now includes the tool.driver.rules[] array containing reportingDescriptor entries for each unique rule referenced by results. This brings output closer to SARIF 2.1.0 spec compliance and enables IDE integrations that require rule metadata. Added SarifReportingDescriptor model to sarif_models.py. Fixes #146, #148 --- src/skillspector/nodes/report.py | 28 ++- src/skillspector/sarif_models.py | 15 +- .../test_sarif_rules_and_empty_findings.py | 159 ++++++++++++++++++ 3 files changed, 199 insertions(+), 3 deletions(-) create mode 100644 tests/nodes/test_sarif_rules_and_empty_findings.py diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index da32dac1..07d2015e 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -44,6 +44,7 @@ SarifMessage, SarifPhysicalLocation, SarifRegion, + SarifReportingDescriptor, SarifResult, SarifRun, SarifTool, @@ -133,9 +134,17 @@ def _compute_risk_score( def _build_sarif(findings: list[Finding]) -> dict[str, object]: - """Build SARIF 2.1.0 log from findings.""" + """Build SARIF 2.1.0 log from findings. + + Filters out empty/malformed findings (missing rule_id or message) and + builds the required tool.driver.rules[] array from referenced rule IDs. + """ results: list[SarifResult] = [] + seen_rule_ids: dict[str, str] = {} + for finding in findings: + if not finding.rule_id or not finding.message: + continue start_line = finding.start_line end_line = finding.end_line region = SarifRegion(start_line=start_line, end_line=end_line) @@ -154,12 +163,27 @@ def _build_sarif(findings: list[Finding]) -> dict[str, object]: ], ) ) + if finding.rule_id not in seen_rule_ids: + seen_rule_ids[finding.rule_id] = finding.message + + rules = [ + SarifReportingDescriptor( + id=rule_id, + short_description=SarifMessage(text=description), + ) + for rule_id, description in sorted(seen_rule_ids.items()) + ] + sarif_log = SarifLog( schema_=SARIF_SCHEMA_URI, runs=[ SarifRun( tool=SarifTool( - driver=SarifDriver(name="skillspector", version=skillspector_version) + driver=SarifDriver( + name="skillspector", + version=skillspector_version, + rules=rules if rules else None, + ) ), results=results, ) diff --git a/src/skillspector/sarif_models.py b/src/skillspector/sarif_models.py index 52edccbb..ed90ac41 100644 --- a/src/skillspector/sarif_models.py +++ b/src/skillspector/sarif_models.py @@ -76,11 +76,24 @@ class SarifResult(BaseModel): locations: list[SarifLocation] +class SarifReportingDescriptor(BaseModel): + """Rule metadata (SARIF reportingDescriptor).""" + + model_config = {"populate_by_name": True} + + id: str + short_description: SarifMessage | None = Field(default=None, alias="shortDescription") + default_configuration: dict[str, object] | None = Field( + default=None, alias="defaultConfiguration" + ) + + class SarifDriver(BaseModel): - """Tool driver (required: name; optional: version).""" + """Tool driver (required: name; optional: version, rules).""" name: str version: str | None = None + rules: list[SarifReportingDescriptor] | None = None class SarifTool(BaseModel): diff --git a/tests/nodes/test_sarif_rules_and_empty_findings.py b/tests/nodes/test_sarif_rules_and_empty_findings.py new file mode 100644 index 00000000..018bdbf2 --- /dev/null +++ b/tests/nodes/test_sarif_rules_and_empty_findings.py @@ -0,0 +1,159 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# 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. + +"""Tests for SARIF rules[] array generation and empty finding filtering.""" + +from __future__ import annotations + +import pytest + +from skillspector.models import Finding +from skillspector.nodes.report import _build_sarif + + +def _make_finding(rule_id: str = "PE3", message: str = "Credential Access", **kwargs) -> Finding: + defaults = { + "severity": "HIGH", + "confidence": 0.9, + "file": "tool.py", + "start_line": 1, + "end_line": 1, + "remediation": "Remove credential access", + "tags": ["privilege_escalation"], + "context": "context", + "matched_text": "match", + "category": "privilege_escalation", + "pattern": "PE3", + "finding": "snippet", + "explanation": "explain", + "code_snippet": "code", + "intent": None, + } + defaults.update(kwargs) + return Finding(rule_id=rule_id, message=message, **defaults) + + +class TestEmptyFindingsFiltered: + """Findings with missing rule_id or message are excluded from SARIF output.""" + + def test_empty_rule_id_filtered(self) -> None: + findings = [ + _make_finding(rule_id="", message="Some message"), + _make_finding(rule_id="PE3", message="Credential Access"), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 1 + assert results[0]["ruleId"] == "PE3" + + def test_none_rule_id_filtered(self) -> None: + findings = [ + _make_finding(rule_id=None, message="Some message"), + _make_finding(rule_id="TM1", message="Tool Misuse"), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 1 + assert results[0]["ruleId"] == "TM1" + + def test_empty_message_filtered(self) -> None: + findings = [ + _make_finding(rule_id="PE3", message=""), + _make_finding(rule_id="MP1", message="Memory Poisoning"), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 1 + assert results[0]["ruleId"] == "MP1" + + def test_none_message_filtered(self) -> None: + findings = [ + _make_finding(rule_id="PE3", message=None), + _make_finding(rule_id="MP1", message="Memory Poisoning"), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 1 + + def test_all_empty_produces_zero_results(self) -> None: + findings = [ + _make_finding(rule_id="", message=""), + _make_finding(rule_id=None, message=None), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 0 + + def test_valid_findings_unchanged(self) -> None: + findings = [ + _make_finding(rule_id="PE3", message="Credential Access"), + _make_finding(rule_id="TM1", message="Tool Misuse"), + ] + sarif = _build_sarif(findings) + results = sarif["runs"][0]["results"] + assert len(results) == 2 + + +class TestSarifRulesArray: + """SARIF output includes tool.driver.rules[] with rule descriptors.""" + + def test_rules_present_in_output(self) -> None: + findings = [_make_finding(rule_id="PE3", message="Credential Access")] + sarif = _build_sarif(findings) + driver = sarif["runs"][0]["tool"]["driver"] + assert "rules" in driver + assert len(driver["rules"]) == 1 + + def test_rule_has_id_and_description(self) -> None: + findings = [_make_finding(rule_id="PE3", message="Credential Access")] + sarif = _build_sarif(findings) + rule = sarif["runs"][0]["tool"]["driver"]["rules"][0] + assert rule["id"] == "PE3" + assert rule["shortDescription"]["text"] == "Credential Access" + + def test_multiple_rules_deduplicated(self) -> None: + findings = [ + _make_finding(rule_id="PE3", message="Credential Access"), + _make_finding(rule_id="PE3", message="Credential Access", file="other.py"), + _make_finding(rule_id="TM1", message="Tool Misuse"), + ] + sarif = _build_sarif(findings) + rules = sarif["runs"][0]["tool"]["driver"]["rules"] + assert len(rules) == 2 + rule_ids = {r["id"] for r in rules} + assert rule_ids == {"PE3", "TM1"} + + def test_rules_sorted_by_id(self) -> None: + findings = [ + _make_finding(rule_id="TM1", message="Tool Misuse"), + _make_finding(rule_id="MP1", message="Memory Poisoning"), + _make_finding(rule_id="PE3", message="Credential Access"), + ] + sarif = _build_sarif(findings) + rules = sarif["runs"][0]["tool"]["driver"]["rules"] + ids = [r["id"] for r in rules] + assert ids == ["MP1", "PE3", "TM1"] + + def test_empty_findings_no_rules(self) -> None: + findings = [_make_finding(rule_id="", message="")] + sarif = _build_sarif(findings) + driver = sarif["runs"][0]["tool"]["driver"] + assert "rules" not in driver or driver.get("rules") is None + + def test_sarif_schema_present(self) -> None: + findings = [_make_finding()] + sarif = _build_sarif(findings) + assert "$schema" in sarif + assert sarif["version"] == "2.1.0"