Skip to content

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

Open
fabioemoutinho wants to merge 2 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 2 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.

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

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

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