fix: correctly report supporting evidence for binary packages (#4558)

Signed-off-by: Keith Zantow <kzantow@gmail.com>

syft/pkg/cataloger/binary/capabilities.yaml

@@ -109,16 +109,6 @@ catalogers:
             cpes:
               - cpe:2.3:a:nodejs:node.js:*:*:*:*:*:*:*:*
             type: BinaryPkg
-      - method: glob
-        criteria:
-          - '**/VERSION*'
-        packages:
-          - class: go-binary-hint
-            name: go
-            purl: pkg:generic/go
-            cpes:
-              - cpe:2.3:a:golang:go:*:*:*:*:*:*:*:*
-            type: BinaryPkg
       - method: glob
         criteria:
           - '**/busybox'

syft/pkg/cataloger/binary/classifier_cataloger.go

@@ -64,6 +64,10 @@ func (c cataloger) Catalog(_ context.Context, resolver file.Resolver) ([]pkg.Pac
 	var relationships []artifact.Relationship
 	var errs error
 
+	// we do not run these classifiers in parallel currently because: when determining primary vs. supporting evidence,
+	// we take preference to the classifier that was defined first, if we modify this to run in parallel,
+	// we need to retain this behavior by giving precedence to the classifier defined first in the c.classifiers list;
+	// if this is ever made parallel, we will need to account for this to have deterministic behavior
 	for _, cls := range c.classifiers {
 		log.WithFields("classifier", cls.Class).Trace("cataloging binaries")
 		newPkgs, err := catalog(resolver, cls)
@@ -101,8 +105,26 @@ func mergePackages(target *pkg.Package, extra *pkg.Package) {
 	if extra.Type != pkg.BinaryPkg && target.Type == pkg.BinaryPkg {
 		target.Type = extra.Type
 	}
-	// add the locations
-	target.Locations.Add(extra.Locations.ToSlice()...)
+	addedEvidence := false
+	// when merging locations together, we need to maintain primary vs. supporting evidence -
+	// if we are merging two packages together that have overlapping evidence, e.g. libpython
+	// which are found by 2 different classifiers, we want to deduplicate evidence for the same
+	// locations when merging so libpython is not considered primary evidence. This allows cases
+	// where we find python binary with version info coming from libpython to deduplicate the libpython
+	// entries, but also surface results for libpython separately as primary evidence when there is
+	// no python binary referencing it
+	for _, location := range extra.Locations.ToSlice() {
+		// if we already have the same location, don't include duplicates
+		if target.Locations.Contains(location) {
+			continue
+		}
+		addedEvidence = true
+		target.Locations.Add(location)
+	}
+	// only include the additional metadata if we added evidence, as it was likely duplicated e.g. libpython
+	if !addedEvidence {
+		return
+	}
 	// update the metadata to indicate which classifiers were used
 	meta, _ := target.Metadata.(pkg.BinarySignature)
 	if m, ok := extra.Metadata.(pkg.BinarySignature); ok {

syft/pkg/cataloger/binary/classifier_cataloger_test.go

@@ -6,6 +6,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"slices"
 	"strings"
 	"testing"
 
@@ -547,6 +548,7 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 			},
 		},
 		{
+			// no python binary, but we find libpython, which is surfaced as primary evidence
 			logicalFixture: "python-shared-lib/3.7.4/linux-amd64",
 			expected: pkg.Package{
 				Name:      "python",
@@ -556,7 +558,6 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 				Metadata:  metadata("python-binary-lib"),
 			},
 		},
-
 		{
 			// note: dynamic (non-snippet) test case
 			logicalFixture: "python-slim-shared-libs/3.11/linux-amd64",
@@ -569,7 +570,6 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 					Matches: []pkg.ClassifierMatch{
 						match("python-binary", "python3.11"),
 						match("python-binary", "libpython3.11.so.1.0"),
-						match("python-binary-lib", "libpython3.11.so.1.0"),
 					},
 				},
 			},
@@ -586,7 +586,6 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 					Matches: []pkg.ClassifierMatch{
 						match("python-binary", "python3.9"),
 						match("python-binary", "libpython3.9.so.1.0"),
-						match("python-binary-lib", "libpython3.9.so.1.0"),
 					},
 				},
 			},
@@ -618,7 +617,6 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 					Matches: []pkg.ClassifierMatch{
 						match("python-binary", "python3.4"),
 						match("python-binary", "libpython3.4m.so.1.0"),
-						match("python-binary-lib", "libpython3.4m.so.1.0"),
 					},
 				},
 			},
@@ -734,8 +732,8 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 				Name:      "go",
 				Version:   "1.15",
 				PURL:      "pkg:generic/go@1.15",
