feat: include DM001 findings in SARIF output#888
Conversation
ead6012 to
598ccae
Compare
sonukapoor
left a comment
There was a problem hiding this comment.
Thanks @rksharma-owg - #881 is a real gap (SARIF was the one output surface still missing the DM001 findings), and threading maintenanceFindings through to buildSarifOutput mirrors the existing JSON path correctly.
A few things before merge:
-
DM001 ends up registered twice in the SARIF: once in
tool.driver.rules(via the ruleMap injection insarif.ts) and again as an orphaned copy intool.extensions[]with diverging metadata, andbuildMaintenanceSarifComponent()gets called twice. The existingoverride-findings-sarif.tsuses the extension-component pattern deliberately (there is a comment explaining why Code Scanning needs it) - could you pick one registration path and match that precedent so DM001 is defined once? -
There is no test with both override and maintenance findings present at once - that is the one case where the extensions-array ordering matters (override hardcodes
toolComponent.index: 0). Worth adding. -
Same override commit as your other PRs - please drop the
overrides: brace-expansion+ jest bump (already handled via the baseline in #897) and rebase.
The core fix is sound; these are about matching the existing SARIF pattern and dropping the unrelated bits.
598ccae to
b730754
Compare
|
Thanks for the review. I rebased onto current Validation:
The local full build still encounters the unrelated |
Problem\n
audit\n\n- Fix issue #881: maintenance findings from --check-maintenance were omitted from SARIF output, so Code Scanning could miss DM001 risk signals.\n\n## Root cause\n- write-outputs only passed vulnerabilities and override findings into writeSarifReport.\n- buildSarifOutput only converted vulnerability and override findings.\n\n## Fix\n- Added SARIF support for maintenance findings using DM001 rules.\n- Wired maintenanceFindings to writeSarifReport and writeOutputs.\n- Added unit tests for maintenance SARIF transformation and write-output call wiring.\n\n## Validation\n- Targeted unit tests are added in
tests/sarif.test.ts,tests/output/maintenance-findings-sarif.test.ts, andtests/write-outputs.test.ts.\n- Full test run is currently blocked by pre-existing repo typing issue insrc/utils/network.tsduring build (Property 'preconnect' is missing in type ...), unrelated to this change.