diff --git a/integration/fs_test.go b/integration/fs_test.go index 54f0516181..7933f7c650 100644 --- a/integration/fs_test.go +++ b/integration/fs_test.go @@ -47,7 +47,7 @@ func TestFilesystem(t *testing.T) { args: args{ securityChecks: "vuln", input: "testdata/fixtures/fs/gomod", - skipFiles: []string{"/testdata/fixtures/fs/gomod/submod2/go.mod"}, + skipFiles: []string{"testdata/fixtures/fs/gomod/submod2/go.mod"}, }, golden: "testdata/gomod-skip.json.golden", }, @@ -56,7 +56,7 @@ func TestFilesystem(t *testing.T) { args: args{ securityChecks: "vuln", input: "testdata/fixtures/fs/gomod", - skipDirs: []string{"/testdata/fixtures/fs/gomod/submod2"}, + skipDirs: []string{"testdata/fixtures/fs/gomod/submod2"}, }, golden: "testdata/gomod-skip.json.golden", }, diff --git a/pkg/fanal/artifact/local/fs.go b/pkg/fanal/artifact/local/fs.go index ebaaa14350..f99f95fa3f 100644 --- a/pkg/fanal/artifact/local/fs.go +++ b/pkg/fanal/artifact/local/fs.go @@ -18,6 +18,7 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/handler" "github.com/aquasecurity/trivy/pkg/fanal/types" "github.com/aquasecurity/trivy/pkg/fanal/walker" + "github.com/aquasecurity/trivy/pkg/log" "github.com/aquasecurity/trivy/pkg/semaphore" ) @@ -51,7 +52,7 @@ func NewArtifact(rootPath string, c cache.ArtifactCache, opt artifact.Option) (a return Artifact{ rootPath: filepath.Clean(rootPath), cache: c, - walker: walker.NewFS(buildAbsPaths(rootPath, opt.SkipFiles), buildAbsPaths(rootPath, opt.SkipDirs), opt.Slow), + walker: walker.NewFS(buildPathsToSkip(rootPath, opt.SkipFiles), buildPathsToSkip(rootPath, opt.SkipDirs), opt.Slow), analyzer: a, handlerManager: handlerManager, @@ -59,16 +60,59 @@ func NewArtifact(rootPath string, c cache.ArtifactCache, opt artifact.Option) (a }, nil } -func buildAbsPaths(base string, paths []string) []string { - var absPaths []string - for _, path := range paths { - if filepath.IsAbs(path) { - absPaths = append(absPaths, path) - } else { - absPaths = append(absPaths, filepath.Join(base, path)) - } +// buildPathsToSkip builds correct patch for skipDirs and skipFiles +func buildPathsToSkip(base string, paths []string) []string { + var relativePaths []string + absBase, err := filepath.Abs(base) + if err != nil { + log.Logger.Warnf("Failed to get an absolute path of %s: %s", base, err) + return nil } - return absPaths + for _, path := range paths { + // Supports three types of flag specification. + // All of them are converted into the relative path from the root directory. + // 1. Relative skip dirs/files from the root directory + // The specified dirs and files will be used as is. + // e.g. $ trivy fs --skip-dirs bar ./foo + // The skip dir from the root directory will be `bar/`. + // 2. Relative skip dirs/files from the working directory + // The specified dirs and files wll be converted to the relative path from the root directory. + // e.g. $ trivy fs --skip-dirs ./foo/bar ./foo + // The skip dir will be converted to `bar/`. + // 3. Absolute skip dirs/files + // The specified dirs and files wll be converted to the relative path from the root directory. + // e.g. $ trivy fs --skip-dirs /bar/foo/baz ./foo + // When the working directory is + // 3.1 /bar: the skip dir will be converted to `baz/`. + // 3.2 /hoge : the skip dir will be converted to `../../bar/foo/baz/`. + + absSkipPath, err := filepath.Abs(path) + if err != nil { + log.Logger.Warnf("Failed to get an absolute path of %s: %s", base, err) + continue + } + rel, err := filepath.Rel(absBase, absSkipPath) + if err != nil { + log.Logger.Warnf("Failed to get a relative path from %s to %s: %s", base, path, err) + continue + } + + var relPath string + switch { + case !filepath.IsAbs(path) && strings.HasPrefix(rel, ".."): + // #1: Use the path as is + relPath = path + case !filepath.IsAbs(path) && !strings.HasPrefix(rel, ".."): + // #2: Use the relative path from the root directory + relPath = rel + case filepath.IsAbs(path): + // #3: Use the relative path from the root directory + relPath = rel + } + relPath = filepath.ToSlash(relPath) + relativePaths = append(relativePaths, relPath) + } + return relativePaths } func (a Artifact) Inspect(ctx context.Context) (types.ArtifactReference, error) { @@ -80,22 +124,13 @@ func (a Artifact) Inspect(ctx context.Context) (types.ArtifactReference, error) directory := a.rootPath // When the directory is the same as the filePath, a file was given - // instead of a directory, rewrite the directory in this case. - if a.rootPath == filePath { - directory = filepath.Dir(a.rootPath) + // instead of a directory, rewrite the file path and directory in this case. + if filePath == "." { + directory, filePath = filepath.Split(a.rootPath) } - // For exported rootfs (e.g. images/alpine/etc/alpine-release) - filePath, err := filepath.Rel(directory, filePath) - if err != nil { - return xerrors.Errorf("filepath rel (%s): %w", filePath, err) - } - - // For Windows - filePath = filepath.ToSlash(filePath) - opts := analyzer.AnalysisOptions{Offline: a.artifactOption.Offline} - if err = a.analyzer.AnalyzeFile(ctx, &wg, limit, result, directory, filePath, info, opener, nil, opts); err != nil { + if err := a.analyzer.AnalyzeFile(ctx, &wg, limit, result, directory, filePath, info, opener, nil, opts); err != nil { return xerrors.Errorf("analyze file (%s): %w", filePath, err) } return nil diff --git a/pkg/fanal/artifact/local/fs_test.go b/pkg/fanal/artifact/local/fs_test.go index 0dcd47fb05..ccb19c8d28 100644 --- a/pkg/fanal/artifact/local/fs_test.go +++ b/pkg/fanal/artifact/local/fs_test.go @@ -3,10 +3,13 @@ package local import ( "context" "errors" + "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/exp/slices" "github.com/aquasecurity/trivy/pkg/fanal/analyzer" "github.com/aquasecurity/trivy/pkg/fanal/analyzer/config" @@ -236,35 +239,88 @@ func TestArtifact_Inspect(t *testing.T) { } } -// TODO: fix the logic in the first place -//func TestBuildAbsPath(t *testing.T) { -// tests := []struct { -// name string -// base string -// paths []string -// expectedPaths []string -// }{ -// {"absolute path", "/testBase", []string{"/testPath"}, []string{"/testPath"}}, -// {"relative path", "/testBase", []string{"testPath"}, []string{"/testBase/testPath"}}, -// {"path have '.'", "/testBase", []string{"./testPath"}, []string{"/testBase/testPath"}}, -// {"path have '..'", "/testBase", []string{"../testPath/"}, []string{"/testPath"}}, -// } -// -// for _, test := range tests { -// t.Run(test.name, func(t *testing.T) { -// got := buildAbsPaths(test.base, test.paths) -// if len(test.paths) != len(got) { -// t.Errorf("paths not equals, expected: %s, got: %s", test.expectedPaths, got) -// } else { -// for i, path := range test.expectedPaths { -// if path != got[i] { -// t.Errorf("paths not equals, expected: %s, got: %s", test.expectedPaths, got) -// } -// } -// } -// }) -// } -//} +func TestBuildPathsToSkip(t *testing.T) { + tests := []struct { + name string + oses []string + paths []string + base string + want []string + }{ + // Linux/macOS + { + name: "path - abs, base - abs, not joining paths", + oses: []string{"linux", "darwin"}, + base: "/foo", + paths: []string{"/foo/bar"}, + want: []string{"bar"}, + }, + { + name: "path - abs, base - rel", + oses: []string{"linux", "darwin"}, + base: "foo", + paths: func() []string { + abs, err := filepath.Abs("foo/bar") + require.NoError(t, err) + return []string{abs} + }(), + want: []string{"bar"}, + }, + { + name: "path - rel, base - rel, joining paths", + oses: []string{"linux", "darwin"}, + base: "foo", + paths: []string{"bar"}, + want: []string{"bar"}, + }, + { + name: "path - rel, base - rel, not joining paths", + oses: []string{"linux", "darwin"}, + base: "foo", + paths: []string{"foo/bar/bar"}, + want: []string{"bar/bar"}, + }, + { + name: "path - rel with dot, base - rel, removing the leading dot and not joining paths", + oses: []string{"linux", "darwin"}, + base: "foo", + paths: []string{"./foo/bar"}, + want: []string{"bar"}, + }, + { + name: "path - rel, base - dot", + oses: []string{"linux", "darwin"}, + base: ".", + paths: []string{"foo/bar"}, + want: []string{"foo/bar"}, + }, + // Windows + { + name: "path - rel, base - rel. Skip common prefix", + oses: []string{"windows"}, + base: "foo", + paths: []string{"foo\\bar\\bar"}, + want: []string{"bar/bar"}, + }, + { + name: "path - rel, base - dot, windows", + oses: []string{"windows"}, + base: ".", + paths: []string{"foo\\bar"}, + want: []string{"foo/bar"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if !slices.Contains(tt.oses, runtime.GOOS) { + t.Skipf("Skip path tests for %q", tt.oses) + } + got := buildPathsToSkip(tt.base, tt.paths) + assert.Equal(t, tt.want, got) + }) + } +} func TestTerraformMisconfigurationScan(t *testing.T) { type fields struct { diff --git a/pkg/fanal/walker/fs.go b/pkg/fanal/walker/fs.go index 99344fadc7..5c364402e0 100644 --- a/pkg/fanal/walker/fs.go +++ b/pkg/fanal/walker/fs.go @@ -29,18 +29,25 @@ func (w FS) Walk(root string, fn WalkFunc) error { walkFn := func(pathname string, fi os.FileInfo) error { pathname = filepath.Clean(pathname) + // For exported rootfs (e.g. images/alpine/etc/alpine-release) + relPath, err := filepath.Rel(root, pathname) + if err != nil { + return xerrors.Errorf("filepath rel (%s): %w", relPath, err) + } + relPath = filepath.ToSlash(relPath) + if fi.IsDir() { - if w.shouldSkipDir(pathname) { + if w.shouldSkipDir(relPath) { return filepath.SkipDir } return nil } else if !fi.Mode().IsRegular() { return nil - } else if w.shouldSkipFile(pathname) { + } else if w.shouldSkipFile(relPath) { return nil } - if err := fn(pathname, fi, w.fileOpener(pathname)); err != nil { + if err := fn(relPath, fi, w.fileOpener(pathname)); err != nil { return xerrors.Errorf("failed to analyze file: %w", err) } return nil diff --git a/pkg/fanal/walker/walk.go b/pkg/fanal/walker/walk.go index 03f7e20351..453229ac0a 100644 --- a/pkg/fanal/walker/walk.go +++ b/pkg/fanal/walker/walk.go @@ -50,7 +50,6 @@ func newWalker(skipFiles, skipDirs []string, slow bool) walker { } func (w *walker) shouldSkipFile(filePath string) bool { - filePath = filepath.ToSlash(filePath) filePath = strings.TrimLeft(filePath, "/") // skip files @@ -58,7 +57,6 @@ func (w *walker) shouldSkipFile(filePath string) bool { } func (w *walker) shouldSkipDir(dir string) bool { - dir = filepath.ToSlash(dir) dir = strings.TrimLeft(dir, "/") // Skip application dirs (relative path)