Skip to content

fix(report): preserve full finding metadata in SARIF output (#229)#230

Open
rodboev wants to merge 3 commits into
NVIDIA:mainfrom
rodboev:pr/sarif-full-finding-metadata-229
Open

fix(report): preserve full finding metadata in SARIF output (#229)#230
rodboev wants to merge 3 commits into
NVIDIA:mainfrom
rodboev:pr/sarif-full-finding-metadata-229

Conversation

@rodboev

@rodboev rodboev commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

--format sarif currently drops finding metadata that --format json already preserves. This update keeps the original finding severity in the SARIF result properties bag too, so downstream tooling can still distinguish CRITICAL from HIGH after SARIF level flattening.

Closes #229

Root cause

_build_sarif() builds each result from only the core SARIF fields and a partial properties bag. Even after the earlier metadata work, SARIF level still flattened both CRITICAL and HIGH to error, and the original severity value was never preserved alongside the other finding fields.

Diff Notes

  • Extend SarifResult so the report layer preserves the original severity alongside the existing SkillSpector-specific finding metadata in the SARIF-compatible properties bag.
  • Keep endLine, rule generation, sorting, empty-finding filtering, and suppression markers unchanged.
  • Add focused regressions for active and suppressed metadata preservation, plus a HIGH-vs-CRITICAL check that still expects level=error for both results.

Scope

This change is limited to report and SARIF schema code. It does not change analyzers, JSON output, CLI behavior, transitive scanning, baseline matching, or provider and MCP behavior.

Verification

  • .\.venv\Scripts\python.exe -m pytest tests/nodes/test_report.py tests/nodes/test_sarif_rules_and_empty_findings.py (63 passed, 1 warning)
  • uv run ruff check src/skillspector/nodes/report.py tests/nodes/test_report.py tests/nodes/test_sarif_rules_and_empty_findings.py
  • uv run ruff format --check src/skillspector/nodes/report.py tests/nodes/test_report.py tests/nodes/test_sarif_rules_and_empty_findings.py

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes because exact finding severity is still lost. SARIF level maps both CRITICAL and HIGH to error, so the new property bag does not yet preserve full finding metadata. Include the original severity for active and suppressed results and add a distinction test.

Comment thread src/skillspector/nodes/report.py
rodboev added 3 commits June 30, 2026 06:53
Signed-off-by: Rod Boev <rod.boev@gmail.com>
Signed-off-by: Rod Boev <rod.boev@gmail.com>
@rodboev rodboev force-pushed the pr/sarif-full-finding-metadata-229 branch from 80e6c6b to 66113ee Compare June 30, 2026 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] --format sarif is a lossy subset of --format json (drops category, confidence, remediation, code_snippet, intent, tags, end_line)

2 participants