Skip to content

Commit

Permalink
resolve links when fetching file contents
Browse files Browse the repository at this point in the history
Signed-off-by: Alex Goodman <[email protected]>
  • Loading branch information
wagoodman committed Jan 30, 2022
1 parent 9f7104d commit 79a06f1
Show file tree
Hide file tree
Showing 15 changed files with 306 additions and 11 deletions.
3 changes: 2 additions & 1 deletion syft/file/classification_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ func (i *ClassificationCataloger) Catalog(resolver source.FileResolver) (map[sou
for _, classifier := range i.classifiers {
result, err := classifier.Classify(resolver, location)
if err != nil {
return nil, err
log.Warnf("file classification cataloger failed with class=%q at location=%+v: %+v", classifier.Class, location, err)
continue
}
if result != nil {
results[location.Coordinates] = append(results[location.Coordinates], *result)
Expand Down
59 changes: 59 additions & 0 deletions syft/file/classification_cataloger_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package file

import (
"github.com/anchore/stereoscope/pkg/imagetest"
"testing"

"github.com/anchore/syft/syft/source"
Expand Down Expand Up @@ -134,6 +135,64 @@ func TestClassifierCataloger_DefaultClassifiers_PositiveCases(t *testing.T) {
}
}

func TestClassifierCataloger_DefaultClassifiers_PositiveCases_Image(t *testing.T) {
tests := []struct {
name string
fixtureImage string
location string
expected []Classification
expectedErr func(assert.TestingT, error, ...interface{}) bool
}{
{
name: "busybox-regression",
fixtureImage: "image-busybox",
location: "/bin/busybox",
expected: []Classification{
{
Class: "busybox-binary",
Metadata: map[string]string{
"version": "1.35.0",
},
},
},
expectedErr: assert.NoError,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {

c, err := NewClassificationCataloger(DefaultClassifiers)
test.expectedErr(t, err)

img := imagetest.GetFixtureImage(t, "docker-archive", test.fixtureImage)
src, err := source.NewFromImage(img, "test-img")
test.expectedErr(t, err)

resolver, err := src.FileResolver(source.SquashedScope)
test.expectedErr(t, err)

actualResults, err := c.Catalog(resolver)
test.expectedErr(t, err)

loc := source.NewLocation(test.location)

ok := false
for actual_loc, actual_classification := range actualResults {
if loc.RealPath == actual_loc.RealPath {
ok = true
assert.Equal(t, test.expected, actual_classification)
}
}

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

})
}
}

func TestClassifierCataloger_DefaultClassifiers_NegativeCases(t *testing.T) {

c, err := NewClassificationCataloger(DefaultClassifiers)
Expand Down
20 changes: 19 additions & 1 deletion syft/file/digest_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package file

import (
"crypto"
"errors"
"fmt"
"hash"
"io"
Expand All @@ -19,6 +20,8 @@ import (
"github.com/anchore/syft/syft/source"
)

var errUndigestableFile = errors.New("undigestable file")

type DigestsCataloger struct {
hashes []crypto.Hash
}
Expand All @@ -39,8 +42,13 @@ func (i *DigestsCataloger) Catalog(resolver source.FileResolver) (map[source.Coo
for _, location := range locations {
stage.Current = location.RealPath
result, err := i.catalogLocation(resolver, location)

if errors.Is(err, errUndigestableFile) {
continue
}

if internal.IsErrPathPermission(err) {
log.Debugf("file digests cataloger skipping - %+v", err)
log.Debugf("file digests cataloger skipping %q: %+v", location.RealPath, err)
continue
}

Expand All @@ -56,6 +64,16 @@ func (i *DigestsCataloger) Catalog(resolver source.FileResolver) (map[source.Coo
}

func (i *DigestsCataloger) catalogLocation(resolver source.FileResolver, location source.Location) ([]Digest, error) {
meta, err := resolver.FileMetadataByLocation(location)
if err != nil {
return nil, err
}

// we should only attempt to report digests for files that are regular files (don't attempt to resolve links)
if meta.Type != source.RegularFile {
return nil, errUndigestableFile
}

contentReader, err := resolver.FileContentsByLocation(location)
if err != nil {
return nil, err
Expand Down
5 changes: 3 additions & 2 deletions syft/file/digest_cataloger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ func TestDigestsCataloger_SimpleContents(t *testing.T) {
expected: testDigests(t, regularFiles, crypto.MD5, crypto.SHA1, crypto.SHA256),
},
{
name: "directory returns error",
name: "directory is ignored",
digests: []crypto.Hash{crypto.MD5},
files: []string{"test-fixtures/last"},
catalogErr: true,
expected: make(map[source.Coordinates][]Digest), // empty
catalogErr: false,
},
}

Expand Down
4 changes: 4 additions & 0 deletions syft/file/secrets_cataloger.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ func (i *SecretsCataloger) catalogLocation(resolver source.FileResolver, locatio
return nil, err
}

if metadata.Size == 0 {
return nil, nil
}

if i.skipFilesAboveSize > 0 && metadata.Size > i.skipFilesAboveSize {
return nil, nil
}
Expand Down
3 changes: 3 additions & 0 deletions syft/file/test-fixtures/classifiers/positive/[
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# note: this SHOULD match as busybox 3.33.3

noise!BusyBox v3.33.3!noise
3 changes: 0 additions & 3 deletions syft/file/test-fixtures/classifiers/positive/busybox

This file was deleted.

1 change: 1 addition & 0 deletions syft/file/test-fixtures/classifiers/positive/busybox
1 change: 1 addition & 0 deletions syft/file/test-fixtures/image-busybox/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
FROM busybox:1.35
16 changes: 16 additions & 0 deletions syft/source/all_layers_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,22 @@ func (r *allLayersResolver) RelativeFileByPath(location Location, path string) *
// FileContentsByLocation fetches file contents for a single file reference, irregardless of the source layer.
// If the path does not exist an error is returned.
func (r *allLayersResolver) FileContentsByLocation(location Location) (io.ReadCloser, error) {
entry, err := r.img.FileCatalog.Get(location.ref)
if err != nil {
return nil, fmt.Errorf("unable to get metadata for path=%q from file catalog: %w", location.RealPath, err)
}

switch entry.Metadata.TypeFlag {
case tar.TypeSymlink, tar.TypeLink:
// the location we are searching may be a symlink, we should always work with the resolved file
newLocation := r.RelativeFileByPath(location, location.VirtualPath)
if newLocation == nil {
// this is a dead link
return nil, fmt.Errorf("no contents for location=%q", location.VirtualPath)
}
location = *newLocation
}

return r.img.FileContentsByRef(location.ref)
}

Expand Down
59 changes: 59 additions & 0 deletions syft/source/all_layers_resolver_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package source

import (
"github.com/stretchr/testify/require"
"io"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -301,3 +303,60 @@ func Test_imageAllLayersResolver_hasFilesystemIDInLocation(t *testing.T) {
}

}

func TestAllLayersImageResolver_FilesContents(t *testing.T) {

tests := []struct {
name string
fixture string
contents []string
}{
{
name: "one degree",
fixture: "link-2",
contents: []string{
"file 2!", // from the first resolved layer's perspective
"NEW file override!", // from the second resolved layers perspective
},
},
{
name: "two degrees",
fixture: "link-indirect",
contents: []string{
"file 2!",
"NEW file override!",
},
},
{
name: "dead link",
fixture: "link-dead",
contents: []string{},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
img := imagetest.GetFixtureImage(t, "docker-archive", "image-symlinks")

resolver, err := newAllLayersResolver(img)
assert.NoError(t, err)

refs, err := resolver.FilesByPath(test.fixture)
require.NoError(t, err)

// the given path should have an overridden file
require.Len(t, refs, len(test.contents))

for idx, loc := range refs {
reader, err := resolver.FileContentsByLocation(loc)
require.NoError(t, err)

actual, err := io.ReadAll(reader)
require.NoError(t, err)

assert.Equal(t, test.contents[idx], string(actual))
}

})
}
}
22 changes: 18 additions & 4 deletions syft/source/directory_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,15 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error)
continue
}

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

// TODO: why not use stored metadata?
fileMeta, err := os.Stat(userStrPath)
fileMeta, err := os.Stat(evaluatedPath)
if errors.Is(err, os.ErrNotExist) {
// note: there are other kinds of errors other than os.ErrNotExist that may be given that is platform
// specific, but essentially hints at the same overall problem (that the path does not exist). Such an
Expand All @@ -317,7 +324,7 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error)
// invalid paths. This logging statement is meant to raise IO or permissions related problems.
var pathErr *os.PathError
if !errors.As(err, &pathErr) {
log.Warnf("path is not valid (%s): %+v", userStrPath, err)
log.Warnf("path is not valid (%s): %+v", evaluatedPath, err)
}
continue
}
Expand All @@ -329,11 +336,17 @@ func (r directoryResolver) FilesByPath(userPaths ...string) ([]Location, error)

if runtime.GOOS == WindowsOS {
userStrPath = windowsToPosix(userStrPath)
evaluatedPath = windowsToPosix(evaluatedPath)
}

exists, ref, err := r.fileTree.File(file.Path(userStrPath))
if err == nil && exists {
references = append(references, NewLocationFromDirectory(r.responsePath(userStrPath), *ref))
loc := NewLocationFromDirectory(r.responsePath(userStrPath), *ref)
if evaluatedPath != userStrPath {
loc.VirtualPath = r.responsePath(evaluatedPath)
}

references = append(references, loc)
}
}

Expand Down Expand Up @@ -404,7 +417,8 @@ func (r *directoryResolver) AllLocations() <-chan Location {
results := make(chan Location)
go func() {
defer close(results)
for _, ref := range r.fileTree.AllFiles() {
// this should be all non-directory types
for _, ref := range r.fileTree.AllFiles(file.TypeReg, file.TypeSymlink, file.TypeHardLink, file.TypeBlockDevice, file.TypeCharacterDevice, file.TypeFifo) {
results <- NewLocationFromDirectory(r.responsePath(string(ref.RealPath)), ref)
}
}()
Expand Down
40 changes: 40 additions & 0 deletions syft/source/directory_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package source

import (
"io"
"io/fs"
"io/ioutil"
"os"
Expand Down Expand Up @@ -259,6 +260,45 @@ func TestDirectoryResolver_FilesByGlobSingle(t *testing.T) {
assert.Equal(t, "image-symlinks/file-1.txt", refs[0].RealPath)
}

func TestDirectoryResolver_FilesByPath_ResolvesSymlinks(t *testing.T) {

tests := []struct {
name string
fixture string
}{
{
name: "one degree",
fixture: "link_to_new_readme",
},
{
name: "two degrees",
fixture: "link_to_link_to_new_readme",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
resolver, err := newDirectoryResolver("./test-fixtures/symlinks-simple")
assert.NoError(t, err)

refs, err := resolver.FilesByPath(test.fixture)
require.NoError(t, err)
assert.Len(t, refs, 1)

reader, err := resolver.FileContentsByLocation(refs[0])
require.NoError(t, err)

actual, err := io.ReadAll(reader)
require.NoError(t, err)

expected, err := os.ReadFile("test-fixtures/symlinks-simple/readme")
require.NoError(t, err)

assert.Equal(t, string(expected), string(actual))
})
}
}

func TestDirectoryResolverDoesNotIgnoreRelativeSystemPaths(t *testing.T) {
// let's make certain that "dev/place" is not ignored, since it is not "/dev/place"
resolver, err := newDirectoryResolver("test-fixtures/system_paths/target")
Expand Down
Loading

0 comments on commit 79a06f1

Please sign in to comment.