feat: centralize finding display to prevent unknown severity dropping(closes #385)#487
Conversation
|
@sonukapoor please review the PR |
sonukapoor
left a comment
There was a problem hiding this comment.
This is exactly what we needed - the debug flag work is gone and the finding display fix stands on its own cleanly.
The logic is correct throughout. selectFindingsForTable is a straight extraction of the existing filter, selectFindingsForCompact correctly separates the urgent-N cap from the unknown bucket so malicious findings can never be silently dropped by a full critical/high slice, and mergeUniqueFindings handles deduplication cleanly. The test covers the real-world scenario (MAL-2025-21003 with three urgent slots already full) and checks the warning message too.
One note on the direct-only filter for unknowns in compact mode: the old code showed all unknowns regardless of relationship but could drop them when 3+ critical/high existed. The new code shows all direct unknowns unconditionally, but transitive unknowns won't appear in the compact preview. That's the right trade-off - MAL-* advisories are almost always direct installs, and transitive unknowns are still visible in verbose mode and under --all. Just want to confirm you considered that and it was intentional.
Merging this.
closes #385
Problem
Unknown-severity findings were counted in summaries but could be silently hidden in terminal output:
Compact mode could drop unknown findings due to slice(0, 3) selection behavior
Verbose mode’s default table threshold could exclude unknown findings
HTML/JSON/SARIF already include all findings correctly.
Fix
Centralizes finding selection so output modes stay consistent and unknown severity findings are not silently dropped.
Changes
src/output/finding-display.ts: shared selectors
selectFindingsForTable() includes unknown findings even when severity thresholds apply
selectFindingsForCompact() ensures unknown findings aren’t lost due to top‑N slicing
src/index.ts: uses shared selector for verbose table
src/output/printers.ts: uses shared selector for compact output
tests/output.test.ts: regression coverage for compact unknown visibility
Notes
This PR is intentionally scoped to finding display logic only (no --debug changes).