Skip to content

feat: add Root column showing the top-level dependency behind each finding - #920

Open
fabioemoutinho wants to merge 3 commits into
OWASP:mainfrom
fabioemoutinho:feat/root-dependency-column
Open

feat: add Root column showing the top-level dependency behind each finding#920
fabioemoutinho wants to merge 3 commits into
OWASP:mainfrom
fabioemoutinho:feat/root-dependency-column

Conversation

@fabioemoutinho

@fabioemoutinho fabioemoutinho commented Aug 1, 2026

Copy link
Copy Markdown

Closes #923

Summary

  • Vulnerable transitive dependencies gave no indication of which top-level (direct) dependency pulled them in.
  • Adds a Root column across all report surfaces:
    • Terminal table: compact name +N form (e.g. express +2) when multiple direct dependencies pull in the same vulnerable transitive package.
    • JSON output (--json / report data): full rootDependencies: string[] array on each finding.
    • HTML report: lists every root dependency name in full (not truncated), since there's more horizontal room.
  • Direct dependencies and findings with no path data show - (there's no meaningful "root" distinct from the package itself).
  • New getRootDependencies(finding) helper in src/utils/finding.ts, mirroring the existing getPrimaryParent convention (same length-based filtering of degenerate direct-dependency paths), and gated on relationship === "transitive" so a package classified direct never shows a contradictory root even if it's also reachable via another root's transitive chain.
  • Renamed the HTML detail view's Parent: label to Root: so it matches the new column instead of using two terms for the same concept.

Test plan

  • getRootDependencies unit tests: single root, multiple unique roots across several paths, degenerate direct-dependency path, empty paths, mixed direct+transitive-path edge case.
  • formatRootDependencySummary unit tests: single name, name +N for multiple roots, - fallback, mixed direct+transitive-path edge case.
  • printTable tests: Root column header present, name +N compact form, - for direct dependencies.
  • HTML report tests: Root header present, full comma-separated list of root names, - fallback for direct dependencies, Root: detail label.
  • serializeFinding test updated to assert rootDependencies is included.
  • Full existing test suite passes with no regressions.

@sonukapoor sonukapoor 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.

Fabio, this is a genuinely useful addition - showing the top-level dependency behind a transitive finding is exactly the kind of actionable context we like to give people, and I love that you covered all three surfaces (terminal, JSON, HTML) with tests to match. I pulled it down and tried it locally and it reads really well.

Just a couple of small things and then I think it's good to go, both left as inline notes. The main one is an edge case where a direct dependency can end up showing a transitive root; the other is a tiny naming consistency nit. Neither is a big lift.

One admin thing whenever you get a chance: could you add Closes #923 to the PR description? I opened that issue to track the feature. Thanks so much for this - really nice work.

Comment thread src/utils/finding.ts
Comment thread src/output/html-reporter.ts
fabioemoutinho added a commit to fabioemoutinho/cve-lite-cli that referenced this pull request Aug 1, 2026
…t labeling

getRootDependencies unioned roots across every dependencyPaths entry
without checking finding.relationship, so a package classified "direct"
(but also reachable via another root's transitive chain, deduped into
the same PackageRef) could still surface a contradictory root. Gate it
on relationship === "transitive", matching the convention already used
by parent-upgrade.ts, transitive-chain-resolver.ts, and
npm-transitive-resolution.ts.

Also rename the HTML detail view's "Parent:" label to "Root:" so it
matches the new column header instead of using two terms for the same
concept.

Addresses review feedback on OWASP#920.
@sonukapoor

Copy link
Copy Markdown
Collaborator

Reran the repro locally and it's sorted - the contradictory row is gone, direct findings show a dash, and transitive ones still resolve their root.

One more you might want to grab while you're in here, not a blocker: the expanded HTML detail row still shows a single root (it reads from primaryParent, so just the first path), while the new column can show name +N across all paths. So a finding pulled in by a few roots shows one name in the detail panel but a count in the column. If you feel like lining those up - detail row listing all roots, or a "+N more" - great. If not, it's a fine follow-up.

Thanks for the quick turnaround.

@sonukapoor

Copy link
Copy Markdown
Collaborator

One more before this lands: renderFindingRow is shared with the multi-folder HTML report (multi-folder-html-reporter.ts), which keeps its own 6-column table header. Once this PR adds the Root <td>, that table ends up with 7 cells under 6 headers and the columns shift. Could you add the Root <th> there too and bump that folder table's colspans (the empty row and the no-results row are colspan="6")? That keeps the multi-folder report aligned and gives it the Root column for parity.

Heads-up that PR #901 also touches that file, so this is cleanest to do once #901 lands - I'll ping you when it does.

…nding

Vulnerable transitive dependencies gave no indication of which direct
dependency pulled them in. The terminal table and JSON output now show
a compact "name +N" summary, while the HTML report lists every root
dependency in full.
…t labeling

getRootDependencies unioned roots across every dependencyPaths entry
without checking finding.relationship, so a package classified "direct"
(but also reachable via another root's transitive chain, deduped into
the same PackageRef) could still surface a contradictory root. Gate it
on relationship === "transitive", matching the convention already used
by parent-upgrade.ts, transitive-chain-resolver.ts, and
npm-transitive-resolution.ts.

Also rename the HTML detail view's "Parent:" label to "Root:" so it
matches the new column header instead of using two terms for the same
concept.

Addresses review feedback on OWASP#920.
… roots

renderFindingRow is shared between the single-folder and multi-folder
HTML reports. The multi-folder report kept its own 6-column table
header and colspans, so the new Root <td> shifted its columns out of
alignment. Add the matching <th>Root</th> and bump both colspans to 7.

Also update the expanded detail row's Context column to list every
root dependency (finding.rootDependencies) instead of only the first
path's root (finding.primaryParent), so it no longer disagrees with
the "name +N" summary shown in the column when multiple roots pull in
the same finding.

Addresses further review feedback on OWASP#920.
@fabioemoutinho
fabioemoutinho force-pushed the feat/root-dependency-column branch from 56173d9 to 449354a Compare August 12, 2026 00:56
@fabioemoutinho

Copy link
Copy Markdown
Author

Addressed both follow-ups in 449354a (rebased onto latest main to pick up #901 first):

  1. Detail row vs. column alignment: the expanded Context column now renders finding.rootDependencies.join(", ") instead of finding.primaryParent, so it lists every root pulling in the finding — no more "one name in the panel, +N in the column" mismatch.
  2. Multi-folder table: added the matching <th>Root</th> to multi-folder-html-reporter.ts and bumped both colspan="6" occurrences (empty-row and no-results-row) to 7, so cell count matches the header again. Added a regression test asserting the header, the rendered root name, and that no stale colspan="6" remains.

Ran the full suite afterward — no regressions (same pre-existing environment-only failures as on main, unrelated to this PR).

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: show the top-level (root) dependency behind each transitive finding

2 participants