ignore target link files based on path (#667)

* ignore target link files based on path log when files are actually indexed add test for sym link resolution golang test nits Signed-off-by: Jonas Galvão Xavier <jonas.agx@gmail.com> * nil catalog should act like an empty catalog Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * remove dir path filtering in favor of file type filtering Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * split out addPathToIndex into specialized functions Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * add test for nul catalog enumeration Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * conditionally discover MIME types for file based on file resolver index Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * change logging around cataloging Signed-off-by: Alex Goodman <alex.goodman@anchore.com> * add tests to cover possible infinite symlink loop for resolver Signed-off-by: Alex Goodman <alex.goodman@anchore.com> Co-authored-by: Alex Goodman <alex.goodman@anchore.com>
2026-11-04 00:16:38 +01:00 · 2021-12-13 20:49:11 -05:00
parent 3a10ca7e6a
commit d3804d1a82
23 changed files with 623 additions and 225 deletions
@@ -1,14 +1,15 @@
 package source

 import (
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path"
 	"path/filepath"
 	"reflect"
 	"strings"
-	"syscall"
 	"testing"
+	"time"

 	"github.com/stretchr/testify/require"

@@ -19,6 +20,55 @@ import (
 	"github.com/wagoodman/go-progress"
 )

+func TestDirectoryResolver_FilesByPath_relativeRoot(t *testing.T) {
+	cases := []struct {
+		name         string
+		relativeRoot string
+		input        string
+		expected     []string
+	}{
+		{
+			name:         "should find a file from an absolute input",
+			relativeRoot: "./test-fixtures/",
+			input:        "/image-symlinks/file-1.txt",
+			expected: []string{
+				"image-symlinks/file-1.txt",
+			},
+		},
+		{
+			name:         "should find a file from a relative path",
+			relativeRoot: "./test-fixtures/",
+			input:        "image-symlinks/file-1.txt",
+			expected: []string{
+				"image-symlinks/file-1.txt",
+			},
+		},
+		{
+			name:         "should find a file from a relative path (root above cwd)",
+			relativeRoot: "../",
+			input:        "sbom/sbom.go",
+			expected: []string{
+				"sbom/sbom.go",
+			},
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			resolver, err := newDirectoryResolver(c.relativeRoot)
+			assert.NoError(t, err)
+
+			refs, err := resolver.FilesByPath(c.input)
+			require.NoError(t, err)
+			assert.Len(t, refs, len(c.expected))
+			s := strset.New()
+			for _, actual := range refs {
+				s.Add(actual.RealPath)
+			}
+			assert.ElementsMatch(t, c.expected, s.List())
+		})
+	}
+}
+
 func TestDirectoryResolver_FilesByPath_absoluteRoot(t *testing.T) {
 	cases := []struct {
 		name         string
@@ -85,23 +135,16 @@ func TestDirectoryResolver_FilesByPath(t *testing.T) {
 		{
 			name:     "finds a file (relative)",
 			root:     "./test-fixtures/",
-			input:    "test-fixtures/image-symlinks/file-1.txt",
-			expected: "test-fixtures/image-symlinks/file-1.txt",
+			input:    "image-symlinks/file-1.txt",
+			expected: "image-symlinks/file-1.txt",
 			refCount: 1,
 		},
 		{
 			name:     "finds a file with relative indirection",
 			root:     "./test-fixtures/../test-fixtures",
-			input:    "test-fixtures/image-symlinks/file-1.txt",
-			expected: "test-fixtures/image-symlinks/file-1.txt",
-			refCount: 1,
-		},
-		{
-			// note: this is asserting the old behavior is not supported
-			name:     "relative lookup with wrong path fails",
-			root:     "./test-fixtures/",
 			input:    "image-symlinks/file-1.txt",
-			refCount: 0,
+			expected: "image-symlinks/file-1.txt",
+			refCount: 1,
 		},
 		{
 			name:     "managed non-existing files (relative)",
@@ -113,7 +156,7 @@ func TestDirectoryResolver_FilesByPath(t *testing.T) {
 			name:     "finds a file (absolute)",
 			root:     "./test-fixtures/",
 			input:    "/image-symlinks/file-1.txt",
-			expected: "test-fixtures/image-symlinks/file-1.txt",
+			expected: "image-symlinks/file-1.txt",
 			refCount: 1,
 		},
 		{
@@ -158,17 +201,17 @@ func TestDirectoryResolver_MultipleFilesByPath(t *testing.T) {
 	}{
 		{
 			name:     "finds multiple files",
-			input:    []string{"test-fixtures/image-symlinks/file-1.txt", "test-fixtures/image-symlinks/file-2.txt"},
+			input:    []string{"image-symlinks/file-1.txt", "image-symlinks/file-2.txt"},
 			refCount: 2,
 		},
 		{
 			name:     "skips non-existing files",
-			input:    []string{"test-fixtures/image-symlinks/bogus.txt", "test-fixtures/image-symlinks/file-1.txt"},
+			input:    []string{"image-symlinks/bogus.txt", "image-symlinks/file-1.txt"},
 			refCount: 1,
 		},
 		{
 			name:     "does not return anything for non-existing directories",
-			input:    []string{"test-fixtures/non-existing/bogus.txt", "test-fixtures/non-existing/file-1.txt"},
+			input:    []string{"non-existing/bogus.txt", "non-existing/file-1.txt"},
 			refCount: 0,
 		},
 	}
@@ -210,7 +253,7 @@ func TestDirectoryResolver_FilesByGlobSingle(t *testing.T) {
 	assert.NoError(t, err)

 	assert.Len(t, refs, 1)
-	assert.Equal(t, "test-fixtures/image-symlinks/file-1.txt", refs[0].RealPath)
+	assert.Equal(t, "image-symlinks/file-1.txt", refs[0].RealPath)
 }

 func TestDirectoryResolverDoesNotIgnoreRelativeSystemPaths(t *testing.T) {
@@ -218,95 +261,128 @@ func TestDirectoryResolverDoesNotIgnoreRelativeSystemPaths(t *testing.T) {
 	resolver, err := newDirectoryResolver("test-fixtures/system_paths/target")
 	assert.NoError(t, err)
 	// ensure the correct filter function is wired up by default
-	expectedFn := reflect.ValueOf(isUnixSystemRuntimePath)
+	expectedFn := reflect.ValueOf(isUnallowableFileType)
 	actualFn := reflect.ValueOf(resolver.pathFilterFns[0])
 	assert.Equal(t, expectedFn.Pointer(), actualFn.Pointer())

 	// all paths should be found (non filtering matches a path)
-	refs, err := resolver.FilesByGlob("**/place")
+	locations, err := resolver.FilesByGlob("**/place")
 	assert.NoError(t, err)
 	// 4: within target/
 	// 1: target/link --> relative path to "place"
 	// 1: outside_root/link_target/place
-	assert.Len(t, refs, 6)
+	assert.Len(t, locations, 6)

 	// ensure that symlink indexing outside of root worked
+	testLocation := "test-fixtures/system_paths/outside_root/link_target/place"
 	ok := false
-	test_location := "test-fixtures/system_paths/outside_root/link_target/place"
-	for _, actual_loc := range refs {
-		if test_location == actual_loc.RealPath {
+	for _, location := range locations {
+		if strings.HasSuffix(location.RealPath, testLocation) {
 			ok = true
 		}
 	}

 	if !ok {
-		t.Fatalf("could not find test location=%q", test_location)
+		t.Fatalf("could not find test location=%q", testLocation)
 	}
 }

-func TestDirectoryResolverUsesPathFilterFunction(t *testing.T) {
-	// let's make certain that the index honors the filter function
-	filter := func(s string) bool {
-		// a dummy function that works for testing purposes
-		return strings.Contains(s, "dev/place") || strings.Contains(s, "proc/place") || strings.Contains(s, "sys/place")
-	}
+var _ fs.FileInfo = (*testFileInfo)(nil)

-	resolver, err := newDirectoryResolver("test-fixtures/system_paths/target", filter)
-	assert.NoError(t, err)
-
-	// ensure the correct filter function is wired up by default
-	expectedFn := reflect.ValueOf(filter)
-	actualFn := reflect.ValueOf(resolver.pathFilterFns[0])
-	assert.Equal(t, expectedFn.Pointer(), actualFn.Pointer())
-	assert.Len(t, resolver.pathFilterFns, 1)
-
-	refs, err := resolver.FilesByGlob("**/place")
-	assert.NoError(t, err)
-	// target/home/place + target/link/.../place + outside_root/.../place
-	assert.Len(t, refs, 3)
+type testFileInfo struct {
+	mode os.FileMode
 }

-func Test_isUnixSystemRuntimePath(t *testing.T) {
+func (t testFileInfo) Name() string {
+	panic("implement me")
+}
+
+func (t testFileInfo) Size() int64 {
+	panic("implement me")
+}
+
+func (t testFileInfo) Mode() fs.FileMode {
+	return t.mode
+}
+
+func (t testFileInfo) ModTime() time.Time {
+	panic("implement me")
+}
+
+func (t testFileInfo) IsDir() bool {
+	panic("implement me")
+}
+
+func (t testFileInfo) Sys() interface{} {
+	panic("implement me")
+}
+
+func Test_isUnallowableFileType(t *testing.T) {
 	tests := []struct {
-		path     string
+		name     string
+		info     os.FileInfo
 		expected bool
 	}{
 		{
-			path:     "proc/place",
+			name: "regular file",
+			info: testFileInfo{
+				mode: 0,
+			},
 			expected: false,
 		},
 		{
-			path:     "/proc/place",
-			expected: true,
-		},
-		{
-			path:     "/proc",
-			expected: true,
-		},
-		{
-			path:     "/pro/c",
+			name: "dir",
+			info: testFileInfo{
+				mode: os.ModeDir,
+			},
 			expected: false,
 		},
 		{
-			path:     "/pro",
+			name: "symlink",
+			info: testFileInfo{
+				mode: os.ModeSymlink,
+			},
 			expected: false,
 		},
 		{
-			path:     "/dev",
+			name: "socket",
+			info: testFileInfo{
+				mode: os.ModeSocket,
+			},
 			expected: true,
 		},
 		{
-			path:     "/sys",
+			name: "named pipe",
+			info: testFileInfo{
+				mode: os.ModeNamedPipe,
+			},
 			expected: true,
 		},
 		{
-			path:     "/something/sys",
-			expected: false,
+			name: "char device",
+			info: testFileInfo{
+				mode: os.ModeCharDevice,
+			},
+			expected: true,
+		},
+		{
+			name: "block device",
+			info: testFileInfo{
+				mode: os.ModeDevice,
+			},
+			expected: true,
+		},
+		{
+			name: "irregular",
+			info: testFileInfo{
+				mode: os.ModeIrregular,
+			},
+			expected: true,
 		},
 	}
 	for _, test := range tests {
-		t.Run(test.path, func(t *testing.T) {
-			assert.Equal(t, test.expected, isUnixSystemRuntimePath(test.path))
+		t.Run(test.name, func(t *testing.T) {
+			assert.Equal(t, test.expected, isUnallowableFileType("dont/care", test.info))
 		})
 	}
 }
@@ -345,9 +421,7 @@ func Test_directoryResolver_index(t *testing.T) {

 			// note: the index uses absolute paths, so assertions MUST keep this in mind
 			cwd, err := os.Getwd()
-			if err != nil {
-				t.Fatalf("could not get working dir: %+v", err)
-			}
+			require.NoError(t, err)

 			p := file.Path(path.Join(cwd, test.path))
 			assert.Equal(t, true, r.fileTree.HasPath(p))
@@ -365,32 +439,27 @@ func Test_handleFileAccessErr(t *testing.T) {
 	tests := []struct {
 		name                string
 		input               error
-		expectedErr         error
 		expectedPathTracked bool
 	}{
 		{
 			name:                "permission error does not propagate",
 			input:               os.ErrPermission,
 			expectedPathTracked: true,
-			expectedErr:         nil,
 		},
 		{
 			name:                "file does not exist error does not propagate",
 			input:               os.ErrNotExist,
 			expectedPathTracked: true,
-			expectedErr:         nil,
 		},
 		{
-			name:                "non-permission errors propagate",
+			name:                "non-permission errors are tracked",
 			input:               os.ErrInvalid,
-			expectedPathTracked: false,
-			expectedErr:         os.ErrInvalid,
+			expectedPathTracked: true,
 		},
 		{
 			name:                "non-errors ignored",
 			input:               nil,
 			expectedPathTracked: false,
-			expectedErr:         nil,
 		},
 	}

@@ -400,7 +469,7 @@ func Test_handleFileAccessErr(t *testing.T) {
 				errPaths: make(map[string]error),
 			}
 			p := "a/place"
-			assert.ErrorIs(t, r.handleFileAccessErr(p, test.input), test.expectedErr)
+			assert.Equal(t, r.isFileAccessErr(p, test.input), test.expectedPathTracked)
 			_, exists := r.errPaths[p]
 			assert.Equal(t, test.expectedPathTracked, exists)
 		})
@@ -493,7 +562,7 @@ func Test_directoryResolver_FilesByMIMEType(t *testing.T) {
 		{
 			fixturePath:   "./test-fixtures/image-simple",
 			mimeType:      "text/plain",
-			expectedPaths: strset.New("test-fixtures/image-simple/file-1.txt", "test-fixtures/image-simple/file-2.txt", "test-fixtures/image-simple/target/really/nested/file-3.txt", "test-fixtures/image-simple/Dockerfile"),
+			expectedPaths: strset.New("file-1.txt", "file-2.txt", "target/really/nested/file-3.txt", "Dockerfile"),
 		},
 	}
 	for _, test := range tests {
@@ -511,33 +580,81 @@ func Test_directoryResolver_FilesByMIMEType(t *testing.T) {
 	}
 }

-func Test_ignoreIrregularFiles(t *testing.T) {
-	// NOTE: craeting a pipe/fifo file on demand since git doesn't let me
-	// commit one. The directory resolver should ignore it (and any other
-	// file that is neither regular, a symlink or directory) when indexing a directory
-	dir := "./test-fixtures/irregular-files"
-	f := "f.fifo"
-
-	err := syscall.Mknod(filepath.Join(dir, f), syscall.S_IFIFO|0666, 0)
-	assert.NoError(t, err)
-	defer func() {
-		err := os.Remove(filepath.Join(dir, f))
-		assert.NoError(t, err)
-	}()
-
-	fileRefs, err := ioutil.ReadDir(dir)
-	assert.NoError(t, err)
-	assert.Len(t, fileRefs, 2) // two files (f.fifo and readme)
-
-	resolver, err := newDirectoryResolver(dir)
+func Test_IndexingNestedSymLinks(t *testing.T) {
+	resolver, err := newDirectoryResolver("./test-fixtures/symlinks-simple")
 	assert.NoError(t, err)

-	assert.Len(t, resolver.fileTree.AllFiles(), 1)
-	rp := resolver.fileTree.AllFiles()[0].RealPath
-	assert.True(t, strings.Contains(string(rp), filepath.Join(dir, "readme")))
+	// check that we can get the real path
+	locations, err := resolver.FilesByPath("./readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+
+	// check that we can access the same file via 1 symlink
+	locations, err = resolver.FilesByPath("./link_to_new_readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+
+	// check that we can access the same file via 2 symlinks
+	locations, err = resolver.FilesByPath("./link_to_link_to_new_readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+}
+
+func Test_IndexingNestedSymLinks_ignoredIndexes(t *testing.T) {
+	filterFn := func(path string, _ os.FileInfo) bool {
+		return strings.HasSuffix(path, string(filepath.Separator)+"readme")
+	}
+
+	resolver, err := newDirectoryResolver("./test-fixtures/symlinks-simple", filterFn)
+	assert.NoError(t, err)
+
+	var testingLocations []Location
+
+	// the path to the real file is PRUNED from the index, so we should NOT expect a location returned
+	locations, err := resolver.FilesByPath("./readme")
+	require.NoError(t, err)
+	assert.Empty(t, locations)
+
+	// check that we can access the same file via 1 symlink
+	locations, err = resolver.FilesByPath("./link_to_new_readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+	testingLocations = append(testingLocations, locations...)
+
+	// check that we can access the same file via 2 symlinks
+	locations, err = resolver.FilesByPath("./link_to_link_to_new_readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+	testingLocations = append(testingLocations, locations...)
+
+	// check that we CANNOT get contents from any of the link locations
+	for _, location := range testingLocations {
+		contentReader, err := resolver.FileContentsByLocation(location)
+		assert.Errorf(t, err, "expected an error for getting content from a location not in the index")
+		assert.Nil(t, contentReader)
+	}
+}
+
+func Test_IndexingNestedSymLinksOutsideOfRoot(t *testing.T) {
+	resolver, err := newDirectoryResolver("./test-fixtures/symlinks-roots/root")
+	assert.NoError(t, err)
+
+	// check that we can get the real path
+	locations, err := resolver.FilesByPath("./readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+
+	// check that we can access the same file via 2 symlinks (link_to_link_to_readme -> link_to_readme -> readme)
+	locations, err = resolver.FilesByPath("./link_to_link_to_readme")
+	require.NoError(t, err)
+	assert.Len(t, locations, 1)
+
 }

 func Test_directoryResolver_FileContentsByLocation(t *testing.T) {
+	cwd, err := os.Getwd()
+	require.NoError(t, err)
+
 	tests := []struct {
 		name     string
 		location Location
@@ -547,7 +664,7 @@ func Test_directoryResolver_FileContentsByLocation(t *testing.T) {
 		{
 			name: "use file reference for content requests",
 			location: NewLocationFromDirectory("some/place", file.Reference{
-				RealPath: "test-fixtures/image-simple/file-1.txt",
+				RealPath: file.Path(filepath.Join(cwd, "test-fixtures/image-simple/file-1.txt")),
 			}),
 			expects: "this file has contents",
 		},
@@ -578,3 +695,79 @@ func Test_directoryResolver_FileContentsByLocation(t *testing.T) {
 		})
 	}
 }
+
+func Test_isUnixSystemRuntimePath(t *testing.T) {
+	tests := []struct {
+		path     string
+		expected bool
+	}{
+		{
+			path:     "proc/place",
+			expected: false,
+		},
+		{
+			path:     "/proc/place",
+			expected: true,
+		},
+		{
+			path:     "/proc",
+			expected: true,
+		},
+		{
+			path:     "/pro/c",
+			expected: false,
+		},
+		{
+			path:     "/pro",
+			expected: false,
+		},
+		{
+			path:     "/dev",
+			expected: true,
+		},
+		{
+			path:     "/sys",
+			expected: true,
+		},
+		{
+			path:     "/something/sys",
+			expected: false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.path, func(t *testing.T) {
+			assert.Equal(t, test.expected, isUnixSystemRuntimePath(test.path, nil))
+		})
+	}
+}
+
+func Test_SymlinkLoopWithGlobsShouldResolve(t *testing.T) {
+	test := func(t *testing.T) {
+		resolver, err := newDirectoryResolver("./test-fixtures/symlinks-loop")
+		require.NoError(t, err)
+
+		locations, err := resolver.FilesByGlob("**/file.target")
+		require.NoError(t, err)
+		// Note: I'm not certain that this behavior is correct, but it is not an infinite loop (which is the point of the test)
+		// - block/loop0/file.target
+		// - devices/loop0/file.target
+		// - devices/loop0/subsystem/loop0/file.target
+		assert.Len(t, locations, 3)
+	}
+
+	testWithTimeout(t, 5*time.Second, test)
+}
+
+func testWithTimeout(t *testing.T, timeout time.Duration, test func(*testing.T)) {
+	done := make(chan bool)
+	go func() {
+		test(t)
+		done <- true
+	}()
+
+	select {
+	case <-time.After(timeout):
+		t.Fatal("test timed out")
+	case <-done:
+	}
+}