-				Locations: locations("VERSION"),
-				Metadata:  metadata("go-binary-hint"),
+				Locations: locations("bin/go", "VERSION"),
+				Metadata:  metadata("go-binary"),
 			},
 		},
 		{
@@ -745,8 +743,8 @@ func Test_Cataloger_PositiveCases(t *testing.T) {
 				Name:      "go",
 				Version:   "1.25-d524e1e",
 				PURL:      "pkg:generic/go@1.25-d524e1e",
-				Locations: locations("VERSION.cache"),
-				Metadata:  metadata("go-binary-hint"),
+				Locations: locations("bin/go", "VERSION.cache"),
+				Metadata:  metadata("go-binary"),
 			},
 		},
 		{
@@ -1823,17 +1821,6 @@ func Test_Cataloger_DefaultClassifiers_PositiveCases_Image(t *testing.T) {
 			require.NoError(t, err)
 
 			for _, p := range packages {
-				expectedLocations := test.expected.Locations.ToSlice()
-				gotLocations := p.Locations.ToSlice()
-				require.Len(t, gotLocations, len(expectedLocations))
-
-				for i, expectedLocation := range expectedLocations {
-					gotLocation := gotLocations[i]
-					if expectedLocation.RealPath != gotLocation.RealPath {
-						t.Fatalf("locations do not match; expected: %v got: %v", expectedLocations, gotLocations)
-					}
-				}
-
 				assertPackagesAreEqual(t, test.expected, p)
 			}
 		})
@@ -2023,12 +2010,13 @@ func assertPackagesAreEqual(t *testing.T, expected pkg.Package, p pkg.Package) {
 	gotLocations := p.Locations.ToSlice()
 
 	if len(expectedLocations) != len(gotLocations) {
-		failMessages = append(failMessages, "locations are not equal length")
+		failMessages = append(failMessages, fmt.Sprintf("locations are not equal: %v != %v", expectedLocations, gotLocations))
 	} else {
-		for i, expectedLocation := range expectedLocations {
-			gotLocation := gotLocations[i]
-			if expectedLocation.RealPath != gotLocation.RealPath {
-				failMessages = append(failMessages, fmt.Sprintf("locations do not match; expected: %v got: %v", expectedLocation.RealPath, gotLocation.RealPath))
+		for _, expectedLocation := range expectedLocations {
+			if !slices.ContainsFunc(gotLocations, func(gotLocation file.Location) bool {
+				return gotLocation.RealPath == expectedLocation.RealPath
+			}) {
+				failMessages = append(failMessages, fmt.Sprintf("location not found; expected: %v in set: %v", expectedLocation.RealPath, gotLocations))
 			}
 		}
 	}

syft/pkg/cataloger/binary/classifiers.go

@@ -71,8 +71,14 @@ func DefaultClassifiers() []binutils.Classifier {
 		{
 			Class:    "go-binary",
 			FileGlob: "**/go",
-			EvidenceMatcher: m.FileContentsVersionMatcher(
-				`(?m)go(?P<version>[0-9]+\.[0-9]+(\.[0-9]+|beta[0-9]+|alpha[0-9]+|rc[0-9]+)?)\x00`),
+			EvidenceMatcher: binutils.MatchAny(
+				m.FileContentsVersionMatcher(
+					`(?m)go(?P<version>[0-9]+\.[0-9]+(\.[0-9]+|beta[0-9]+|alpha[0-9]+|rc[0-9]+)?)\x00`),
+				binutils.SupportingEvidenceMatcher("../VERSION*",
+					m.FileContentsVersionMatcher(
+						`(?m)go(?P<version>[0-9]+\.[0-9]+(\.[0-9]+|beta[0-9]+|alpha[0-9]+|rc[0-9]+|-[_0-9a-z]+)?)\s`),
+				),
+			),
 			Package: "go",
 			PURL:    mustPURL("pkg:generic/go@version"),
 			CPEs:    singleCPE("cpe:2.3:a:golang:go:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
@@ -142,15 +148,6 @@ func DefaultClassifiers() []binutils.Classifier {
 			PURL:    mustPURL("pkg:generic/node@version"),
 			CPEs:    singleCPE("cpe:2.3:a:nodejs:node.js:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
 		},
-		{
-			Class:    "go-binary-hint",
-			FileGlob: "**/VERSION*",
-			EvidenceMatcher: m.FileContentsVersionMatcher(
-				`(?m)go(?P<version>[0-9]+\.[0-9]+(\.[0-9]+|beta[0-9]+|alpha[0-9]+|rc[0-9]+)?(-[0-9a-f]{7})?)`),
-			Package: "go",
-			PURL:    mustPURL("pkg:generic/go@version"),
-			CPEs:    singleCPE("cpe:2.3:a:golang:go:*:*:*:*:*:*:*:*", cpe.NVDDictionaryLookupSource),
-		},
 		{
 			Class:    "busybox-binary",
 			FileGlob: "**/busybox",

syft/pkg/cataloger/binary/test-fixtures/classifiers/snippets/go-version-hint/1.15/any/VERSION

@@ -1 +1 @@
-go1.15-beta2
+go1.15 Fri 2003

syft/pkg/cataloger/binary/test-fixtures/classifiers/snippets/go-version-hint/1.15/any/bin/go

@@ -0,0 +1 @@
+no version in this binary

syft/pkg/cataloger/binary/test-fixtures/classifiers/snippets/go-version-hint/1.25/any/bin/go

@@ -0,0 +1 @@
+no version in this binary