fix considering base path when ignoring known bad unix paths (#2644)

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
This commit is contained in:
Alex Goodman 2024-02-14 15:57:38 -05:00 committed by GitHub
parent 8e62ff9831
commit a909e3cec9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 131 additions and 21 deletions

View File

@ -21,7 +21,7 @@ import (
"github.com/anchore/syft/syft/internal/windows" "github.com/anchore/syft/syft/internal/windows"
) )
type PathIndexVisitor func(string, os.FileInfo, error) error type PathIndexVisitor func(string, string, os.FileInfo, error) error
type directoryIndexer struct { type directoryIndexer struct {
path string path string
@ -239,14 +239,14 @@ func allContainedPaths(p string) []string {
return all return all
} }
func (r *directoryIndexer) indexPath(path string, info os.FileInfo, err error) (string, error) { func (r *directoryIndexer) indexPath(givenPath string, info os.FileInfo, err error) (string, error) {
// ignore any path which a filter function returns true // ignore any path which a filter function returns true
for _, filterFn := range r.pathIndexVisitors { for _, filterFn := range r.pathIndexVisitors {
if filterFn == nil { if filterFn == nil {
continue continue
} }
if filterErr := filterFn(path, info, err); filterErr != nil { if filterErr := filterFn(r.base, givenPath, info, err); filterErr != nil {
if errors.Is(filterErr, fs.SkipDir) { if errors.Is(filterErr, fs.SkipDir) {
// signal to walk() to skip this directory entirely (even if we're processing a file) // signal to walk() to skip this directory entirely (even if we're processing a file)
return "", filterErr return "", filterErr
@ -258,24 +258,24 @@ func (r *directoryIndexer) indexPath(path string, info os.FileInfo, err error) (
if info == nil { 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. // 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[path] = fmt.Errorf("no file info observable at path=%q", path) r.errPaths[givenPath] = fmt.Errorf("no file info observable at path=%q", givenPath)
return "", nil return "", nil
} }
// here we check to see if we need to normalize paths to posix on the way in coming from windows // here we check to see if we need to normalize paths to posix on the way in coming from windows
if windows.HostRunningOnWindows() { if windows.HostRunningOnWindows() {
path = windows.ToPosix(path) givenPath = windows.ToPosix(givenPath)
} }
newRoot, err := r.addPathToIndex(path, info) newRoot, err := r.addPathToIndex(givenPath, info)
if r.isFileAccessErr(path, err) { if r.isFileAccessErr(givenPath, err) {
return "", nil return "", nil
} }
return newRoot, 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) { if r.isFileAccessErr(path, err) {
return ErrSkipPath return ErrSkipPath
} }
@ -422,7 +422,7 @@ func (r directoryIndexer) hasBeenIndexed(p string) (bool, *file.Metadata) {
return true, &entry.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: // this prevents visiting:
// - link destinations twice, once for the real file and another through the virtual path // - link destinations twice, once for the real file and another through the virtual path
// - infinite link cycles // - infinite link cycles
@ -436,14 +436,17 @@ func (r *directoryIndexer) disallowRevisitingVisitor(path string, _ os.FileInfo,
return nil return nil
} }
func disallowUnixSystemRuntimePath(path string, _ os.FileInfo, _ error) error { func disallowUnixSystemRuntimePath(base, path string, _ os.FileInfo, _ error) error {
if internal.HasAnyOfPrefixes(path, unixSystemRuntimePrefixes...) { // 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...) {
return fs.SkipDir return fs.SkipDir
} }
return nil return nil
} }
func disallowByFileType(_ string, info os.FileInfo, _ error) error { func disallowByFileType(_, _ string, info os.FileInfo, _ error) error {
if info == nil { if info == nil {
// we can't filter out by filetype for non-existent files // we can't filter out by filetype for non-existent files
return nil return nil
@ -458,13 +461,39 @@ func disallowByFileType(_ string, info os.FileInfo, _ error) error {
return nil return nil
} }
func requireFileInfo(_ string, info os.FileInfo, _ error) error { func requireFileInfo(_, _ string, info os.FileInfo, _ error) error {
if info == nil { if info == nil {
return ErrSkipPath return ErrSkipPath
} }
return nil 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) { func indexingProgress(path string) (*progress.Stage, *progress.Manual) {
stage := &progress.Stage{} stage := &progress.Stage{}
prog := progress.NewManual(-1) prog := progress.NewManual(-1)

View File

@ -135,7 +135,7 @@ func TestDirectoryIndexer_handleFileAccessErr(t *testing.T) {
} }
func TestDirectoryIndexer_IncludeRootPathInIndex(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 != "/" { if path != "/" {
return fs.SkipDir return fs.SkipDir
} }
@ -224,7 +224,7 @@ func TestDirectoryIndexer_index(t *testing.T) {
func TestDirectoryIndexer_SkipsAlreadyVisitedLinkDestinations(t *testing.T) { func TestDirectoryIndexer_SkipsAlreadyVisitedLinkDestinations(t *testing.T) {
var observedPaths []string 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") fields := strings.Split(p, "test-fixtures/symlinks-prune-indexing")
if len(fields) < 2 { if len(fields) < 2 {
return nil return nil
@ -383,3 +383,84 @@ 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))
})
}
}

View File

@ -918,7 +918,7 @@ func Test_isUnallowableFileType(t *testing.T) {
} }
for _, test := range tests { for _, test := range tests {
t.Run(test.name, func(t *testing.T) { 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) { 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") { if strings.HasSuffix(path, string(filepath.Separator)+"readme") {
return ErrSkipPath return ErrSkipPath
} }
@ -1144,7 +1144,7 @@ func Test_isUnixSystemRuntimePath(t *testing.T) {
} }
for _, test := range tests { for _, test := range tests {
t.Run(test.path, func(t *testing.T) { 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))
}) })
} }
} }

View File

@ -197,7 +197,7 @@ func getDirectoryExclusionFunctions(root string, exclusions []string) ([]fileres
} }
return []fileresolver.PathIndexVisitor{ return []fileresolver.PathIndexVisitor{
func(path string, info os.FileInfo, _ error) error { func(_, path string, info os.FileInfo, _ error) error {
for _, exclusion := range exclusions { for _, exclusion := range exclusions {
// this is required to handle Windows filepaths // this is required to handle Windows filepaths
path = filepath.ToSlash(path) path = filepath.ToSlash(path)

View File

@ -392,7 +392,7 @@ func Test_getDirectoryExclusionFunctions_crossPlatform(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
for _, f := range fns { for _, f := range fns {
result := f(test.path, test.finfo, nil) result := f("", test.path, test.finfo, nil)
require.Equal(t, test.walkHint, result) require.Equal(t, test.walkHint, result)
} }
}) })

View File

@ -177,7 +177,7 @@ func (s FileSource) FileResolver(_ Scope) (file.Resolver, error) {
exclusionFunctions = append([]fileresolver.PathIndexVisitor{ exclusionFunctions = append([]fileresolver.PathIndexVisitor{
// note: we should exclude these kinds of paths first before considering any other user-provided exclusions // 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 { if p == absParentDir {
// this is the root directory... always include it // this is the root directory... always include it
return nil return nil