From d8d794b48a7b0a390a9407ad2e8e551a11e5899b Mon Sep 17 00:00:00 2001 From: Michael Kedar Date: Thu, 30 Jan 2025 14:09:07 +1100 Subject: [PATCH] fix(guided remediation): reduce memory footprint by computing dependency subgraphs instead of chains (#1538) Guided remediation had been using `DependencyChains` to track paths to a vulnerable package (for computing things like depth and which direct dependencies to relax). It was computing *every* possible path in the graph to a dependency, which grows roughly exponentially with depth / connectivity. This was using an unreasonable amount of memory on some particularly large/complex projects. I've changed the logic to instead compute one `DependencySubgraph` - the set of nodes and edges that would contain every path to a dependency. This should significantly reduce memory usage (and cpu usage from allocs) when running on larger projects. This change has touched quite a few places in the code, and the logic is a bit complex. I've tried my best to check that everything still behaves as expected. --- cmd/osv-scanner/fix/noninteractive.go | 4 +- internal/remediation/in_place.go | 21 +- internal/remediation/in_place_test.go | 7 +- internal/remediation/override.go | 39 +- internal/remediation/relax.go | 19 +- internal/remediation/remediation.go | 14 +- internal/remediation/remediation_test.go | 55 +- internal/remediation/testhelpers_test.go | 7 +- .../__snapshots__/resolve_test.snap | 1594 +++++++++++++---- internal/resolution/dependency_chain.go | 137 -- internal/resolution/dependency_subgraph.go | 246 +++ .../resolution/dependency_subgraph_test.go | 335 ++++ internal/resolution/resolve.go | 40 +- internal/resolution/resolve_test.go | 20 +- internal/tui/dependency-graph.go | 117 +- internal/tui/vuln-info.go | 4 +- 16 files changed, 2008 insertions(+), 651 deletions(-) delete mode 100644 internal/resolution/dependency_chain.go create mode 100644 internal/resolution/dependency_subgraph.go create mode 100644 internal/resolution/dependency_subgraph_test.go diff --git a/cmd/osv-scanner/fix/noninteractive.go b/cmd/osv-scanner/fix/noninteractive.go index 9e08c2b806d..5ca0367606d 100644 --- a/cmd/osv-scanner/fix/noninteractive.go +++ b/cmd/osv-scanner/fix/noninteractive.go @@ -418,8 +418,8 @@ func makeResultVuln(vuln resolution.Vulnerability) vulnOutput { } affected := make(map[packageOutput]struct{}) - for _, c := range append(vuln.ProblemChains, vuln.NonProblemChains...) { - vk, _ := c.End() + for _, sg := range vuln.Subgraphs { + vk := sg.Nodes[sg.Dependency].Version affected[packageOutput{Name: vk.Name, Version: vk.Version}] = struct{}{} } v.Packages = maps.Keys(affected) diff --git a/internal/remediation/in_place.go b/internal/remediation/in_place.go index f520eb5d271..147994d592c 100644 --- a/internal/remediation/in_place.go +++ b/internal/remediation/in_place.go @@ -117,9 +117,10 @@ func ComputeInPlacePatches(ctx context.Context, cl client.ResolutionClient, grap for vk, vulns := range res.vkVulns { reqVers := make(map[string]struct{}) for _, vuln := range vulns { - for _, c := range vuln.ProblemChains { - _, req := c.End() - reqVers[req] = struct{}{} + for _, sg := range vuln.Subgraphs { + for _, e := range sg.Nodes[sg.Dependency].Parents { + reqVers[e.Requirement] = struct{}{} + } } } set, err := buildConstraintSet(vk.Semver(), maps.Keys(reqVers)) @@ -268,24 +269,22 @@ func inPlaceVulnsNodes(ctx context.Context, m clientinterfaces.VulnerabilityMatc nodeIDs = append(nodeIDs, resolve.NodeID(nID)) } } - nodeChains := resolution.ComputeChains(graph, nodeIDs) - // Computing ALL chains might be overkill... - // We only actually care about the shortest chain, the unique dependents of the vulnerable node, and maybe the unique direct dependencies. + nodeSubgraphs := resolution.ComputeSubgraphs(graph, nodeIDs) for i, nID := range nodeIDs { - chains := nodeChains[i] vk := graph.Nodes[nID].Version result.vkNodes[vk] = append(result.vkNodes[vk], nID) for _, vuln := range nodeVulns[nID] { resVuln := resolution.Vulnerability{ - OSV: *vuln, - ProblemChains: slices.Clone(chains), - DevOnly: !slices.ContainsFunc(chains, func(dc resolution.DependencyChain) bool { return !resolution.ChainIsDev(dc, nil) }), + OSV: *vuln, + Subgraphs: []*resolution.DependencySubgraph{nodeSubgraphs[i]}, + DevOnly: nodeSubgraphs[i].IsDevOnly(nil), } idx := slices.IndexFunc(result.vkVulns[vk], func(rv resolution.Vulnerability) bool { return rv.OSV.ID == resVuln.OSV.ID }) if idx >= 0 { - result.vkVulns[vk][idx].ProblemChains = append(result.vkVulns[vk][idx].ProblemChains, resVuln.ProblemChains...) result.vkVulns[vk][idx].DevOnly = result.vkVulns[vk][idx].DevOnly && resVuln.DevOnly + + result.vkVulns[vk][idx].Subgraphs = append(result.vkVulns[vk][idx].Subgraphs, resVuln.Subgraphs...) } else { result.vkVulns[vk] = append(result.vkVulns[vk], resVuln) } diff --git a/internal/remediation/in_place_test.go b/internal/remediation/in_place_test.go index 2bf4e0a897f..cb8379a7be7 100644 --- a/internal/remediation/in_place_test.go +++ b/internal/remediation/in_place_test.go @@ -53,11 +53,8 @@ func checkInPlaceResults(t *testing.T, res remediation.InPlaceResult) { toMinimalVuln := func(v resolution.Vulnerability) minimalVuln { t.Helper() nodes := make(map[resolve.NodeID]struct{}) - for _, c := range v.ProblemChains { - nodes[c.Edges[0].To] = struct{}{} - } - for _, c := range v.NonProblemChains { - nodes[c.Edges[0].To] = struct{}{} + for _, sg := range v.Subgraphs { + nodes[sg.Dependency] = struct{}{} } sortedNodes := maps.Keys(nodes) slices.Sort(sortedNodes) diff --git a/internal/remediation/override.go b/internal/remediation/override.go index b24613e3657..41c7931bad9 100644 --- a/internal/remediation/override.go +++ b/internal/remediation/override.go @@ -131,31 +131,20 @@ func overridePatchVulns(ctx context.Context, cl client.ResolutionClient, result // Keep track of VersionKeys we've seen for this vuln to avoid duplicates. // Usually, there will only be one VersionKey per vuln, but some vulns affect multiple packages. seenVKs := make(map[resolve.VersionKey]struct{}) - // Use the DependencyChains to find all the affected nodes. - for _, c := range v.ProblemChains { - // Currently, there is no way to know if a specific classifier or type exists for a given version with deps.dev. - // Blindly updating versions can lead to compilation failures if the artifact+version+classifier+type doesn't exist. - // We can't reliably attempt remediation in these cases, so don't try. - // TODO: query Maven registry for existence of classifiers in getVersionsGreater - typ := c.Edges[0].Type - if typ.HasAttr(dep.MavenClassifier) || typ.HasAttr(dep.MavenArtifactType) { - return nil, nil, fmt.Errorf("%w: cannot fix vulns in artifacts with classifier or type", errOverrideImpossible) - } - vk, _ := c.End() - if _, seen := seenVKs[vk]; !seen { - vkVulns[vk] = append(vkVulns[vk], &result.Vulns[i]) - seenVKs[vk] = struct{}{} - } - } - for _, c := range v.NonProblemChains { - typ := c.Edges[0].Type - if typ.HasAttr(dep.MavenClassifier) || typ.HasAttr(dep.MavenArtifactType) { - return nil, nil, fmt.Errorf("%w: cannot fix vulns in artifacts with classifier or type", errOverrideImpossible) - } - vk, _ := c.End() - if _, seen := seenVKs[vk]; !seen { - vkVulns[vk] = append(vkVulns[vk], &result.Vulns[i]) - seenVKs[vk] = struct{}{} + // Use the Subgraphs to find all the affected nodes. + for _, sg := range v.Subgraphs { + for _, e := range sg.Nodes[sg.Dependency].Parents { + // Currently, there is no way to know if a specific classifier or type exists for a given version with deps.dev. + // Blindly updating versions can lead to compilation failures if the artifact+version+classifier+type doesn't exist. + // We can't reliably attempt remediation in these cases, so don't try. + if e.Type.HasAttr(dep.MavenClassifier) || e.Type.HasAttr(dep.MavenArtifactType) { + return nil, nil, fmt.Errorf("%w: cannot fix vulns in artifacts with classifier or type", errOverrideImpossible) + } + vk := sg.Nodes[sg.Dependency].Version + if _, seen := seenVKs[vk]; !seen { + vkVulns[vk] = append(vkVulns[vk], &result.Vulns[i]) + seenVKs[vk] = struct{}{} + } } } } diff --git a/internal/remediation/relax.go b/internal/remediation/relax.go index 99bbba80ef8..e88a7609438 100644 --- a/internal/remediation/relax.go +++ b/internal/remediation/relax.go @@ -87,7 +87,7 @@ func tryRelaxRemediate( } newRes := orig - toRelax := reqsToRelax(newRes, vulnIDs, opts) + toRelax := reqsToRelax(ctx, cl, newRes, vulnIDs, opts) for len(toRelax) > 0 { // Try relaxing all necessary requirements manif := newRes.Manifest.Clone() @@ -109,24 +109,27 @@ func tryRelaxRemediate( if err != nil { return nil, err } - toRelax = reqsToRelax(newRes, vulnIDs, opts) + toRelax = reqsToRelax(ctx, cl, newRes, vulnIDs, opts) } return newRes, nil } -func reqsToRelax(res *resolution.Result, vulnIDs []string, opts Options) []int { +func reqsToRelax(ctx context.Context, cl resolve.Client, res *resolution.Result, vulnIDs []string, opts Options) []int { toRelax := make(map[resolve.VersionKey]string) for _, v := range res.Vulns { // Don't do a full opts.MatchVuln() since we know we don't need to check every condition if !slices.Contains(vulnIDs, v.OSV.ID) || (!opts.DevDeps && v.DevOnly) { continue } - // Only relax dependencies if their chain length is less than MaxDepth - for _, ch := range v.ProblemChains { - if opts.MaxDepth <= 0 || len(ch.Edges) <= opts.MaxDepth { - vk, req := ch.Direct() - toRelax[vk] = req + // Only relax dependencies if their distance is less than MaxDepth + for _, sg := range v.Subgraphs { + constr := sg.ConstrainingSubgraph(ctx, cl, &v.OSV) + for _, edge := range constr.Nodes[0].Children { + gNode := constr.Nodes[edge.To] + if opts.MaxDepth <= 0 || gNode.Distance+1 <= opts.MaxDepth { + toRelax[gNode.Version] = edge.Requirement + } } } } diff --git a/internal/remediation/remediation.go b/internal/remediation/remediation.go index 810fb1e8f7e..6244f40d63d 100644 --- a/internal/remediation/remediation.go +++ b/internal/remediation/remediation.go @@ -102,18 +102,8 @@ func (opts Options) matchDepth(v resolution.Vulnerability) bool { return true } - if len(v.ProblemChains)+len(v.NonProblemChains) == 0 { - panic("vulnerability with no dependency chains") - } - - for _, ch := range v.ProblemChains { - if len(ch.Edges) <= opts.MaxDepth { - return true - } - } - - for _, ch := range v.NonProblemChains { - if len(ch.Edges) <= opts.MaxDepth { + for _, sg := range v.Subgraphs { + if sg.Nodes[0].Distance <= opts.MaxDepth { return true } } diff --git a/internal/remediation/remediation_test.go b/internal/remediation/remediation_test.go index 2116a1c9349..b435554ee06 100644 --- a/internal/remediation/remediation_test.go +++ b/internal/remediation/remediation_test.go @@ -23,8 +23,30 @@ func TestMatchVuln(t *testing.T) { Aliases: []string{"CVE-111", "OSV-2"}, }, DevOnly: false, - ProblemChains: []resolution.DependencyChain{{ - Edges: []resolve.Edge{{From: 2, To: 3}, {From: 1, To: 2}, {From: 0, To: 1}}, + Subgraphs: []*resolution.DependencySubgraph{{ + Dependency: 3, + Nodes: map[resolve.NodeID]resolution.GraphNode{ + 3: { + Distance: 0, + Parents: []resolve.Edge{{From: 2, To: 3}}, + Children: []resolve.Edge{}, + }, + 2: { + Distance: 1, + Parents: []resolve.Edge{{From: 1, To: 2}}, + Children: []resolve.Edge{{From: 2, To: 3}}, + }, + 1: { + Distance: 2, + Parents: []resolve.Edge{{From: 0, To: 1}}, + Children: []resolve.Edge{{From: 1, To: 2}}, + }, + 0: { + Distance: 3, + Parents: []resolve.Edge{}, + Children: []resolve.Edge{{From: 0, To: 1}}, + }, + }, }}, } // ID: VULN-002, Dev: true, Severity: N/A, Depth: 2 @@ -34,11 +56,30 @@ func TestMatchVuln(t *testing.T) { // No severity }, DevOnly: true, - ProblemChains: []resolution.DependencyChain{{ - Edges: []resolve.Edge{{From: 2, To: 3}, {From: 1, To: 2}, {From: 0, To: 1}}, - }}, - NonProblemChains: []resolution.DependencyChain{{ - Edges: []resolve.Edge{{From: 1, To: 3}, {From: 0, To: 1}}, + Subgraphs: []*resolution.DependencySubgraph{{ + Dependency: 3, + Nodes: map[resolve.NodeID]resolution.GraphNode{ + 3: { + Distance: 0, + Parents: []resolve.Edge{{From: 2, To: 3}, {From: 1, To: 3}}, + Children: []resolve.Edge{}, + }, + 2: { + Distance: 1, + Parents: []resolve.Edge{{From: 1, To: 2}}, + Children: []resolve.Edge{{From: 2, To: 3}}, + }, + 1: { + Distance: 1, + Parents: []resolve.Edge{{From: 0, To: 1}}, + Children: []resolve.Edge{{From: 1, To: 2}, {From: 1, To: 3}}, + }, + 0: { + Distance: 2, + Parents: []resolve.Edge{}, + Children: []resolve.Edge{{From: 0, To: 1}}, + }, + }, }}, } ) diff --git a/internal/remediation/testhelpers_test.go b/internal/remediation/testhelpers_test.go index a70f68bd71e..770d811cb3b 100644 --- a/internal/remediation/testhelpers_test.go +++ b/internal/remediation/testhelpers_test.go @@ -58,11 +58,8 @@ func checkRemediationResults(t *testing.T, res []resolution.Difference) { toMinimalVuln := func(v resolution.Vulnerability) minimalVuln { t.Helper() nodes := make(map[resolve.NodeID]struct{}) - for _, c := range v.ProblemChains { - nodes[c.Edges[0].To] = struct{}{} - } - for _, c := range v.NonProblemChains { - nodes[c.Edges[0].To] = struct{}{} + for _, sg := range v.Subgraphs { + nodes[sg.Dependency] = struct{}{} } sortedNodes := maps.Keys(nodes) slices.Sort(sortedNodes) diff --git a/internal/resolution/__snapshots__/resolve_test.snap b/internal/resolution/__snapshots__/resolve_test.snap index cf4b7406c8c..00593827a46 100755 --- a/internal/resolution/__snapshots__/resolve_test.snap +++ b/internal/resolution/__snapshots__/resolve_test.snap @@ -16,101 +16,279 @@ complex 9.9.9 { "ID": "CMPLX-0000-0000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "complex", + "VersionType": 1, + "Version": "9.9.9" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "alice", + "VersionType": 1, + "Version": "1.0.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] }, { "ID": "CMPLX-1000-0000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 2, - "Requirement": "2.2.2", - "Type": {} - } - ], - [ - { - "From": 1, - "To": 2, - "Requirement": "^2.0.0", - "Type": {} - }, - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} - } - ], - [ - { - "From": 3, - "To": 2, - "Requirement": "^2.2.2", - "Type": {} - }, - { - "From": 0, - "To": 3, - "Requirement": "~3.3.3", - "Type": {} + "Subgraphs": [ + { + "Dependency": 2, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "complex", + "VersionType": 1, + "Version": "9.9.9" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 2, + "Requirement": "2.2.2", + "Type": {} + }, + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 0, + "To": 3, + "Requirement": "~3.3.3", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "alice", + "VersionType": 1, + "Version": "1.0.1" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 1, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "bob", + "VersionType": 1, + "Version": "2.2.2" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "2.2.2", + "Type": {} + }, + { + "From": 1, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + }, + { + "From": 3, + "To": 2, + "Requirement": "^2.2.2", + "Type": {} + }, + { + "From": 4, + "To": 2, + "Requirement": "^2.0.1", + "Type": {} + } + ], + "Children": null + }, + "3": { + "Version": { + "System": 3, + "Name": "dave", + "VersionType": 1, + "Version": "3.3.3" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 3, + "Requirement": "~3.3.3", + "Type": {} + } + ], + "Children": [ + { + "From": 3, + "To": 2, + "Requirement": "^2.2.2", + "Type": {} + }, + { + "From": 3, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "4": { + "Version": { + "System": 3, + "Name": "chuck", + "VersionType": 1, + "Version": "2.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 3, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 4, + "To": 2, + "Requirement": "^2.0.1", + "Type": {} + } + ] + } } - ], - [ - { - "From": 4, - "To": 2, - "Requirement": "^2.0.1", - "Type": {} - }, - { - "From": 3, - "To": 4, - "Requirement": "^2.0.0", - "Type": {} - }, - { - "From": 0, - "To": 3, - "Requirement": "~3.3.3", - "Type": {} - } - ] - ], - "NonProblemChains": [] + } + ] }, { "ID": "CMPLX-2000-0000", "DevOnly": true, - "ProblemChains": [ - [ - { - "From": 3, - "To": 4, - "Requirement": "^2.0.0", - "Type": {} - }, - { - "From": 0, - "To": 3, - "Requirement": "~3.3.3", - "Type": {} + "Subgraphs": [ + { + "Dependency": 4, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "complex", + "VersionType": 1, + "Version": "9.9.9" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 3, + "Requirement": "~3.3.3", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "dave", + "VersionType": 1, + "Version": "3.3.3" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 3, + "Requirement": "~3.3.3", + "Type": {} + } + ], + "Children": [ + { + "From": 3, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "4": { + "Version": { + "System": 3, + "Name": "chuck", + "VersionType": 1, + "Version": "2.0.0" + }, + "Distance": 0, + "Parents": [ + { + "From": 3, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -131,57 +309,148 @@ diamond 1.0.0 { "ID": "DIA-000-000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 1, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} - } - ], - [ - { - "From": 2, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} - } - ], - [ - { - "From": 4, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 2, - "To": 4, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 3, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "diamond", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "pkg", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 1, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "dep-one", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 2, + "To": 4, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 1, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 4, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + }, + "4": { + "Version": { + "System": 3, + "Name": "dep-two", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 2, + "To": 4, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 4, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -199,52 +468,187 @@ different-pkgs 3.0.0 { "ID": "OSV-000-000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 2, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 3, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "different-pkgs", + "VersionType": 1, + "Version": "3.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] }, { "ID": "OSV-000-001", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "different-pkgs", + "VersionType": 1, + "Version": "3.0.0" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad2", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ], - [ - { - "From": 2, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} + }, + { + "Dependency": 3, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "different-pkgs", + "VersionType": 1, + "Version": "3.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 2, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -260,17 +664,49 @@ direct 1.0.0 { "ID": "OSV-000-001", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^2.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "direct", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "2.2.2" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -290,74 +726,294 @@ duplicates 1.1.1 { "ID": "OSV-000-000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "duplicates", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ], - [ - { - "From": 3, - "To": 5, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} + }, + { + "Dependency": 5, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "duplicates", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 3, + "To": 5, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "5": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 3, + "To": 5, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] }, { "ID": "OSV-000-001", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "duplicates", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ], - [ - { - "From": 2, - "To": 4, - "Requirement": "^2.0.0", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^2.0.0", - "Type": {} + }, + { + "Dependency": 4, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "duplicates", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "2.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "4": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "2.2.2" + }, + "Distance": 0, + "Parents": [ + { + "From": 2, + "To": 4, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": null + } } - ], - [ - { - "From": 3, - "To": 5, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 3, - "Requirement": "^1.0.0", - "Type": {} + }, + { + "Dependency": 5, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "duplicates", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "3": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 3, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 3, + "To": 5, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "5": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 3, + "To": 5, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -374,23 +1030,74 @@ existing 1.0.0 { "ID": "OSV-000-001", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 1, - "To": 2, - "Requirement": "^2.0.0", - "Type": {} - }, - { - "From": 0, - "To": 1, - "Requirement": "^2.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 2, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "existing", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "2.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 1, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "2.2.2" + }, + "Distance": 0, + "Parents": [ + { + "From": 1, + "To": 2, + "Requirement": "^2.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -408,61 +1115,170 @@ non-problem 1.0.0 { "ID": "OSV-000-000", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "non-problem", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 0, + "To": 2, + "Requirement": "^3.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 2, + "To": 1, + "Requirement": "*", + "Type": {} + } + ], + "Children": null + }, + "2": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "3.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^3.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 1, + "Requirement": "*", + "Type": {} + } + ] + } } - ] - ], - "NonProblemChains": [ - [ - { - "From": 2, - "To": 1, - "Requirement": "*", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^3.0.0", - "Type": {} - } - ] + } ] }, { "ID": "OSV-000-001", "DevOnly": false, - "ProblemChains": [ - [ - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} - } - ], - [ - { - "From": 2, - "To": 1, - "Requirement": "*", - "Type": {} - }, - { - "From": 0, - "To": 2, - "Requirement": "^3.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 1, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "non-problem", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 0, + "To": 2, + "Requirement": "^3.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + }, + { + "From": 2, + "To": 1, + "Requirement": "*", + "Type": {} + } + ], + "Children": null + }, + "2": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "3.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 2, + "Requirement": "^3.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 2, + "To": 1, + "Requirement": "*", + "Type": {} + } + ] + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- @@ -479,44 +1295,146 @@ simple 1.0.0 { "ID": "OSV-000-000", "DevOnly": true, - "ProblemChains": [ - [ - { - "From": 1, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 2, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "simple", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 1, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 1, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] }, { "ID": "OSV-000-001", "DevOnly": true, - "ProblemChains": [ - [ - { - "From": 1, - "To": 2, - "Requirement": "^1.0.0", - "Type": {} - }, - { - "From": 0, - "To": 1, - "Requirement": "^1.0.0", - "Type": {} + "Subgraphs": [ + { + "Dependency": 2, + "Nodes": { + "0": { + "Version": { + "System": 3, + "Name": "simple", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 2, + "Parents": null, + "Children": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "1": { + "Version": { + "System": 3, + "Name": "dependency", + "VersionType": 1, + "Version": "1.0.0" + }, + "Distance": 1, + "Parents": [ + { + "From": 0, + "To": 1, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": [ + { + "From": 1, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ] + }, + "2": { + "Version": { + "System": 3, + "Name": "bad", + "VersionType": 1, + "Version": "1.1.1" + }, + "Distance": 0, + "Parents": [ + { + "From": 1, + "To": 2, + "Requirement": "^1.0.0", + "Type": {} + } + ], + "Children": null + } } - ] - ], - "NonProblemChains": [] + } + ] } ] --- diff --git a/internal/resolution/dependency_chain.go b/internal/resolution/dependency_chain.go deleted file mode 100644 index 2703215daa8..00000000000 --- a/internal/resolution/dependency_chain.go +++ /dev/null @@ -1,137 +0,0 @@ -package resolution - -import ( - "context" - "slices" - - "deps.dev/util/resolve" - "deps.dev/util/resolve/dep" - "github.com/google/osv-scanner/v2/internal/resolution/manifest" - "github.com/google/osv-scanner/v2/internal/resolution/util" - vulnUtil "github.com/google/osv-scanner/v2/internal/utility/vulns" - "github.com/google/osv-scanner/v2/pkg/lockfile" - "github.com/google/osv-scanner/v2/pkg/models" -) - -type DependencyChain struct { - Graph *resolve.Graph - Edges []resolve.Edge // Edge from root node is at the end of the list -} - -// At returns the dependency information of the dependency at the specified index along the chain. -// Returns the resolved VersionKey of the dependency, and the version requirement string. -// index 0 is the end dependency (usually the vulnerability) -// index len(Edges)-1 is the direct dependency from the root node -func (dc DependencyChain) At(index int) (resolve.VersionKey, string) { - edge := dc.Edges[index] - return dc.Graph.Nodes[edge.To].Version, edge.Requirement -} - -func (dc DependencyChain) Direct() (resolve.VersionKey, string) { - return dc.At(len(dc.Edges) - 1) -} - -func (dc DependencyChain) End() (resolve.VersionKey, string) { - return dc.At(0) -} - -func ChainIsDev(dc DependencyChain, groups map[manifest.RequirementKey][]string) bool { - edge := dc.Edges[len(dc.Edges)-1] - // This check only applies to the graphs created from the in-place lockfile scanning. - if edge.Type.HasAttr(dep.Dev) { - return true - } - - // As a workaround for npm workspaces, repeat above in-place check 1 layer deeper. - if len(dc.Edges) > 1 && dc.Edges[len(dc.Edges)-2].Type.HasAttr(dep.Dev) { - return true - } - - req := resolve.RequirementVersion{ - VersionKey: dc.Graph.Nodes[edge.To].Version, - Type: edge.Type.Clone(), - } - ecosystem, ok := util.OSVEcosystem[req.System] - if !ok { - return false - } - // TODO: Below check doesn't support npm workspaces correctly. - return lockfile.Ecosystem(ecosystem).IsDevGroup(groups[manifest.MakeRequirementKey(req)]) -} - -// ComputeChains computes all paths from each specified NodeID to the root node. -func ComputeChains(g *resolve.Graph, nodes []resolve.NodeID) [][]DependencyChain { - // find the parent nodes of each node in graph, for easier traversal - parentEdges := make(map[resolve.NodeID][]resolve.Edge) - for _, e := range g.Edges { - // check for a self-dependency, just in case - if e.From == e.To { - continue - } - parentEdges[e.To] = append(parentEdges[e.To], e) - } - - allChains := make([][]DependencyChain, len(nodes)) - // for each node, traverse up all possible paths to the root node - for i, node := range nodes { - var toProcess []DependencyChain - for _, pEdge := range parentEdges[node] { - toProcess = append(toProcess, DependencyChain{ - Graph: g, - Edges: []resolve.Edge{pEdge}, - }) - } - for len(toProcess) > 0 { - chain := toProcess[0] - toProcess = toProcess[1:] - edge := chain.Edges[len(chain.Edges)-1] - if edge.From == 0 { // we are at the root, add it to the final list - allChains[i] = append(allChains[i], chain) - continue - } - // add all parent edges to the queue - for _, pEdge := range parentEdges[edge.From] { - // check for a dependency cycle before adding them - if !slices.ContainsFunc(chain.Edges, func(e resolve.Edge) bool { return e.To == pEdge.To }) { - toProcess = append(toProcess, DependencyChain{ - Graph: g, - Edges: append(slices.Clone(chain.Edges), pEdge), - }) - } - } - } - } - - return allChains -} - -// chainConstrains check if a DependencyChain is 'Problematic' -// i.e. if it is forcing the vulnerable package to chosen in resolution. -func chainConstrains(ctx context.Context, cl resolve.Client, chain DependencyChain, vuln *models.Vulnerability) bool { - // TODO: Logic needs to be ecosystem-specific. - if len(chain.Edges) == 0 { - return false - } - // Just check if the direct requirement of the vulnerable package is constraining it. - // This still has some false positives. - // e.g. if we have - // A@* -> B@2.* - // D@* -> B@2.1.1 -> C@1.0.0 - // resolving both together picks B@2.1.1 & thus constrains C to C@1.0.0 for A - // But resolving A alone could pick B@2.2.0 which might not depend on C - // Similarly, a direct dependency could be constrained by an indirect dependency with similar results. - - // Check if the latest allowable version of the package is vulnerable - vk, req := chain.End() - vk.Version = req - vk.VersionType = resolve.Requirement - vers, err := cl.MatchingVersions(ctx, vk) - if err != nil { - // TODO: handle error - return true - } - - bestVk := vers[len(vers)-1] // This should be the highest version for npm - - return vulnUtil.IsAffected(*vuln, util.VKToPackageDetails(bestVk.VersionKey)) -} diff --git a/internal/resolution/dependency_subgraph.go b/internal/resolution/dependency_subgraph.go new file mode 100644 index 00000000000..f0141ad33b4 --- /dev/null +++ b/internal/resolution/dependency_subgraph.go @@ -0,0 +1,246 @@ +package resolution + +import ( + "context" + "slices" + + "deps.dev/util/resolve" + "deps.dev/util/resolve/dep" + "github.com/google/osv-scanner/v2/internal/resolution/manifest" + "github.com/google/osv-scanner/v2/internal/resolution/util" + vulnUtil "github.com/google/osv-scanner/v2/internal/utility/vulns" + "github.com/google/osv-scanner/v2/pkg/lockfile" + "github.com/google/osv-scanner/v2/pkg/models" +) + +type GraphNode struct { + Version resolve.VersionKey + Distance int // The shortest distance to the end Dependency Node (which has a Distance of 0) + Parents []resolve.Edge // Parent edges i.e. with Edge.To == this ID + Children []resolve.Edge // Child edges i.e. with Edge.From == this ID +} + +type DependencySubgraph struct { + Dependency resolve.NodeID // The NodeID of the end dependency of this subgraph. + Nodes map[resolve.NodeID]GraphNode +} + +// ComputeSubgraphs computes the DependencySubgraphs for each specified NodeID. +// The computed Subgraphs contains all nodes and edges that transitively depend on the specified node, and the node itself. +// +// Modifying any of the returned DependencySubgraphs may cause unexpected behaviour. +func ComputeSubgraphs(g *resolve.Graph, nodes []resolve.NodeID) []*DependencySubgraph { + // Find the parent nodes of each node in graph, for easier traversal. + // These slices are shared between the returned subgraphs. + parentEdges := make(map[resolve.NodeID][]resolve.Edge) + for _, e := range g.Edges { + // Check for a self-dependency, just in case. + if e.From == e.To { + continue + } + parentEdges[e.To] = append(parentEdges[e.To], e) + } + + // For each node, compute the subgraph. + subGraphs := make([]*DependencySubgraph, 0, len(nodes)) + for _, nodeID := range nodes { + // Starting at the node of interest, visit all unvisited parents, + // adding the corresponding edges to the GraphNodes. + gNodes := make(map[resolve.NodeID]GraphNode) + seen := make(map[resolve.NodeID]struct{}) + seen[nodeID] = struct{}{} + toProcess := []resolve.NodeID{nodeID} + currDistance := 0 // The current distance from end dependency. + for len(toProcess) > 0 { + // Track the next set of nodes to process, which will be +1 Distance away from end. + var next []resolve.NodeID + for _, node := range toProcess { + // Construct the GraphNode + parents := parentEdges[node] + gNode := gNodes[node] // Grab the existing GraphNode, which will have some Children populated. + gNode.Version = g.Nodes[node].Version + gNode.Distance = currDistance + gNode.Parents = parents + gNodes[node] = gNode + // Populate parent's children and add to next set. + for _, edge := range parents { + nID := edge.From + pNode := gNodes[nID] + pNode.Children = append(pNode.Children, edge) + gNodes[nID] = pNode + if _, ok := seen[nID]; !ok { + seen[nID] = struct{}{} + next = append(next, nID) + } + } + } + toProcess = next + currDistance++ + } + + subGraphs = append(subGraphs, &DependencySubgraph{ + Dependency: nodeID, + Nodes: gNodes, + }) + } + + return subGraphs +} + +// IsDevOnly checks if this DependencySubgraph solely contains dev (or test) dependencies. +// If groups is nil, checks the dep.Type of the direct graph edges for the Dev Attr (for in-place). +// Otherwise, uses the groups of the direct dependencies to determine if a non-dev path exists (for relax/override). +func (ds *DependencySubgraph) IsDevOnly(groups map[manifest.RequirementKey][]string) bool { + if groups != nil { + // Check if any of the direct dependencies are not in the dev group. + return !slices.ContainsFunc(ds.Nodes[0].Children, func(e resolve.Edge) bool { + req := resolve.RequirementVersion{ + VersionKey: ds.Nodes[e.To].Version, + Type: e.Type.Clone(), + } + ecosystem, ok := util.OSVEcosystem[req.System] + if !ok { + return true + } + + return !lockfile.Ecosystem(ecosystem).IsDevGroup(groups[manifest.MakeRequirementKey(req)]) + }) + } + + // groups == nil + // Check if any of the direct dependencies do not have the Dev attr. + for _, e := range ds.Nodes[0].Children { + if e.Type.HasAttr(dep.Dev) { + continue + } + // As a workaround for npm workspaces, check for the a Dev attr in the direct dependency's dependencies. + for _, e2 := range ds.Nodes[e.To].Children { + if !e2.Type.HasAttr(dep.Dev) { + return false + } + } + // If the vulnerable dependency is a direct dependency, it'd have no Children. + // Since we've already checked that it doesn't have the Dev attr, it must be a non-dev dependency. + if e.To == ds.Dependency { + return false + } + } + + return true +} + +// ConstrainingSubgraph tries to construct a subgraph of the subgraph that includes only the edges that contribute to a vulnerability. +// It identifies the dependencies which constrain the vulnerable package to use a vulnerable version. +// This is used by the 'relax' remediation strategy to identify which direct dependencies need to be updated. +// +// e.g. for a subgraph with: +// +// A -> C@<2.0 +// B -> C@<3.0 +// C resolves to C@1.9 +// +// If the vuln affecting C is fixed in version 2.0, the constraining subgraph would only contain A, +// since B would allow versions >=2.0 of C to be selected if not for A. +// +// This is a heuristic approach and may produce false positives (meaning possibly unnecessary dependencies would be flagged to be relaxed). +// If the constraining subgraph cannot be computed for some reason, returns the original DependencySubgraph. +func (ds *DependencySubgraph) ConstrainingSubgraph(ctx context.Context, cl resolve.Client, vuln *models.Vulnerability) *DependencySubgraph { + // Just check if the direct requirement of the vulnerable package is constraining it. + // This still has some false positives. + // e.g. if we have + // A@* -> B@2.* + // D@* -> B@2.1.1 -> C@1.0.0 + // resolving both together picks B@2.1.1 & thus constrains C to C@1.0.0 for A + // But resolving A alone could pick B@2.2.0 which might not depend on C + // Similarly, a direct dependency could be constrained by an indirect dependency with similar results. + end := ds.Nodes[ds.Dependency] + newParents := make([]resolve.Edge, 0, len(end.Parents)) + for _, pEdge := range end.Parents { + // Check if the latest allowable version of the package is vulnerable + vk := end.Version + vk.Version = pEdge.Requirement + vk.VersionType = resolve.Requirement + vers, err := cl.MatchingVersions(ctx, vk) + if err != nil || len(vers) == 0 { + // Could not determine MatchingVersions - assume this is constraining. + newParents = append(newParents, pEdge) + continue + } + bestVK := vers[len(vers)-1] // This should be the highest version for npm + + if vulnUtil.IsAffected(*vuln, util.VKToPackageDetails(bestVK.VersionKey)) { + newParents = append(newParents, pEdge) + } + } + + if len(newParents) == 0 { + // There has to be at least one constraining path for the vulnerability to appear. + // If our heuristic couldn't determine any, treat the whole subgraph as constraining. + return ds + } + + // Rebuild the DependencySubgraph using the dependency's newParents. + // Same logic as in ComputeSubgraphs. + newNodes := make(map[resolve.NodeID]GraphNode) + newNodes[ds.Dependency] = GraphNode{ + Version: end.Version, + Distance: 0, + Parents: newParents, + } + + seen := make(map[resolve.NodeID]struct{}) + seen[ds.Dependency] = struct{}{} + toProcess := make([]resolve.NodeID, 0, len(newParents)) + for _, e := range newParents { + toProcess = append(toProcess, e.From) + seen[e.From] = struct{}{} + } + + currDistance := 1 + for len(toProcess) > 0 { + var next []resolve.NodeID + for _, nID := range toProcess { + oldNode := ds.Nodes[nID] + newNode := GraphNode{ + Version: oldNode.Version, + Distance: currDistance, + Parents: slices.Clone(oldNode.Parents), + Children: slices.Clone(oldNode.Children), + } + // Remove the non-constraining edge from the node's children if it ends up in the subgraph. + newNode.Children = slices.DeleteFunc(newNode.Children, func(e resolve.Edge) bool { + if e.To != ds.Dependency { + return false + } + + return !slices.ContainsFunc(newParents, func(pEdge resolve.Edge) bool { + return pEdge.From == e.From && + pEdge.Requirement == e.Requirement && + pEdge.Type.Compare(e.Type) == 0 + }) + }) + newNodes[nID] = newNode + for _, e := range newNode.Parents { + if _, ok := seen[e.From]; !ok { + seen[e.From] = struct{}{} + next = append(next, e.From) + } + } + } + toProcess = next + currDistance++ + } + // Remove children edges to nodes that are not in the computed subgraph. + for nID, edge := range newNodes { + edge.Children = slices.DeleteFunc(edge.Children, func(e resolve.Edge) bool { + _, ok := seen[e.To] + return !ok + }) + newNodes[nID] = edge + } + + return &DependencySubgraph{ + Dependency: ds.Dependency, + Nodes: newNodes, + } +} diff --git a/internal/resolution/dependency_subgraph_test.go b/internal/resolution/dependency_subgraph_test.go new file mode 100644 index 00000000000..a7b7d57a7ce --- /dev/null +++ b/internal/resolution/dependency_subgraph_test.go @@ -0,0 +1,335 @@ +package resolution_test + +import ( + "cmp" + "context" + "maps" + "slices" + "testing" + + "deps.dev/util/resolve" + "deps.dev/util/resolve/schema" + gocmp "github.com/google/go-cmp/cmp" + "github.com/google/osv-scanner/v2/internal/resolution" + "github.com/google/osv-scanner/v2/internal/resolution/manifest" + "github.com/google/osv-scanner/v2/pkg/models" +) + +func TestDependencySubgraph(t *testing.T) { + t.Parallel() + g, err := schema.ParseResolve(` +a 0.0.1 + b@^1.0.1 1.0.1 + $c@^1.0.0 + d: d@^2.2.2 2.2.2 + c: c@^1.0.2 1.0.2 + e@1.0.0 1.0.0 + $d@^2.0.0 + f@^1.1.1 1.1.1 + $c@^1.0.1 + g@^2.2.2 2.2.2 + h@^3.3.3 3.3.3 + $d@^2.2.0 +`, resolve.NPM) + if err != nil { + t.Fatalf("failed to parse test graph: %v", err) + } + + nodes := make([]resolve.NodeID, len(g.Nodes)-1) + for i := range nodes { + nodes[i] = resolve.NodeID(i + 1) + } + + subgraphs := resolution.ComputeSubgraphs(g, nodes) + for _, sg := range subgraphs { + checkSubgraphVersions(t, sg, g) + checkSubgraphEdges(t, sg) + checkSubgraphNodesReachable(t, sg) + checkSubgraphDistances(t, sg) + } +} + +func TestConstrainingSubgraph(t *testing.T) { + t.Parallel() + const vulnPkgName = "vuln" + g, err := schema.ParseResolve(` +root 1.0.0 + vuln: vuln@<3 1.0.1 + nonprob1@^1.0.0 1.0.0 + $vuln@>1 + prob1@^1.0.0 1.0.0 + $vuln@^1.0.0 + prob2@^2.0.0 2.0.0 + nonprob2@* 1.0.0 + $vuln@* + $vuln@* + dep@3.0.0 3.0.0 + $vuln@1.0.1 +`, resolve.NPM) + if err != nil { + t.Fatalf("failed to parse test graph: %v", err) + } + + nID := slices.IndexFunc(g.Nodes, func(n resolve.Node) bool { return n.Version.Name == vulnPkgName }) + if nID < 0 { + t.Fatalf("failed to find vulnerable node in test graph") + } + subgraph := resolution.ComputeSubgraphs(g, []resolve.NodeID{resolve.NodeID(nID)})[0] + + cl := resolve.NewLocalClient() + v := resolve.Version{ + VersionKey: resolve.VersionKey{ + PackageKey: resolve.PackageKey{ + System: resolve.NPM, + Name: vulnPkgName, + }, + VersionType: resolve.Concrete, + }, + } + v.Version = "1.0.0" + cl.AddVersion(v, []resolve.RequirementVersion{}) + v.Version = "1.0.1" + cl.AddVersion(v, []resolve.RequirementVersion{}) + v.Version = "2.0.0" + cl.AddVersion(v, []resolve.RequirementVersion{}) + vuln := &models.Vulnerability{ + ID: "VULN-001", + Affected: []models.Affected{{ + Package: models.Package{ + Ecosystem: "npm", + Name: vulnPkgName, + }, + Ranges: []models.Range{ + { + Type: "SEMVER", + Events: []models.Event{{Introduced: "0"}, {Fixed: "2.0.0"}}, + }, + }, + }, + }} + got := subgraph.ConstrainingSubgraph(context.Background(), cl, vuln) + checkSubgraphVersions(t, got, g) + checkSubgraphEdges(t, got) + checkSubgraphNodesReachable(t, got) + checkSubgraphDistances(t, got) + + // Checking that we have the expected remaining nodes + expectedRemoved := []string{"nonprob1", "nonprob2"} + for _, pkgName := range expectedRemoved { + nID := slices.IndexFunc(g.Nodes, func(n resolve.Node) bool { return n.Version.Name == pkgName }) + if nID < 0 { + t.Fatalf("failed to find expected node in test graph") + } + if _, found := got.Nodes[resolve.NodeID(nID)]; found { + t.Errorf("non-constraining node was not removed from constraining subgraph: %s", pkgName) + } + } + if len(got.Nodes) != len(subgraph.Nodes)-len(expectedRemoved) { + t.Errorf("extraneous nodes found in constraining subgraph") + } + for nID := range got.Nodes { + if _, ok := subgraph.Nodes[nID]; !ok { + t.Errorf("extraneous node (%v) found in constraining subgraph", nID) + } + } + + // Check that ConstrainingSubgraph is stable if reapplied + again := got.ConstrainingSubgraph(context.Background(), cl, vuln) + if diff := gocmp.Diff(got, again); diff != "" { + t.Errorf("ConstrainingSubgraph output changed on reapply (-want +got):\n%s", diff) + } +} + +func TestSubgraphIsDevOnly(t *testing.T) { + t.Parallel() + g, err := schema.ParseResolve(` +a 1.0.0 + b@1.0.0 1.0.0 + prod: prod@1.0.0 1.0.0 + Dev|c@1.0.0 1.0.0 + $prod@1.0.0 + dev: dev@1.0.0 1.0.0 + Dev|d@1.0.0 1.0.0 + $dev@1.0.0 +`, resolve.NPM) + if err != nil { + t.Fatalf("failed to parse test graph: %v", err) + } + + prodID := slices.IndexFunc(g.Nodes, func(n resolve.Node) bool { return n.Version.Name == "prod" }) + if prodID < 0 { + t.Fatalf("failed to find vulnerable node in test graph") + } + devID := slices.IndexFunc(g.Nodes, func(n resolve.Node) bool { return n.Version.Name == "dev" }) + if devID < 0 { + t.Fatalf("failed to find vulnerable node in test graph") + } + + subgraphs := resolution.ComputeSubgraphs(g, []resolve.NodeID{resolve.NodeID(prodID), resolve.NodeID(devID)}) + prodGraph := subgraphs[0] + devGraph := subgraphs[1] + + if prodGraph.IsDevOnly(nil) { + t.Errorf("non-dev subgraph has IsDevOnly(nil) == true") + } + if !devGraph.IsDevOnly(nil) { + t.Errorf("dev-only subgraph has IsDevOnly(nil) == false") + } + + groups := map[manifest.RequirementKey][]string{ + {PackageKey: resolve.PackageKey{System: resolve.NPM, Name: "c"}, EcosystemSpecific: ""}: {"dev"}, + {PackageKey: resolve.PackageKey{System: resolve.NPM, Name: "d"}, EcosystemSpecific: ""}: {"dev"}, + } + if prodGraph.IsDevOnly(groups) { + t.Errorf("non-dev subgraph has IsDevOnly(groups) == true") + } + if !devGraph.IsDevOnly(groups) { + t.Errorf("dev-only subgraph has IsDevOnly(groups) == false") + } +} + +func checkSubgraphVersions(t *testing.T, sg *resolution.DependencySubgraph, g *resolve.Graph) { + // Check that the nodes and versions in the subgraph are correct + t.Helper() + if _, ok := sg.Nodes[0]; !ok { + t.Errorf("DependencySubgraph missing root node (0)") + } + if _, ok := sg.Nodes[sg.Dependency]; !ok { + t.Errorf("DependencySubgraph missing Dependency node (%v)", sg.Dependency) + } + for nID, node := range sg.Nodes { + if nID < 0 || int(nID) >= len(g.Nodes) { + t.Errorf("DependencySubgraph contains invalid node ID: %v", nID) + continue + } + want := g.Nodes[nID].Version + got := node.Version + if diff := gocmp.Diff(want, got); diff != "" { + t.Errorf("DependencySubgraph node %v does not match Graph (-want +got):\n%s", nID, diff) + } + } +} + +func checkSubgraphEdges(t *testing.T, sg *resolution.DependencySubgraph) { + // Check that every edge in a node's Parents appears in that parent's Children and vice versa. + t.Helper() + // Check the root node has no parents & end node has no children + if root, ok := sg.Nodes[0]; !ok { + t.Errorf("DependencySubgraph missing root node (0)") + } else if len(root.Parents) != 0 { + t.Errorf("DependencySubgraph root node (0) has parent nodes: %v", root.Parents) + } + if end, ok := sg.Nodes[sg.Dependency]; !ok { + t.Errorf("DependencySubgraph missing Dependency node (%v)", sg.Dependency) + } else if len(end.Children) != 0 { + t.Errorf("DependencySubgraph Dependency node (%v) has child nodes: %v", sg.Dependency, end.Children) + } + + edgeEq := func(a, b resolve.Edge) bool { + return a.From == b.From && + a.To == b.To && + a.Requirement == b.Requirement && + a.Type.Compare(b.Type) == 0 + } + + // Check each node's parents/children for same edges + for nID, node := range sg.Nodes { + // Only the root node should have no parents + if len(node.Parents) == 0 && nID != 0 { + t.Errorf("DependencySubgraph node %v has no parent nodes", nID) + } + for _, e := range node.Parents { + if e.To != nID { + t.Errorf("DependencySubgraph node %v contains invalid parent edge: %v", nID, e) + continue + } + parent, ok := sg.Nodes[e.From] + if !ok { + t.Errorf("DependencySubgraph edge missing node in subgraph: %v", e) + } + if !slices.ContainsFunc(parent.Children, func(edge resolve.Edge) bool { return edgeEq(e, edge) }) { + t.Errorf("DependencySubgraph node %v missing child edge: %v", e.From, e) + } + } + + // Only the end node should have no children + if len(node.Children) == 0 && nID != sg.Dependency { + t.Errorf("DependencySubgraph node %v has no child nodes", nID) + } + for _, e := range node.Children { + if e.From != nID { + t.Errorf("DependencySubgraph node %v contains invalid child edge: %v", nID, e) + continue + } + child, ok := sg.Nodes[e.To] + if !ok { + t.Errorf("DependencySubgraph edge missing node in subgraph: %v", e) + } + if !slices.ContainsFunc(child.Parents, func(edge resolve.Edge) bool { return edgeEq(e, edge) }) { + t.Errorf("DependencySubgraph node %v missing parent edge: %v", e.To, e) + } + } + } +} + +func checkSubgraphNodesReachable(t *testing.T, sg *resolution.DependencySubgraph) { + // Check that every node in the subgraph is reachable from the root node. + t.Helper() + seen := make(map[resolve.NodeID]struct{}) + todo := make([]resolve.NodeID, 0, len(sg.Nodes)) + todo = append(todo, 0) + seen[0] = struct{}{} + for len(todo) > 0 { + nID := todo[0] + todo = todo[1:] + node, ok := sg.Nodes[nID] + if !ok { + t.Errorf("DependencySubgraph missing expected node %v", nID) + continue + } + for _, e := range node.Children { + if _, ok := seen[e.To]; !ok { + todo = append(todo, e.To) + seen[e.To] = struct{}{} + } + } + } + + got := slices.Sorted(maps.Keys(seen)) + want := slices.Sorted(maps.Keys(sg.Nodes)) + if diff := gocmp.Diff(want, got); diff != "" { + t.Errorf("DependencySubgraph reachable nodes mismatch (-want +got):\n%s", diff) + } +} + +func checkSubgraphDistances(t *testing.T, sg *resolution.DependencySubgraph) { + // Check that the distances of each node have the correct value. + t.Helper() + if end, ok := sg.Nodes[sg.Dependency]; !ok { + t.Errorf("DependencySubgraph missing Dependency node (%v)", sg.Dependency) + } else if end.Distance != 0 { + t.Errorf("DependencySubgraph end Dependency distance is not 0") + } + + // Each node's distance should be one more than its smallest child's distance. + for nID, node := range sg.Nodes { + // The end dependency should have a distance of 0 + if nID == sg.Dependency { + if node.Distance != 0 { + t.Errorf("DependencySubgraph Dependency node (%v) has nonzero distance: %d", nID, node.Distance) + } + + continue + } + + if len(node.Children) == 0 { + t.Errorf("DependencySubgraph node %v has no child nodes", nID) + continue + } + e := slices.MinFunc(node.Children, func(a, b resolve.Edge) int { return cmp.Compare(sg.Nodes[a.To].Distance, sg.Nodes[b.To].Distance) }) + want := sg.Nodes[e.To].Distance + 1 + if node.Distance != want { + t.Errorf("DependencySubgraph node %v Distance = %d, want = %d", nID, node.Distance, want) + } + } +} diff --git a/internal/resolution/resolve.go b/internal/resolution/resolve.go index 120fa85943a..2371c7f1de4 100644 --- a/internal/resolution/resolve.go +++ b/internal/resolution/resolve.go @@ -20,16 +20,20 @@ import ( type Vulnerability struct { OSV models.Vulnerability DevOnly bool - // Chains are paths through requirements from direct dependency to vulnerable package. - // A 'Problem' chain constrains the package to a vulnerable version. - // 'NonProblem' chains re-use the vulnerable version, but would not resolve to a vulnerable version in isolation. - ProblemChains []DependencyChain - NonProblemChains []DependencyChain + // Subgraphs are the collections of nodes and edges that reach the vulnerable node. + // Subgraphs all contain the root node (NodeID 0) with no incoming edges (Parents), + // and the vulnerable node (NodeID DependencySubgraph.Dependency) with no outgoing edges (Children). + Subgraphs []*DependencySubgraph } func (rv Vulnerability) IsDirect() bool { - fn := func(dc DependencyChain) bool { return len(dc.Edges) == 1 } - return slices.ContainsFunc(rv.ProblemChains, fn) || slices.ContainsFunc(rv.NonProblemChains, fn) + for _, sg := range rv.Subgraphs { + if sg.Nodes[0].Distance == 1 { + return true + } + } + + return false } type Result struct { @@ -225,11 +229,11 @@ func (res *Result) computeVulns(ctx context.Context, cl client.ResolutionClient) } } - nodeChains := ComputeChains(res.Graph, vulnerableNodes) - vulnChains := make(map[string][]DependencyChain) + nodeSubgraphs := ComputeSubgraphs(res.Graph, vulnerableNodes) + vulnSubgraphs := make(map[string][]*DependencySubgraph) for i, nID := range vulnerableNodes { for _, vuln := range nodeVulns[nID] { - vulnChains[vuln.ID] = append(vulnChains[vuln.ID], nodeChains[i]...) + vulnSubgraphs[vuln.ID] = append(vulnSubgraphs[vuln.ID], nodeSubgraphs[i]) } } @@ -239,20 +243,8 @@ func (res *Result) computeVulns(ctx context.Context, cl client.ResolutionClient) // TODO: Combine aliased IDs for id, vuln := range vulnInfo { rv := Vulnerability{OSV: vuln, DevOnly: true} - for _, chain := range vulnChains[id] { - if chainConstrains(ctx, cl, chain, &rv.OSV) { - rv.ProblemChains = append(rv.ProblemChains, chain) - } else { - rv.NonProblemChains = append(rv.NonProblemChains, chain) - } - rv.DevOnly = rv.DevOnly && ChainIsDev(chain, res.Manifest.Groups) - } - if len(rv.ProblemChains) == 0 { - // There has to be at least one problem chain for the vulnerability to appear. - // If our heuristic couldn't determine any, treat them all as problematic. - rv.ProblemChains = rv.NonProblemChains - rv.NonProblemChains = nil - } + rv.Subgraphs = vulnSubgraphs[id] + rv.DevOnly = !slices.ContainsFunc(rv.Subgraphs, func(ds *DependencySubgraph) bool { return !ds.IsDevOnly(res.Manifest.Groups) }) res.Vulns = append(res.Vulns, rv) } diff --git a/internal/resolution/resolve_test.go b/internal/resolution/resolve_test.go index 28083090d62..ec747c7a315 100644 --- a/internal/resolution/resolve_test.go +++ b/internal/resolution/resolve_test.go @@ -20,25 +20,17 @@ func checkResult(t *testing.T, result *resolution.Result) { snap.MatchText(t, result.Graph.String()) type minimalVuln struct { - ID string - DevOnly bool - ProblemChains [][]resolve.Edge - NonProblemChains [][]resolve.Edge + ID string + DevOnly bool + Subgraphs []*resolution.DependencySubgraph } minVulns := make([]minimalVuln, len(result.Vulns)) for i, v := range result.Vulns { minVulns[i] = minimalVuln{ - ID: v.OSV.ID, - DevOnly: v.DevOnly, - ProblemChains: make([][]resolve.Edge, len(v.ProblemChains)), - NonProblemChains: make([][]resolve.Edge, len(v.NonProblemChains)), - } - for j, c := range v.ProblemChains { - minVulns[i].ProblemChains[j] = c.Edges - } - for j, c := range v.NonProblemChains { - minVulns[i].NonProblemChains[j] = c.Edges + ID: v.OSV.ID, + DevOnly: v.DevOnly, + Subgraphs: v.Subgraphs, } } slices.SortFunc(minVulns, func(a, b minimalVuln) int { diff --git a/internal/tui/dependency-graph.go b/internal/tui/dependency-graph.go index 2520ea1fc79..9417ea7be61 100644 --- a/internal/tui/dependency-graph.go +++ b/internal/tui/dependency-graph.go @@ -11,10 +11,10 @@ import ( ) type chainGraphNode struct { - vk resolve.VersionKey - isDirect bool // if this is a direct dependency - children []*chainGraphNode - // in this representation, a child is something that depends on this node + vk resolve.VersionKey + isDirect bool // if this is a direct dependency + dependents []*chainGraphNode + // in this representation, the dependents are the children of this node // so the root of the tree is rendered at the bottom } @@ -22,63 +22,60 @@ type ChainGraph struct { *chainGraphNode } -// for each unique vulnerable node, construct the graph from that node to each connected direct dependency, -// choosing only the shortest path -func FindChainGraphs(chains []resolution.DependencyChain) []ChainGraph { - // TODO: this is not deterministic - - // identifier for unique direct dep causes of unique vulnerabilities, - // used as a map key, so needs to be comparable - type chainEndpoints struct { - vulnDep resolve.NodeID - directDep resolve.NodeID - } - - // Find the shortest-length dependency chain for each direct/vulnerable node pair - shortestChains := make(map[chainEndpoints]resolution.DependencyChain) - for _, c := range chains { - endpoints := chainEndpoints{c.Edges[0].To, c.Edges[len(c.Edges)-1].To} - old, ok := shortestChains[endpoints] - if !ok { - shortestChains[endpoints] = c - continue - } - if len(old.Edges) > len(c.Edges) { - shortestChains[endpoints] = c +func subgraphEdges(sg *resolution.DependencySubgraph, direct resolve.NodeID) []resolve.Edge { + // find the shortest chain of edges from direct to the vulnerable node, excluding the root->direct edge. + // return them in reverse order, with edges[0].To = sg.Dependency + edges := make([]resolve.Edge, 0, sg.Nodes[0].Distance-1) + nID := direct + for nID != sg.Dependency { + n := sg.Nodes[nID] + idx := slices.IndexFunc(n.Children, func(e resolve.Edge) bool { return sg.Nodes[e.To].Distance == n.Distance-1 }) + if idx < 0 { + break } + edge := n.Children[idx] + edges = append(edges, edge) + nID = edge.To } + slices.Reverse(edges) + + return edges +} +// for each unique vulnerable node, construct the graph from that node to each connected direct dependency, +// choosing only the shortest path +func FindChainGraphs(subgraphs []*resolution.DependencySubgraph) []ChainGraph { // Construct the ChainGraphs - nodes := make(map[resolve.NodeID]*chainGraphNode) - var ret []ChainGraph - for _, c := range shortestChains { - if _, ok := nodes[c.Edges[0].To]; !ok { - // haven't encountered this specific vulnerable node before - // create it and add it to the returned graphs - vk, _ := c.End() - n := &chainGraphNode{ - vk: vk, - children: nil, - isDirect: c.Edges[0].From == 0, - } - ret = append(ret, ChainGraph{n}) - nodes[c.Edges[0].To] = n + ret := make([]ChainGraph, 0, len(subgraphs)) + for _, sg := range subgraphs { + nodes := make(map[resolve.NodeID]*chainGraphNode) + isDirect := func(nID resolve.NodeID) bool { + return slices.ContainsFunc(sg.Nodes[nID].Parents, func(e resolve.Edge) bool { return e.From == 0 }) } - // Going up the chain, add the node to the previous' children if it's not there already - for i, e := range c.Edges[:len(c.Edges)-1] { - p := nodes[e.To] - n, ok := nodes[e.From] - if !ok { - vk, _ := c.At(i + 1) - n = &chainGraphNode{ - vk: vk, - children: nil, - isDirect: i == len(c.Edges)-2, + // Create and add the vulnerable node to the returned graphs + n := &chainGraphNode{ + vk: sg.Nodes[sg.Dependency].Version, + dependents: nil, + isDirect: isDirect(sg.Dependency), + } + ret = append(ret, ChainGraph{n}) + nodes[sg.Dependency] = n + for _, startEdge := range sg.Nodes[0].Children { + // Going up the chain, add the node to the previous' children if it's not there already + for _, e := range subgraphEdges(sg, startEdge.To) { + p := nodes[e.To] + n, ok := nodes[e.From] + if !ok { + n = &chainGraphNode{ + vk: sg.Nodes[e.From].Version, + dependents: nil, + isDirect: isDirect(e.From), + } + nodes[e.From] = n + } + if !slices.Contains(p.dependents, n) { + p.dependents = append(p.dependents, n) } - nodes[e.From] = n - } - if !slices.Contains(p.children, n) { - p.children = append(p.children, n) } } } @@ -119,13 +116,13 @@ func (c *chainGraphNode) subString(isVuln bool) (string, int) { nodeOffset := lipgloss.Width(nodeStr) / 2 // No children, just show the text - if len(c.children) == 0 { + if len(c.dependents) == 0 { return nodeStr, nodeOffset } // one child, add a single line connecting this to the child above it - if len(c.children) == 1 { - childStr, childCenter := c.children[0].subString(false) + if len(c.dependents) == 1 { + childStr, childCenter := c.dependents[0].subString(false) if nodeOffset > childCenter { // left-pad the child if the parent is wider childStr = lipgloss.JoinHorizontal(lipgloss.Bottom, strings.Repeat(" ", nodeOffset-childCenter), childStr) @@ -139,11 +136,11 @@ func (c *chainGraphNode) subString(isVuln bool) (string, int) { // multiple children: // Join the children together on one line - nChilds := len(c.children) + nChilds := len(c.dependents) paddedChildStrings := make([]string, 0, 2*nChilds) // string of children, with padding strings in between childOffsets := make([]int, 0, nChilds) // where above the children to connect the lines to them width := 0 - for _, ch := range c.children { + for _, ch := range c.dependents { str, off := ch.subString(false) paddedChildStrings = append(paddedChildStrings, str, " ") childOffsets = append(childOffsets, width+off) diff --git a/internal/tui/vuln-info.go b/internal/tui/vuln-info.go index 07405e4f611..54927bc5c82 100644 --- a/internal/tui/vuln-info.go +++ b/internal/tui/vuln-info.go @@ -2,7 +2,6 @@ package tui import ( "fmt" - "slices" "strings" "github.com/charmbracelet/bubbles/key" @@ -68,8 +67,7 @@ func NewVulnInfo(vuln *resolution.Vulnerability) *vulnInfo { *v.mdStyle.Document.Margin = 0 v.mdStyle.Document.BlockPrefix = "" - chains := append(slices.Clone(vuln.ProblemChains), vuln.NonProblemChains...) - v.chainGraphs = FindChainGraphs(chains) + v.chainGraphs = FindChainGraphs(vuln.Subgraphs) return &v }