improved handeling of dir resolver roots accessed via symlink

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

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")
 		})
 	}
 }

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
 		}
 

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) {

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
 		}

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)
 
 			}
 

syft/source/test-fixtures/symlinked-root/nested/link-root

@@ -0,0 +1 @@
+../real-root

syft/source/test-fixtures/symlinked-root/real-root/file1.txt

@@ -0,0 +1 @@
+contents!

syft/source/test-fixtures/symlinked-root/real-root/nested/file2.txt

@@ -0,0 +1 @@
+more contents!

syft/source/test-fixtures/symlinked-root/real-root/nested/linked-file1.txt

@@ -0,0 +1 @@
+../file1.txt