From 4480ea49b0af38515ec7ecefe1ba2c7255757177 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 29 Jun 2026 11:06:55 -0400 Subject: [PATCH 1/3] fix(report): preserve full finding metadata in SARIF output (#229) Signed-off-by: Rod Boev --- src/skillspector/nodes/report.py | 21 +++++++-- src/skillspector/sarif_models.py | 1 + tests/nodes/test_report.py | 42 ++++++++++++++++- .../test_sarif_rules_and_empty_findings.py | 46 +++++++++++++++++++ 4 files changed, 105 insertions(+), 5 deletions(-) diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 6295e12c..1dc5f3dd 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -99,6 +99,21 @@ def _sanitize_finding(finding: Finding) -> Finding: return replace(finding, **{f: _clean_text(getattr(finding, f)) for f in _SANITIZED_FIELDS}) +def _build_sarif_properties(finding: Finding) -> dict[str, object] | None: + """Project selected finding metadata into a SARIF properties dictionary.""" + finding_dict = finding.to_dict() + metadata: dict[str, object] = { + "category": finding_dict["category"], + "confidence": finding_dict["confidence"], + "remediation": finding_dict["remediation"], + "code_snippet": finding_dict["code_snippet"], + "intent": finding_dict["intent"], + "tags": finding_dict["tags"], + } + cleaned = {key: value for key, value in metadata.items() if value is not None} + return cleaned or None + + def _severity_to_sarif_level(severity: str) -> Literal["error", "warning", "note"]: """Map Finding.severity to SARIF result level.""" return { @@ -209,14 +224,13 @@ def _build_sarif( 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) + region = SarifRegion(start_line=finding.start_line, end_line=finding.end_line) results.append( SarifResult( rule_id=finding.rule_id, message=SarifMessage(text=finding.message), level=_severity_to_sarif_level(finding.severity), + properties=_build_sarif_properties(finding), locations=[ SarifLocation( physical_location=SarifPhysicalLocation( @@ -241,6 +255,7 @@ def _build_sarif( rule_id=finding.rule_id, message=SarifMessage(text=finding.message), level=_severity_to_sarif_level(finding.severity), + properties=_build_sarif_properties(finding), locations=[ SarifLocation( physical_location=SarifPhysicalLocation( diff --git a/src/skillspector/sarif_models.py b/src/skillspector/sarif_models.py index c3256ad8..08a8e51a 100644 --- a/src/skillspector/sarif_models.py +++ b/src/skillspector/sarif_models.py @@ -84,6 +84,7 @@ class SarifResult(BaseModel): # When present, the result is suppressed; SARIF consumers (e.g. GitHub code # scanning) exclude suppressed results from counts but keep them for audit. suppressions: list[SarifSuppression] | None = None + properties: dict[str, object] | None = None class SarifReportingDescriptor(BaseModel): diff --git a/tests/nodes/test_report.py b/tests/nodes/test_report.py index 71445d65..3305d7ef 100644 --- a/tests/nodes/test_report.py +++ b/tests/nodes/test_report.py @@ -506,6 +506,30 @@ def test_report_output_format_sarif(self) -> None: assert "runs" in data assert data.get("$schema") or "runs" in data + def test_report_output_format_sarif_includes_finding_properties(self) -> None: + finding = _finding("E2", "HIGH", "env harvest", confidence=0.85, file="tool.py") + finding.category = "environment" + finding.remediation = "Drop env var usage" + finding.code_snippet = "os.environ['TOKEN']" + finding.intent = "secret_exfiltration" + finding.tags = ["env", "secret"] + state: SkillspectorState = { + "filtered_findings": [finding], + "component_metadata": [], + "has_executable_scripts": False, + "manifest": {}, + "skill_path": None, + "output_format": "sarif", + } + result = report(state) + result_row = result["sarif_report"]["runs"][0]["results"][0] + assert result_row["properties"]["category"] == "environment" + assert result_row["properties"]["confidence"] == 0.85 + assert result_row["properties"]["remediation"] == "Drop env var usage" + assert result_row["properties"]["code_snippet"] == "os.environ['TOKEN']" + assert result_row["properties"]["intent"] == "secret_exfiltration" + assert result_row["properties"]["tags"] == ["env", "secret"] + def test_report_default_output_format_is_sarif(self) -> None: """When output_format is missing, report uses sarif.""" state: SkillspectorState = { @@ -552,8 +576,14 @@ def test_report_dedup_affects_score_only_not_report_output(self) -> None: def test_report_baseline_suppresses_finding_and_lowers_score() -> None: """A baseline-suppressed CRITICAL finding does not count toward the risk score.""" baseline = Baseline(rules=[SuppressionRule(rule_id="P5", reason="false positive")]) + suppressed_finding = _finding("P5", "CRITICAL", confidence=1.0) + suppressed_finding.category = "critical_path" + suppressed_finding.remediation = "Drop suspicious logic" + suppressed_finding.code_snippet = "exec(payload)" + suppressed_finding.intent = "command_execution" + suppressed_finding.tags = ["critical", "injection"] state: SkillspectorState = { - "filtered_findings": [_finding("P5", "CRITICAL")], + "filtered_findings": [suppressed_finding], "component_metadata": [], "has_executable_scripts": False, "manifest": {}, @@ -569,7 +599,15 @@ def test_report_baseline_suppresses_finding_and_lowers_score() -> None: # (audit trail) so consumers exclude them from counts. sarif_results = result["sarif_report"]["runs"][0]["results"] assert len(sarif_results) == 1 - assert sarif_results[0]["suppressions"][0]["kind"] == "external" + suppressed_result = sarif_results[0] + assert suppressed_result["suppressions"][0]["kind"] == "external" + assert suppressed_result["suppressions"][0]["justification"] == "false positive" + assert suppressed_result["properties"]["category"] == "critical_path" + assert suppressed_result["properties"]["confidence"] == 1.0 + assert suppressed_result["properties"]["remediation"] == "Drop suspicious logic" + assert suppressed_result["properties"]["code_snippet"] == "exec(payload)" + assert suppressed_result["properties"]["intent"] == "command_execution" + assert suppressed_result["properties"]["tags"] == ["critical", "injection"] assert len(result["suppressed_findings"]) == 1 diff --git a/tests/nodes/test_sarif_rules_and_empty_findings.py b/tests/nodes/test_sarif_rules_and_empty_findings.py index d4f9f945..df5ce508 100644 --- a/tests/nodes/test_sarif_rules_and_empty_findings.py +++ b/tests/nodes/test_sarif_rules_and_empty_findings.py @@ -19,6 +19,7 @@ from skillspector.models import Finding from skillspector.nodes.report import _build_sarif +from skillspector.suppression import SuppressedFinding def _make_finding(rule_id: str = "PE3", message: str = "Credential Access", **kwargs) -> Finding: @@ -155,3 +156,48 @@ def test_sarif_schema_present(self) -> None: sarif = _build_sarif(findings) assert "$schema" in sarif assert sarif["version"] == "2.1.0" + + +class TestSarifResultProperties: + """SARIF results should preserve selected finding metadata in properties.""" + + def test_active_finding_metadata_in_properties(self) -> None: + finding = _make_finding( + category="network_security", + confidence=0.77, + remediation="Sanitize network credentials", + code_snippet="payload", + intent="exfiltration", + tags=["llm-unconfirmed", "network"], + end_line=10, + ) + sarif = _build_sarif([finding]) + result = sarif["runs"][0]["results"][0] + assert result["properties"]["category"] == "network_security" + assert result["properties"]["confidence"] == 0.77 + assert result["properties"]["remediation"] == "Sanitize network credentials" + assert result["properties"]["code_snippet"] == "payload" + assert result["properties"]["intent"] == "exfiltration" + assert result["properties"]["tags"] == ["llm-unconfirmed", "network"] + region = result["locations"][0]["physicalLocation"]["region"] + assert region["endLine"] == 10 + + def test_suppressed_finding_keeps_properties_and_suppression_marker(self) -> None: + finding = _make_finding( + rule_id="P5", + message="Credential leak", + category="authn_security", + confidence=1.0, + remediation="Rotate keys", + code_snippet="secret", + intent="exposed_secret", + tags=["critical", "auth"], + end_line=20, + ) + sarif = _build_sarif([], [SuppressedFinding(finding=finding, reason="false positive")]) + result = sarif["runs"][0]["results"][0] + assert result["suppressions"][0]["kind"] == "external" + assert result["suppressions"][0]["justification"] == "false positive" + assert result["properties"]["category"] == "authn_security" + assert result["properties"]["confidence"] == 1.0 + assert result["properties"]["intent"] == "exposed_secret" From eb7d221b23757544a6a2b299c073cec20dc9c0b3 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Mon, 29 Jun 2026 11:13:25 -0400 Subject: [PATCH 2/3] fix(report): preserve remaining SARIF finding fields (#229) Signed-off-by: Rod Boev --- src/skillspector/nodes/report.py | 3 +++ tests/nodes/test_report.py | 12 ++++++++++++ tests/nodes/test_sarif_rules_and_empty_findings.py | 12 ++++++++++++ 3 files changed, 27 insertions(+) diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 1dc5f3dd..3c27e9c7 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -104,7 +104,10 @@ def _build_sarif_properties(finding: Finding) -> dict[str, object] | None: finding_dict = finding.to_dict() metadata: dict[str, object] = { "category": finding_dict["category"], + "pattern": finding_dict["pattern"], "confidence": finding_dict["confidence"], + "finding": finding_dict["finding"], + "explanation": finding_dict["explanation"], "remediation": finding_dict["remediation"], "code_snippet": finding_dict["code_snippet"], "intent": finding_dict["intent"], diff --git a/tests/nodes/test_report.py b/tests/nodes/test_report.py index 3305d7ef..066ed332 100644 --- a/tests/nodes/test_report.py +++ b/tests/nodes/test_report.py @@ -509,6 +509,9 @@ def test_report_output_format_sarif(self) -> None: def test_report_output_format_sarif_includes_finding_properties(self) -> None: finding = _finding("E2", "HIGH", "env harvest", confidence=0.85, file="tool.py") finding.category = "environment" + finding.pattern = r"os\.environ" + finding.finding = "TOKEN lookup" + finding.explanation = "Environment-derived secret access" finding.remediation = "Drop env var usage" finding.code_snippet = "os.environ['TOKEN']" finding.intent = "secret_exfiltration" @@ -524,7 +527,10 @@ def test_report_output_format_sarif_includes_finding_properties(self) -> None: result = report(state) result_row = result["sarif_report"]["runs"][0]["results"][0] assert result_row["properties"]["category"] == "environment" + assert result_row["properties"]["pattern"] == r"os\.environ" assert result_row["properties"]["confidence"] == 0.85 + assert result_row["properties"]["finding"] == "TOKEN lookup" + assert result_row["properties"]["explanation"] == "Environment-derived secret access" assert result_row["properties"]["remediation"] == "Drop env var usage" assert result_row["properties"]["code_snippet"] == "os.environ['TOKEN']" assert result_row["properties"]["intent"] == "secret_exfiltration" @@ -578,6 +584,9 @@ def test_report_baseline_suppresses_finding_and_lowers_score() -> None: baseline = Baseline(rules=[SuppressionRule(rule_id="P5", reason="false positive")]) suppressed_finding = _finding("P5", "CRITICAL", confidence=1.0) suppressed_finding.category = "critical_path" + suppressed_finding.pattern = r"exec\(" + suppressed_finding.finding = "exec call" + suppressed_finding.explanation = "Dynamic execution remains reachable" suppressed_finding.remediation = "Drop suspicious logic" suppressed_finding.code_snippet = "exec(payload)" suppressed_finding.intent = "command_execution" @@ -603,7 +612,10 @@ def test_report_baseline_suppresses_finding_and_lowers_score() -> None: assert suppressed_result["suppressions"][0]["kind"] == "external" assert suppressed_result["suppressions"][0]["justification"] == "false positive" assert suppressed_result["properties"]["category"] == "critical_path" + assert suppressed_result["properties"]["pattern"] == r"exec\(" assert suppressed_result["properties"]["confidence"] == 1.0 + assert suppressed_result["properties"]["finding"] == "exec call" + assert suppressed_result["properties"]["explanation"] == "Dynamic execution remains reachable" assert suppressed_result["properties"]["remediation"] == "Drop suspicious logic" assert suppressed_result["properties"]["code_snippet"] == "exec(payload)" assert suppressed_result["properties"]["intent"] == "command_execution" diff --git a/tests/nodes/test_sarif_rules_and_empty_findings.py b/tests/nodes/test_sarif_rules_and_empty_findings.py index df5ce508..abd78d51 100644 --- a/tests/nodes/test_sarif_rules_and_empty_findings.py +++ b/tests/nodes/test_sarif_rules_and_empty_findings.py @@ -164,7 +164,10 @@ class TestSarifResultProperties: def test_active_finding_metadata_in_properties(self) -> None: finding = _make_finding( category="network_security", + pattern=r"socket\.connect", confidence=0.77, + finding="network connect", + explanation="Outbound network path remains open", remediation="Sanitize network credentials", code_snippet="payload", intent="exfiltration", @@ -174,7 +177,10 @@ def test_active_finding_metadata_in_properties(self) -> None: sarif = _build_sarif([finding]) result = sarif["runs"][0]["results"][0] assert result["properties"]["category"] == "network_security" + assert result["properties"]["pattern"] == r"socket\.connect" assert result["properties"]["confidence"] == 0.77 + assert result["properties"]["finding"] == "network connect" + assert result["properties"]["explanation"] == "Outbound network path remains open" assert result["properties"]["remediation"] == "Sanitize network credentials" assert result["properties"]["code_snippet"] == "payload" assert result["properties"]["intent"] == "exfiltration" @@ -187,7 +193,10 @@ def test_suppressed_finding_keeps_properties_and_suppression_marker(self) -> Non rule_id="P5", message="Credential leak", category="authn_security", + pattern=r"api[_-]?key", confidence=1.0, + finding="credential leak", + explanation="Credential material is exposed in output", remediation="Rotate keys", code_snippet="secret", intent="exposed_secret", @@ -199,5 +208,8 @@ def test_suppressed_finding_keeps_properties_and_suppression_marker(self) -> Non assert result["suppressions"][0]["kind"] == "external" assert result["suppressions"][0]["justification"] == "false positive" assert result["properties"]["category"] == "authn_security" + assert result["properties"]["pattern"] == r"api[_-]?key" assert result["properties"]["confidence"] == 1.0 + assert result["properties"]["finding"] == "credential leak" + assert result["properties"]["explanation"] == "Credential material is exposed in output" assert result["properties"]["intent"] == "exposed_secret" From 66113eee3b09f3d1637cb0d815dc0a3997a91aac Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Tue, 30 Jun 2026 06:53:39 -0400 Subject: [PATCH 3/3] fix(report): preserve exact SARIF severity metadata (#229) Signed-off-by: Rod Boev --- src/skillspector/nodes/report.py | 1 + tests/nodes/test_report.py | 23 +++++++++++++++++++ .../test_sarif_rules_and_empty_findings.py | 2 ++ 3 files changed, 26 insertions(+) diff --git a/src/skillspector/nodes/report.py b/src/skillspector/nodes/report.py index 3c27e9c7..a8d5992e 100644 --- a/src/skillspector/nodes/report.py +++ b/src/skillspector/nodes/report.py @@ -103,6 +103,7 @@ def _build_sarif_properties(finding: Finding) -> dict[str, object] | None: """Project selected finding metadata into a SARIF properties dictionary.""" finding_dict = finding.to_dict() metadata: dict[str, object] = { + "severity": finding_dict["severity"], "category": finding_dict["category"], "pattern": finding_dict["pattern"], "confidence": finding_dict["confidence"], diff --git a/tests/nodes/test_report.py b/tests/nodes/test_report.py index 066ed332..3dfed86b 100644 --- a/tests/nodes/test_report.py +++ b/tests/nodes/test_report.py @@ -526,6 +526,7 @@ def test_report_output_format_sarif_includes_finding_properties(self) -> None: } result = report(state) result_row = result["sarif_report"]["runs"][0]["results"][0] + assert result_row["properties"]["severity"] == "HIGH" assert result_row["properties"]["category"] == "environment" assert result_row["properties"]["pattern"] == r"os\.environ" assert result_row["properties"]["confidence"] == 0.85 @@ -611,6 +612,7 @@ def test_report_baseline_suppresses_finding_and_lowers_score() -> None: suppressed_result = sarif_results[0] assert suppressed_result["suppressions"][0]["kind"] == "external" assert suppressed_result["suppressions"][0]["justification"] == "false positive" + assert suppressed_result["properties"]["severity"] == "CRITICAL" assert suppressed_result["properties"]["category"] == "critical_path" assert suppressed_result["properties"]["pattern"] == r"exec\(" assert suppressed_result["properties"]["confidence"] == 1.0 @@ -743,3 +745,24 @@ def test_report_doc_findings_no_multiplier() -> None: # Without the multiplier: 2 HIGH = 50, not 65 assert result["risk_score"] == 50 assert result["risk_severity"] == "MEDIUM" + + +def test_report_sarif_preserves_high_vs_critical_severity() -> None: + """HIGH and CRITICAL both map to SARIF error, but properties keep the exact severity.""" + state: SkillspectorState = { + "filtered_findings": [ + _finding("R1", "HIGH", message="high finding", file="high.py"), + _finding("R2", "CRITICAL", message="critical finding", file="critical.py"), + ], + "component_metadata": [], + "has_executable_scripts": False, + "manifest": {}, + "skill_path": None, + "output_format": "sarif", + } + results = report(state)["sarif_report"]["runs"][0]["results"] + by_rule = {item["ruleId"]: item for item in results} + assert by_rule["R1"]["level"] == "error" + assert by_rule["R2"]["level"] == "error" + assert by_rule["R1"]["properties"]["severity"] == "HIGH" + assert by_rule["R2"]["properties"]["severity"] == "CRITICAL" diff --git a/tests/nodes/test_sarif_rules_and_empty_findings.py b/tests/nodes/test_sarif_rules_and_empty_findings.py index abd78d51..02a47587 100644 --- a/tests/nodes/test_sarif_rules_and_empty_findings.py +++ b/tests/nodes/test_sarif_rules_and_empty_findings.py @@ -176,6 +176,7 @@ def test_active_finding_metadata_in_properties(self) -> None: ) sarif = _build_sarif([finding]) result = sarif["runs"][0]["results"][0] + assert result["properties"]["severity"] == "HIGH" assert result["properties"]["category"] == "network_security" assert result["properties"]["pattern"] == r"socket\.connect" assert result["properties"]["confidence"] == 0.77 @@ -207,6 +208,7 @@ def test_suppressed_finding_keeps_properties_and_suppression_marker(self) -> Non result = sarif["runs"][0]["results"][0] assert result["suppressions"][0]["kind"] == "external" assert result["suppressions"][0]["justification"] == "false positive" + assert result["properties"]["severity"] == "HIGH" assert result["properties"]["category"] == "authn_security" assert result["properties"]["pattern"] == r"api[_-]?key" assert result["properties"]["confidence"] == 1.0