fix(dotnet): resolve packages.lock dependencies deterministically

syft/pkg/cataloger/dotnet/parse_packages_lock.go

@@ -4,9 +4,11 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"maps"
 	"slices"
 	"sort"
 
+	"github.com/anchore/go-version"
 	"github.com/anchore/packageurl-go"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/internal/relationship"
@@ -18,6 +20,9 @@ import (
 
 var _ generic.Parser = parseDotnetPackagesLock
 
+// directDependencyType is the "type" NuGet writes for a package the project references directly.
+const directDependencyType = "Direct"
+
 type dotnetPackagesLock struct {
 	Version      int                                         `json:"version"`
 	Dependencies map[string]map[string]dotnetPackagesLockDep `json:"dependencies"`
@@ -31,11 +36,7 @@ type dotnetPackagesLockDep struct {
 	Dependencies map[string]string `json:"dependencies,omitempty"`
 }
 
-func parseDotnetPackagesLock(_ context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) { //nolint:funlen
-	var pkgs []pkg.Package
-	var pkgMap = make(map[string]pkg.Package)
-	var relationships []artifact.Relationship
-
+func parseDotnetPackagesLock(_ context.Context, _ file.Resolver, _ *generic.Environment, reader file.LocationReadCloser) ([]pkg.Package, []artifact.Relationship, error) {
 	dec := json.NewDecoder(reader)
 
 	// unmarshal file
@@ -44,15 +45,47 @@ func parseDotnetPackagesLock(_ context.Context, _ file.Resolver, _ *generic.Envi
 		return nil, nil, fmt.Errorf("failed to parse packages.lock.json file: %w", err)
 	}
 
-	// collect all deps here
+	names, allDependencies := collectPackagesLockDeps(lockFile)
+
+	// create artifact for each pkg
+	var pkgs []pkg.Package
+	pkgMap := make(map[string]pkg.Package)
+
+	for _, nameVersion := range names {
+		name, _ := extractNameAndVersion(nameVersion)
+
+		dep := allDependencies[nameVersion]
+		dotnetPkg := newDotnetPackagesLockPackage(name, dep, reader.WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation))
+		if dotnetPkg != nil {
+			pkgs = append(pkgs, *dotnetPkg)
+			pkgMap[nameVersion] = *dotnetPkg
+		}
+	}
+
+	relationships := packagesLockRelationships(lockFile, pkgMap)
+
+	// sort the relationships for deterministic output
+	relationship.Sort(relationships)
+
+	return pkgs, relationships, nil
+}
+
+// collectPackagesLockDeps flattens the per-target-framework entries into one entry per name and version, returning
+// the sorted keys alongside the entries so that package order is deterministic. A package may appear under several
+// target frameworks, and "Direct" wins when it does: some target framework references the package directly.
+func collectPackagesLockDeps(lockFile dotnetPackagesLock) ([]string, map[string]dotnetPackagesLockDep) {
 	allDependencies := make(map[string]dotnetPackagesLockDep)
 
 	var names []string
-	for _, dependencies := range lockFile.Dependencies {
-		for name, dep := range dependencies {
+	for _, targetFramework := range slices.Sorted(maps.Keys(lockFile.Dependencies)) {
+		for _, name := range slices.Sorted(maps.Keys(lockFile.Dependencies[targetFramework])) {
+			dep := lockFile.Dependencies[targetFramework][name]
 			depNameVersion := createNameAndVersion(name, dep.Resolved)
 
-			if slices.Contains(names, depNameVersion) {
+			if existing, ok := allDependencies[depNameVersion]; ok {
+				if existing.Type != directDependencyType && dep.Type == directDependencyType {
+					allDependencies[depNameVersion] = dep
+				}
 				continue
 			}
 
@@ -64,55 +97,63 @@ func parseDotnetPackagesLock(_ context.Context, _ file.Resolver, _ *generic.Envi
 	// sort the names so that the order of the packages is deterministic
 	sort.Strings(names)
 
-	// create artifact for each pkg
-	for _, nameVersion := range names {
-		name, _ := extractNameAndVersion(nameVersion)
+	return names, allDependencies
+}
 
-		dep := allDependencies[nameVersion]
-		dotnetPkg := newDotnetPackagesLockPackage(name, dep, reader.WithAnnotation(pkg.EvidenceAnnotationKey, pkg.PrimaryEvidenceAnnotation))
-		if dotnetPkg != nil {
-			pkgs = append(pkgs, *dotnetPkg)
-			pkgMap[nameVersion] = *dotnetPkg
-		}
-	}
+// packagesLockRelationships resolves each dependency within its own target framework so that lockfiles pinning
+// multiple versions of the same package resolve to the correct one.
+func packagesLockRelationships(lockFile dotnetPackagesLock, pkgMap map[string]pkg.Package) []artifact.Relationship {
+	var relationships []artifact.Relationship
 
-	// fill up relationships
-	for depNameVersion, dep := range allDependencies {
-		parentPkg, ok := pkgMap[depNameVersion]
-		if !ok {
-			log.Debugf("package \"%s\" not found in map of all packages", depNameVersion)
-			continue
-		}
+	seen := make(map[string]struct{})
+	for _, targetFramework := range slices.Sorted(maps.Keys(lockFile.Dependencies)) {
+		frameworkDeps := lockFile.Dependencies[targetFramework]
 
-		for childDepName, childDepVersion := range dep.Dependencies {
-			childDepNameVersion := createNameAndVersion(childDepName, childDepVersion)
+		for _, name := range slices.Sorted(maps.Keys(frameworkDeps)) {
+			dep := frameworkDeps[name]
+			depNameVersion := createNameAndVersion(name, dep.Resolved)
 
-			// try and find pkg for dependency with exact name and version
-			childPkg, ok := pkgMap[childDepNameVersion]
+			parentPkg, ok := pkgMap[depNameVersion]
 			if !ok {
-				// no exact match found, lets match on name only, lockfile will contain other version of pkg
-				cpkg, ok := findPkgByName(childDepName, pkgMap)
-				if !ok {
-					log.Debugf("dependency \"%s\" of package \"%s\" not found in map of all packages", childDepNameVersion, depNameVersion)
-					continue
-				}
-
-				childPkg = *cpkg
+				log.Debugf("package \"%s\" not found in map of all packages", depNameVersion)
+				continue
 			}
 
-			rel := artifact.Relationship{
-				From: childPkg,
-				To:   parentPkg,
-				Type: artifact.DependencyOfRelationship,
-			}
-			relationships = append(relationships, rel)
+			relationships = append(relationships, packagesLockDepRelationships(dep, parentPkg, depNameVersion, frameworkDeps, pkgMap, seen)...)
 		}
 	}
 
-	// sort the relationships for deterministic output
-	relationship.Sort(relationships)
+	return relationships
+}
 
-	return pkgs, relationships, nil
+// packagesLockDepRelationships returns the edges declared by a single package under one target framework, skipping
+// any edge already recorded in seen -- frameworks that resolve to the same versions would otherwise repeat it.
+func packagesLockDepRelationships(dep dotnetPackagesLockDep, parentPkg pkg.Package, depNameVersion string, frameworkDeps map[string]dotnetPackagesLockDep, pkgMap map[string]pkg.Package, seen map[string]struct{}) []artifact.Relationship {
+	var relationships []artifact.Relationship
+
+	for _, childDepName := range slices.Sorted(maps.Keys(dep.Dependencies)) {
+		childDepVersion := dep.Dependencies[childDepName]
+
+		childPkg, ok := findDependencyPkg(childDepName, childDepVersion, frameworkDeps, pkgMap)
+		if !ok {
+			log.Debugf("dependency \"%s\" of package \"%s\" not found in map of all packages", createNameAndVersion(childDepName, childDepVersion), depNameVersion)
+			continue
+		}
+
+		key := string(childPkg.ID()) + string(parentPkg.ID())
+		if _, exists := seen[key]; exists {
+			continue
+		}
+		seen[key] = struct{}{}
+
+		relationships = append(relationships, artifact.Relationship{
+			From: *childPkg,
+			To:   parentPkg,
+			Type: artifact.DependencyOfRelationship,
+		})
+	}
+
+	return relationships
 }
 
 func newDotnetPackagesLockPackage(name string, dep dotnetPackagesLockDep, locations ...file.Location) *pkg.Package {
@@ -151,13 +192,57 @@ func packagesLockPackageURL(name, version string) string {
 	).ToString()
 }
 
+// findDependencyPkg finds the package a dependency points at. The version of a dependency edge is the lower bound
+// of a version range, not a pin, so prefer whatever version this target framework actually resolved to.
+func findDependencyPkg(name, declaredVersion string, frameworkDeps map[string]dotnetPackagesLockDep, pkgMap map[string]pkg.Package) (*pkg.Package, bool) {
+	if dep, ok := frameworkDeps[name]; ok {
+		if p, ok := pkgMap[createNameAndVersion(name, dep.Resolved)]; ok {
+			return &p, true
+		}
+	}
+
+	if p, ok := pkgMap[createNameAndVersion(name, declaredVersion)]; ok {
+		return &p, true
+	}
+
+	return findPkgByName(name, pkgMap)
+}
+
+// findPkgByName returns the lowest-versioned package with the given name. This is a last-resort fallback for
+// lockfiles where a dependency edge names a package that is absent from its own target framework
 func findPkgByName(pkgName string, pkgMap map[string]pkg.Package) (*pkg.Package, bool) {
-	for pkgNameVersion, pkg := range pkgMap {
+	var candidates []string
+	for pkgNameVersion := range pkgMap {
 		name, _ := extractNameAndVersion(pkgNameVersion)
 		if name == pkgName {
-			return &pkg, true
+			candidates = append(candidates, pkgNameVersion)
 		}
 	}
 
-	return nil, false
+	if len(candidates) == 0 {
+		return nil, false
+	}
+
+	// versions that don't parse sort after those that do
+	sort.Slice(candidates, func(i, j int) bool {
+		_, vi := extractNameAndVersion(candidates[i])
+		_, vj := extractNameAndVersion(candidates[j])
+
+		si, erri := version.NewVersion(vi)
+		sj, errj := version.NewVersion(vj)
+
+		if (erri == nil) != (errj == nil) {
+			return erri == nil
+		}
+
+		if erri == nil && !si.Equal(sj) {
+			return si.LessThan(sj)
+		}
+
+		return candidates[i] < candidates[j]
+	})
+
+	p := pkgMap[candidates[0]]
+
+	return &p, true
 }

syft/pkg/cataloger/dotnet/parse_packages_lock_test.go

@@ -197,3 +197,97 @@ func TestParseDotnetPackagesLock(t *testing.T) {
 
 	pkgtest.TestFileParser(t, fixture, parseDotnetPackagesLock, expectedPkgs, expectedRelationships)
 }
+
+func TestParseDotnetPackagesLock_multipleTargetFrameworks(t *testing.T) {
+	fixture := "testdata/packages.lock-multi-framework.json"
+	fixtureLocationSet := file.NewLocationSet(file.NewLocation(fixture))
+
+	myLibPkg := pkg.Package{
+		Name:      "MyLib",
+		Version:   "1.0.0",
+		PURL:      "pkg:nuget/MyLib@1.0.0",
+		Locations: fixtureLocationSet,
+		Language:  pkg.Dotnet,
+		Type:      pkg.DotnetPkg,
+		Metadata: pkg.DotnetPackagesLockEntry{
+			Name:        "MyLib",
+			Version:     "1.0.0",
+			ContentHash: "mylibhash==",
+			Type:        "Direct",
+		},
+	}
+
+	log4net2Pkg := pkg.Package{
+		Name:      "log4net",
+		Version:   "2.0.5",
+		PURL:      "pkg:nuget/log4net@2.0.5",
+		Locations: fixtureLocationSet,
+		Language:  pkg.Dotnet,
+		Type:      pkg.DotnetPkg,
+		Metadata: pkg.DotnetPackagesLockEntry{
+			Name:        "log4net",
+			Version:     "2.0.5",
+			ContentHash: "log4net205hash==",
+			Type:        "Transitive",
+		},
+	}
+
+	log4net1Pkg := pkg.Package{
+		Name:      "log4net",
+		Version:   "1.2.15",
+		PURL:      "pkg:nuget/log4net@1.2.15",
+		Locations: fixtureLocationSet,
+		Language:  pkg.Dotnet,
+		Type:      pkg.DotnetPkg,
+		Metadata: pkg.DotnetPackagesLockEntry{
+			Name:        "log4net",
+			Version:     "1.2.15",
+			ContentHash: "log4net1215hash==",
+			Type:        "Transitive",
+		},
+	}
+
+	// resolved to the same version under both frameworks, but declared "Direct" by only one of them
+	newtonsoftPkg := pkg.Package{
+		Name:      "Newtonsoft.Json",
+		Version:   "13.0.3",
+		PURL:      "pkg:nuget/Newtonsoft.Json@13.0.3",
+		Locations: fixtureLocationSet,
+		Language:  pkg.Dotnet,
+		Type:      pkg.DotnetPkg,
+		Metadata: pkg.DotnetPackagesLockEntry{
+			Name:        "Newtonsoft.Json",
+			Version:     "13.0.3",
+			ContentHash: "newtonsoft1303hash==",
+			Type:        "Direct",
+		},
+	}
+
+	expectedPkgs := []pkg.Package{
+		myLibPkg,
+		newtonsoftPkg,
+		log4net1Pkg,
+		log4net2Pkg,
+	}
+
+	// the same package is resolved to a different version per target framework, so both edges must be captured
+	expectedRelationships := []artifact.Relationship{
+		{
+			From: log4net1Pkg,
+			To:   myLibPkg,
+			Type: artifact.DependencyOfRelationship,
+		},
+		{
+			From: log4net2Pkg,
+			To:   myLibPkg,
+			Type: artifact.DependencyOfRelationship,
+		},
+		{
+			From: newtonsoftPkg,
+			To:   myLibPkg,
+			Type: artifact.DependencyOfRelationship,
+		},
+	}
+
+	pkgtest.TestFileParser(t, fixture, parseDotnetPackagesLock, expectedPkgs, expectedRelationships)
+}

syft/pkg/cataloger/dotnet/testdata/packages.lock-multi-framework.json

@@ -0,0 +1,49 @@
+{
+    "version": 1,
+    "dependencies": {
+        "net8.0": {
+            "MyLib": {
+                "type": "Direct",
+                "requested": "[1.0.0, )",
+                "resolved": "1.0.0",
+                "contentHash": "mylibhash==",
+                "dependencies": {
+                    "log4net": "2.0.5"
+                }
+            },
+            "log4net": {
+                "type": "Transitive",
+                "resolved": "2.0.5",
+                "contentHash": "log4net205hash=="
+            },
+            "Newtonsoft.Json": {
+                "type": "Direct",
+                "requested": "[13.0.3, )",
+                "resolved": "13.0.3",
+                "contentHash": "newtonsoft1303hash=="
+            }
+        },
+        "netstandard2.0": {
+            "MyLib": {
+                "type": "Direct",
+                "requested": "[1.0.0, )",
+                "resolved": "1.0.0",
+                "contentHash": "mylibhash==",
+                "dependencies": {
+                    "log4net": "1.2.15",
+                    "Newtonsoft.Json": "13.0.3"
+                }
+            },
+            "log4net": {
+                "type": "Transitive",
+                "resolved": "1.2.15",
+                "contentHash": "log4net1215hash=="
+            },
+            "Newtonsoft.Json": {
+                "type": "Transitive",
+                "resolved": "13.0.3",
+                "contentHash": "newtonsoft1303hash=="
+            }
+        }
+    }
+}