[wip] add more tests for resolvers around symlinks

Signed-off-by: Alex Goodman <[email protected]>

syft/file/all_regular_files.go

@@ -7,24 +7,26 @@ import (
 
 func allRegularFiles(resolver source.FileResolver) (locations []source.Location) {
 	for location := range resolver.AllLocations() {
-		resolvedLocations, err := resolver.FilesByPath(location.RealPath)
+
+		metadata, err := resolver.FileMetadataByLocation(location)
 		if err != nil {
-			log.Warnf("unable to resolve %+v: %+v", location, err)
+			log.Warnf("unable to get metadata for %+v: %+v", location, err)
 			continue
 		}
 
-		for _, resolvedLocation := range resolvedLocations {
-			metadata, err := resolver.FileMetadataByLocation(resolvedLocation)
-			if err != nil {
-				log.Warnf("unable to get metadata for %+v: %+v", location, err)
-				continue
-			}
+		// filter out anything that is not a regular file. Why not evaluate symlinks here? All symlinks resolve to
+		// either a) another path with a file/dir or b) nothing. Any other existing path will already be returned
+		// from resolver.AllLocations().
 
-			if metadata.Type != source.RegularFile {
-				continue
-			}
-			locations = append(locations, resolvedLocation)
+		// TODO: a challenge for the future: can we allow for symlink resolution here for consumers that need to observe file nodes with the virtual paths intact?
+		// I tried this out but ran into a problem with the directory resolver; the requestPath() call misinterprets the real input path as if it's from
+		// the root of the resolver directory.
+		if metadata.Type != source.RegularFile {
+			continue
 		}
+
+		locations = append(locations, location)
+
 	}
 	return locations
 }

syft/file/classification_cataloger_test.go

@@ -89,7 +89,7 @@ func TestClassifierCataloger_DefaultClassifiers_PositiveCases(t *testing.T) {
 		{
 			name:       "positive-busybox",
 			fixtureDir: "test-fixtures/classifiers/positive",
-			location:   "busybox",
+			location:   "[", // note: busybox is a link to [
 			expected: []Classification{
 				{
 					Class: "busybox-binary",
@@ -120,10 +120,10 @@ func TestClassifierCataloger_DefaultClassifiers_PositiveCases(t *testing.T) {
 			loc := source.NewLocation(test.location)
 
 			ok := false
-			for actual_loc, actual_classification := range actualResults {
-				if loc.RealPath == actual_loc.RealPath {
+			for actualLoc, actualClassification := range actualResults {
+				if loc.RealPath == actualLoc.RealPath {
 					ok = true
-					assert.Equal(t, test.expected, actual_classification)
+					assert.Equal(t, test.expected, actualClassification)
 				}
 			}
 
@@ -146,7 +146,7 @@ func TestClassifierCataloger_DefaultClassifiers_PositiveCases_Image(t *testing.T
 		{
 			name:         "busybox-regression",
 			fixtureImage: "image-busybox",
-			location:     "/bin/busybox",
+			location:     "/bin/[",
 			expected: []Classification{
 				{
 					Class: "busybox-binary",
@@ -178,10 +178,10 @@ func TestClassifierCataloger_DefaultClassifiers_PositiveCases_Image(t *testing.T
 			loc := source.NewLocation(test.location)
 
 			ok := false
-			for actual_loc, actual_classification := range actualResults {
-				if loc.RealPath == actual_loc.RealPath {
+			for actuaLoc, actualClassification := range actualResults {
+				if loc.RealPath == actuaLoc.RealPath {
 					ok = true
-					assert.Equal(t, test.expected, actual_classification)
+					assert.Equal(t, test.expected, actualClassification)
 				}
 			}
 

syft/file/classifier.go

@@ -52,7 +52,8 @@ var DefaultClassifiers = []Classifier{
 	{
 		Class: "busybox-binary",
 		FilepathPatterns: []*regexp.Regexp{
-			regexp.MustCompile(`(.*/|^)busybox$`),
+			// we match on either files called "busybox" or "[", which busybox tends to link to (at least in the busybox image)
+			regexp.MustCompile(`(.*/|^)(busybox|\[)$`),
 		},
 		EvidencePatternTemplates: []string{
 			`(?m)BusyBox\s+v(?P<version>[0-9]+\.[0-9]+\.[0-9]+)`,

syft/source/all_layers_resolver.go

@@ -126,7 +126,7 @@ func (r *allLayersResolver) FilesByGlob(patterns ...string) ([]Location, error)
 
 	for _, pattern := range patterns {
 		for idx, layerIdx := range r.layers {
-			results, err := r.img.Layers[layerIdx].Tree.FilesByGlob(pattern, filetree.DoNotFollowDeadBasenameLinks)
+			results, err := r.img.Layers[layerIdx].Tree.FilesByGlob(pattern, filetree.FollowBasenameLinks, filetree.DoNotFollowDeadBasenameLinks)
 			if err != nil {
 				return nil, fmt.Errorf("failed to resolve files by glob (%s): %w", pattern, err)
 			}

syft/source/all_layers_resolver_test.go

@@ -360,3 +360,184 @@ func TestAllLayersImageResolver_FilesContents(t *testing.T) {
 		})
 	}
 }
+
+func Test_imageAllLayersResolver_resolvesLinks(t *testing.T) {
+	tests := []struct {
+		name     string
+		runner   func(FileResolver) []Location
+		expected []Location
+	}{
+		{
+			name: "by mimetype",
+			runner: func(resolver FileResolver) []Location {
+				// links should not show up when searching mimetype
+				actualLocations, err := resolver.FilesByMIMEType("text/plain")
+				assert.NoError(t, err)
+				return actualLocations
+			},
+			expected: []Location{
+				{
+					Coordinates: Coordinates{
+						RealPath: "/etc/group",
+					},
+					VirtualPath: "/etc/group",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/etc/passwd",
+					},
+					VirtualPath: "/etc/passwd",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/etc/shadow",
+					},
+					VirtualPath: "/etc/shadow",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-1.txt",
+					},
+					VirtualPath: "/file-1.txt",
+				},
+				// copy 1
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/file-2.txt",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-3.txt",
+					},
+					VirtualPath: "/file-3.txt",
+				},
+				// copy 2
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/file-2.txt",
+				},
+				// copy 1
+				{
+					Coordinates: Coordinates{
+						RealPath: "/parent/file-4.txt",
+					},
+					VirtualPath: "/parent/file-4.txt",
+				},
+				// copy 2
+				{
+					Coordinates: Coordinates{
+						RealPath: "/parent/file-4.txt",
+					},
+					VirtualPath: "/parent/file-4.txt",
+				},
+			},
+		},
+		{
+			name: "by glob",
+			runner: func(resolver FileResolver) []Location {
+				// links are searched, but resolve to the real files
+				actualLocations, err := resolver.FilesByGlob("*ink-*")
+				assert.NoError(t, err)
+				return actualLocations
+			},
+			expected: []Location{
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-1.txt",
+					},
+					VirtualPath: "/link-1",
+				},
+				// copy 1
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-2",
+				},
+				// copy 2
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-2",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-3.txt",
+					},
+					VirtualPath: "/link-within",
+				},
+			},
+		},
+		{
+			name: "by path to degree 1 link",
+			runner: func(resolver FileResolver) []Location {
+				// links resolve to the final file
+				actualLocations, err := resolver.FilesByPath("/link-2")
+				assert.NoError(t, err)
+				return actualLocations
+			},
+			expected: []Location{
+				// we have multiple copies across layers
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-2",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-2",
+				},
+			},
+		},
+		{
+			name: "by path to degree 2 link",
+			runner: func(resolver FileResolver) []Location {
+				// multiple links resolves to the final file
+				actualLocations, err := resolver.FilesByPath("/link-indirect")
+				assert.NoError(t, err)
+				return actualLocations
+			},
+			expected: []Location{
+				// we have multiple copies across layers
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-indirect",
+				},
+				{
+					Coordinates: Coordinates{
+						RealPath: "/file-2.txt",
+					},
+					VirtualPath: "/link-indirect",
+				},
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+
+			img := imagetest.GetFixtureImage(t, "docker-archive", "image-symlinks")
+
+			resolver, err := newAllLayersResolver(img)
+			assert.NoError(t, err)
+
+			actualLocations := test.runner(resolver)
+			assert.Len(t, actualLocations, len(test.expected))
+			for i, actual := range actualLocations {
+				assert.Equal(t, test.expected[i].RealPath, actual.RealPath)
+				assert.Equal(t, test.expected[i].VirtualPath, actual.VirtualPath)
+			}
+		})
+	}
+
+}

syft/source/directory_resolver.go

@@ -341,13 +341,13 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error)
 			evaluatedPath = windowsToPosix(evaluatedPath)
 		}
 
-		exists, ref, err := r.fileTree.File(file.Path(userStrPath))
+		exists, ref, err := r.fileTree.File(file.Path(userStrPath), filetree.FollowBasenameLinks)
 		if err == nil && exists {
-			loc := NewLocationFromDirectory(r.responsePath(userStrPath), *ref)
-			if evaluatedPath != userStrPath {
-				loc.VirtualPath = r.responsePath(evaluatedPath)
-			}
-
+			loc := NewVirtualLocationFromDirectory(
+				r.responsePath(string(ref.RealPath)), // the actual path relative to the resolver root
+				r.responsePath(userStrPath),          // the path used to access this file, relative to the resolver root
+				*ref,
+			)
 			references = append(references, loc)
 		}
 	}
@@ -360,12 +360,17 @@ func (r directoryResolver) FilesByGlob(patterns ...string) ([]Location, error) {
 	result := make([]Location, 0)
 
 	for _, pattern := range patterns {
-		globResults, err := r.fileTree.FilesByGlob(pattern)
+		globResults, err := r.fileTree.FilesByGlob(pattern, filetree.FollowBasenameLinks)
 		if err != nil {
 			return nil, err
 		}
 		for _, globResult := range globResults {
-			result = append(result, NewLocationFromDirectory(r.responsePath(string(globResult.MatchPath)), globResult.Reference))
+			loc := NewVirtualLocationFromDirectory(
+				r.responsePath(string(globResult.Reference.RealPath)), // the actual path relative to the resolver root
+				r.responsePath(string(globResult.MatchPath)),          // the path used to access this file, relative to the resolver root
+				globResult.Reference,
+			)
+			result = append(result, loc)
 		}
 	}