fix: resolve SBOM dependency edges from the lock graph - #1106
Merged
Conversation
…d paths The SBOM dependency graph reconstructed parent edges by prefix-matching PackageRef.paths, which upsertPackage caps at five per package. On a large tree that discards a lot: measured on examples/lit, 592 of 2549 packages exceed five paths, with semver@6.3.1 alone having 49. When the five surviving routes all traverse packages excluded from the scan, the prefix lookup finds nothing and the package ends up with no parent edge. The lock graph already answers this directly. parentsFor() is a public, pre-frozen, complete child-to-parents map that the display cap never touches, so no second traversal and no raised cap are needed. The five-path bound stays where it earns its place, in remediation output, where a representative route is enough. Two cases needed handling beyond the straight lookup. Workspace members are symlinked rather than installed, so the graph records them without a version and they never appear in the scanned package set; their dependencies were being dropped entirely. And --prod-only shrinks the package set without shrinking the graph, so an edge can name a package absent from the document. Both now anchor to the root project rather than orphaning the package, because it is genuinely in the tree and the root is the only anchor available. A dangling reference is never emitted. Measured on examples/lit: DEPENDENCY_OF edges 2664 -> 4082, packages with no parent 111 -> 1, and that one is the root project, which correctly has none. Zero dangling references. pnpm, Yarn and Bun have no lock graph, so the resolver returns an empty list and the path-derived behaviour is kept. That fallback is selected by length rather than truthiness, since an empty array is truthy and checking the reference alone silently emptied the graph for every non-npm ecosystem. Verified against examples/astro: 3513 edges retained. Closes #1079
The guide documented the five-path cap as a limitation affecting all projects, measured at roughly 5 percent of packages left without a parent edge on a large monorepo. That is no longer true for npm, where edges now come from the resolved lockfile graph. Split the guidance by package manager rather than deleting it: npm is complete, and pnpm, Yarn and Bun still derive edges from capped paths and retain the limitation. Says so plainly instead of implying the graph is exhaustive everywhere.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The SBOM dependency graph reconstructed parent edges by prefix-matching
PackageRef.paths, whichupsertPackagecaps at five per package. On a large tree that discards a lot. Measured onexamples/lit, 592 of 2549 packages exceed five paths, withsemver@6.3.1alone having 49 andglob@7.2.3having 33. When the five surviving routes all traverse packages excluded from the scan, the prefix lookup finds nothing and the package ends up with no parent edge.The lock graph already had the answer
parentsFor()is a public, pre-frozen, complete child-to-parents map on the npm lock graph, and the display cap never touches it. So no second traversal and no raised cap were needed, which removes the performance question that made this look expensive on the issue. The five-path bound stays where it earns its place, in remediation output, where a representative route is enough.Two cases beyond the straight lookup
Workspace members are symlinked rather than installed, so the graph records them without a version and they never appear in the scanned package set. Their dependencies were being dropped entirely, which is what most of the orphans on
litactually were.--prod-onlyshrinks the package set without shrinking the graph, so an edge can name a package absent from the document.Both now anchor to the root project rather than orphaning the package. It is genuinely in the tree, and the root is the only anchor the document can offer. Emitting a reference to a package that is not in the document would be a dangling SPDX reference, which never happens.
Measured on examples/lit
The single remaining orphan is
@lit-internal/monorepo, the root project, which correctly has no parent.Non-npm ecosystems
pnpm, Yarn and Bun have no lock graph, so the resolver returns an empty list and path-derived behaviour is kept. That fallback is selected by length rather than truthiness: an empty array is truthy, and checking the reference alone silently emptied the graph for every non-npm ecosystem. Caught during verification and pinned with a test. Verified on
examples/astro: 3513 edges retained.Verification
1790 tests, build clean, docs site builds. Both new code paths mutation-tested: disabling
buildEdgesFromResolvedfails 2 tests, removing the root-anchoring fallback fails 3.The SPDX guide previously documented this as a limitation for all projects. It now splits by package manager, since npm is complete and the others are not, rather than implying the graph is exhaustive everywhere.
Closes #1079