fix: change logic to build relative paths for skip-dirs and skip-files (#3331)

Co-authored-by: knqyf263 <knqyf263@gmail.com>

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

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 {
+// buildPathsToSkip builds correct patch for skipDirs and skipFiles
-	var absPaths []string
+func buildPathsToSkip(base string, paths []string) []string {
-	for _, path := range paths {
+	var relativePaths []string
-		if filepath.IsAbs(path) {
+	absBase, err := filepath.Abs(base)
-			absPaths = append(absPaths, path)
+	if err != nil {
-		} else {
+		log.Logger.Warnf("Failed to get an absolute path of %s: %s", base, err)
-			absPaths = append(absPaths, filepath.Join(base, path))
+		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.
+		// instead of a directory, rewrite the file path and directory in this case.
-		if a.rootPath == filePath {
+		if filePath == "." {
-			directory = filepath.Dir(a.rootPath)
+			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

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 TestBuildPathsToSkip(t *testing.T) {
-//func TestBuildAbsPath(t *testing.T) {
+	tests := []struct {
-//	tests := []struct {
+		name  string
-//		name          string
+		oses  []string
-//		base          string
+		paths []string
-//		paths         []string
+		base  string
-//		expectedPaths []string
+		want  []string
-//	}{
+	}{
-//		{"absolute path", "/testBase", []string{"/testPath"}, []string{"/testPath"}},
+		// Linux/macOS
-//		{"relative path", "/testBase", []string{"testPath"}, []string{"/testBase/testPath"}},
+		{
-//		{"path have '.'", "/testBase", []string{"./testPath"}, []string{"/testBase/testPath"}},
+			name:  "path - abs, base - abs, not joining paths",
-//		{"path have '..'", "/testBase", []string{"../testPath/"}, []string{"/testPath"}},
+			oses:  []string{"linux", "darwin"},
-//	}
+			base:  "/foo",
-//
+			paths: []string{"/foo/bar"},
-//	for _, test := range tests {
+			want:  []string{"bar"},
-//		t.Run(test.name, func(t *testing.T) {
+		},
-//			got := buildAbsPaths(test.base, test.paths)
+		{
-//			if len(test.paths) != len(got) {
+			name: "path - abs, base - rel",
-//				t.Errorf("paths not equals, expected: %s, got: %s", test.expectedPaths, got)
+			oses: []string{"linux", "darwin"},
-//			} else {
+			base: "foo",
-//				for i, path := range test.expectedPaths {
+			paths: func() []string {
-//					if path != got[i] {
+				abs, err := filepath.Abs("foo/bar")
-//						t.Errorf("paths not equals, expected: %s, got: %s", test.expectedPaths, got)
+				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 {

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

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)