Skip to content

feat: centralize finding display to prevent unknown severity dropping(closes #385)#487

Merged
sonukapoor merged 2 commits into
OWASP:mainfrom
coder-Yash886:feat/centralize-finding-display-385-clean
May 28, 2026
Merged

feat: centralize finding display to prevent unknown severity dropping(closes #385)#487
sonukapoor merged 2 commits into
OWASP:mainfrom
coder-Yash886:feat/centralize-finding-display-385-clean

Conversation

@coder-Yash886
Copy link
Copy Markdown
Contributor

@coder-Yash886 coder-Yash886 commented May 28, 2026

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).

@coder-Yash886
Copy link
Copy Markdown
Contributor Author

@sonukapoor please review the PR

Copy link
Copy Markdown
Collaborator

@sonukapoor sonukapoor left a comment

Choose a reason for hiding this comment

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

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.

@sonukapoor sonukapoor merged commit 50e7654 into OWASP:main May 28, 2026
6 checks passed
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.

feat: centralize finding display across compact, verbose, report, JSON, and SARIF outputs

2 participants