From 04fc5e2a7fd43186aed824535748975820e6e79f Mon Sep 17 00:00:00 2001 From: Joseph Palermo Date: Fri, 23 Feb 2024 20:33:08 -0800 Subject: [PATCH] Revert "fix considering base path when ignoring known bad unix paths (#2644)" This change caused the skipping of any paths including "/proc", "/dev" and "/sys" which was not the intended change and excludes many things that should be scanned. https://github.com/anchore/syft/issues/2667 This reverts commit a909e3cec92da4cc68c95e72f860d233ae0531a2. Signed-off-by: Joseph Palermo --- .../fileresolver/directory_indexer.go | 55 +++--------- .../fileresolver/directory_indexer_test.go | 85 +------------------ syft/internal/fileresolver/directory_test.go | 6 +- syft/source/directory_source.go | 2 +- syft/source/directory_source_test.go | 2 +- syft/source/file_source.go | 2 +- 6 files changed, 21 insertions(+), 131 deletions(-) diff --git a/syft/internal/fileresolver/directory_indexer.go b/syft/internal/fileresolver/directory_indexer.go index d5f5956f5e0..8cb52634acc 100644 --- a/syft/internal/fileresolver/directory_indexer.go +++ b/syft/internal/fileresolver/directory_indexer.go @@ -21,7 +21,7 @@ import ( "github.com/anchore/syft/syft/internal/windows" ) -type PathIndexVisitor func(string, string, os.FileInfo, error) error +type PathIndexVisitor func(string, os.FileInfo, error) error type directoryIndexer struct { path string @@ -246,14 +246,14 @@ func allContainedPaths(p string) []string { return all } -func (r *directoryIndexer) indexPath(givenPath string, info os.FileInfo, err error) (string, error) { +func (r *directoryIndexer) indexPath(path string, info os.FileInfo, err error) (string, error) { // ignore any path which a filter function returns true for _, filterFn := range r.pathIndexVisitors { if filterFn == nil { continue } - if filterErr := filterFn(r.base, givenPath, info, err); filterErr != nil { + if filterErr := filterFn(path, info, err); filterErr != nil { if errors.Is(filterErr, fs.SkipDir) { // signal to walk() to skip this directory entirely (even if we're processing a file) return "", filterErr @@ -265,24 +265,24 @@ func (r *directoryIndexer) indexPath(givenPath string, info os.FileInfo, err err if info == nil { // walk may not be able to provide a FileInfo object, don't allow for this to stop indexing; keep track of the paths and continue. - r.errPaths[givenPath] = fmt.Errorf("no file info observable at path=%q", givenPath) + r.errPaths[path] = fmt.Errorf("no file info observable at path=%q", path) return "", nil } // here we check to see if we need to normalize paths to posix on the way in coming from windows if windows.HostRunningOnWindows() { - givenPath = windows.ToPosix(givenPath) + path = windows.ToPosix(path) } - newRoot, err := r.addPathToIndex(givenPath, info) - if r.isFileAccessErr(givenPath, err) { + newRoot, err := r.addPathToIndex(path, info) + if r.isFileAccessErr(path, err) { return "", nil } return newRoot, nil } -func (r *directoryIndexer) disallowFileAccessErr(_, path string, _ os.FileInfo, err error) error { +func (r *directoryIndexer) disallowFileAccessErr(path string, _ os.FileInfo, err error) error { if r.isFileAccessErr(path, err) { return ErrSkipPath } @@ -429,7 +429,7 @@ func (r directoryIndexer) hasBeenIndexed(p string) (bool, *file.Metadata) { return true, &entry.Metadata } -func (r *directoryIndexer) disallowRevisitingVisitor(_, path string, _ os.FileInfo, _ error) error { +func (r *directoryIndexer) disallowRevisitingVisitor(path string, _ os.FileInfo, _ error) error { // this prevents visiting: // - link destinations twice, once for the real file and another through the virtual path // - infinite link cycles @@ -443,17 +443,14 @@ func (r *directoryIndexer) disallowRevisitingVisitor(_, path string, _ os.FileIn return nil } -func disallowUnixSystemRuntimePath(base, path string, _ os.FileInfo, _ error) error { - // note: we need to consider all paths relative to the base when being filtered, which is how they would appear - // when the resolver is being used. Then something like /some/mountpoint/dev with a base of /some/mountpoint - // would be considered as /dev when being filtered. - if internal.HasAnyOfPrefixes(relativePath(base, path), unixSystemRuntimePrefixes...) { +func disallowUnixSystemRuntimePath(path string, _ os.FileInfo, _ error) error { + if internal.HasAnyOfPrefixes(path, unixSystemRuntimePrefixes...) { return fs.SkipDir } return nil } -func disallowByFileType(_, _ string, info os.FileInfo, _ error) error { +func disallowByFileType(_ string, info os.FileInfo, _ error) error { if info == nil { // we can't filter out by filetype for non-existent files return nil @@ -468,39 +465,13 @@ func disallowByFileType(_, _ string, info os.FileInfo, _ error) error { return nil } -func requireFileInfo(_, _ string, info os.FileInfo, _ error) error { +func requireFileInfo(_ string, info os.FileInfo, _ error) error { if info == nil { return ErrSkipPath } return nil } -func relativePath(basePath, givenPath string) string { - var relPath string - var relErr error - - if basePath != "" { - relPath, relErr = filepath.Rel(basePath, givenPath) - cleanPath := filepath.Clean(relPath) - if relErr == nil { - if cleanPath == "." { - relPath = string(filepath.Separator) - } else { - relPath = cleanPath - } - } - if !filepath.IsAbs(relPath) { - relPath = string(filepath.Separator) + relPath - } - } - - if relErr != nil || basePath == "" { - relPath = givenPath - } - - return relPath -} - func indexingProgress(path string) (*progress.Stage, *progress.Manual) { stage := &progress.Stage{} prog := progress.NewManual(-1) diff --git a/syft/internal/fileresolver/directory_indexer_test.go b/syft/internal/fileresolver/directory_indexer_test.go index 9e677423587..e2ef1b2017f 100644 --- a/syft/internal/fileresolver/directory_indexer_test.go +++ b/syft/internal/fileresolver/directory_indexer_test.go @@ -135,7 +135,7 @@ func TestDirectoryIndexer_handleFileAccessErr(t *testing.T) { } func TestDirectoryIndexer_IncludeRootPathInIndex(t *testing.T) { - filterFn := func(_, path string, _ os.FileInfo, _ error) error { + filterFn := func(path string, _ os.FileInfo, _ error) error { if path != "/" { return fs.SkipDir } @@ -236,7 +236,7 @@ func TestDirectoryIndexer_index_survive_badSymlink(t *testing.T) { func TestDirectoryIndexer_SkipsAlreadyVisitedLinkDestinations(t *testing.T) { var observedPaths []string - pathObserver := func(_, p string, _ os.FileInfo, _ error) error { + pathObserver := func(p string, _ os.FileInfo, _ error) error { fields := strings.Split(p, "test-fixtures/symlinks-prune-indexing") if len(fields) < 2 { return nil @@ -395,84 +395,3 @@ func Test_allContainedPaths(t *testing.T) { }) } } - -func Test_relativePath(t *testing.T) { - tests := []struct { - name string - basePath string - givenPath string - want string - }{ - { - name: "root: same relative path", - basePath: "a/b/c", - givenPath: "a/b/c", - want: "/", - }, - { - name: "root: same absolute path", - basePath: "/a/b/c", - givenPath: "/a/b/c", - want: "/", - }, - { - name: "contained path: relative", - basePath: "a/b/c", - givenPath: "a/b/c/dev", - want: "/dev", - }, - { - name: "contained path: absolute", - basePath: "/a/b/c", - givenPath: "/a/b/c/dev", - want: "/dev", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.want, relativePath(tt.basePath, tt.givenPath)) - }) - } -} - -func Test_disallowUnixSystemRuntimePath(t *testing.T) { - tests := []struct { - name string - base string - path string - wantErr require.ErrorAssertionFunc - }{ - { - name: "no base, matching path", - base: "", - path: "/dev", - wantErr: require.Error, - }, - { - name: "no base, non-matching path", - base: "", - path: "/not-dev", - wantErr: require.NoError, - }, - { - name: "base, matching path", - base: "/a/b/c", - path: "/a/b/c/dev", - wantErr: require.Error, - }, - { - name: "base, non-matching path due to bad relative alignment", - base: "/a/b/c", - path: "/dev", - wantErr: require.NoError, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if tt.wantErr == nil { - tt.wantErr = require.NoError - } - tt.wantErr(t, disallowUnixSystemRuntimePath(tt.base, tt.path, nil, nil)) - }) - } -} diff --git a/syft/internal/fileresolver/directory_test.go b/syft/internal/fileresolver/directory_test.go index 170b0558db3..ab96c7144c7 100644 --- a/syft/internal/fileresolver/directory_test.go +++ b/syft/internal/fileresolver/directory_test.go @@ -918,7 +918,7 @@ func Test_isUnallowableFileType(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - assert.Equal(t, test.expected, disallowByFileType("", "dont/care", test.info, nil)) + assert.Equal(t, test.expected, disallowByFileType("dont/care", test.info, nil)) }) } } @@ -999,7 +999,7 @@ func Test_IndexingNestedSymLinks(t *testing.T) { } func Test_IndexingNestedSymLinks_ignoredIndexes(t *testing.T) { - filterFn := func(_, path string, _ os.FileInfo, _ error) error { + filterFn := func(path string, _ os.FileInfo, _ error) error { if strings.HasSuffix(path, string(filepath.Separator)+"readme") { return ErrSkipPath } @@ -1144,7 +1144,7 @@ func Test_isUnixSystemRuntimePath(t *testing.T) { } for _, test := range tests { t.Run(test.path, func(t *testing.T) { - assert.Equal(t, test.expected, disallowUnixSystemRuntimePath("", test.path, nil, nil)) + assert.Equal(t, test.expected, disallowUnixSystemRuntimePath(test.path, nil, nil)) }) } } diff --git a/syft/source/directory_source.go b/syft/source/directory_source.go index 3b974521cde..e8e5e217e68 100644 --- a/syft/source/directory_source.go +++ b/syft/source/directory_source.go @@ -197,7 +197,7 @@ func getDirectoryExclusionFunctions(root string, exclusions []string) ([]fileres } return []fileresolver.PathIndexVisitor{ - func(_, path string, info os.FileInfo, _ error) error { + func(path string, info os.FileInfo, _ error) error { for _, exclusion := range exclusions { // this is required to handle Windows filepaths path = filepath.ToSlash(path) diff --git a/syft/source/directory_source_test.go b/syft/source/directory_source_test.go index 3ea197e29e5..c324fb91b9c 100644 --- a/syft/source/directory_source_test.go +++ b/syft/source/directory_source_test.go @@ -392,7 +392,7 @@ func Test_getDirectoryExclusionFunctions_crossPlatform(t *testing.T) { require.NoError(t, err) for _, f := range fns { - result := f("", test.path, test.finfo, nil) + result := f(test.path, test.finfo, nil) require.Equal(t, test.walkHint, result) } }) diff --git a/syft/source/file_source.go b/syft/source/file_source.go index 9e0bd5a86e3..6fcc2f66938 100644 --- a/syft/source/file_source.go +++ b/syft/source/file_source.go @@ -177,7 +177,7 @@ func (s FileSource) FileResolver(_ Scope) (file.Resolver, error) { exclusionFunctions = append([]fileresolver.PathIndexVisitor{ // note: we should exclude these kinds of paths first before considering any other user-provided exclusions - func(_, p string, _ os.FileInfo, _ error) error { + func(p string, _ os.FileInfo, _ error) error { if p == absParentDir { // this is the root directory... always include it return nil