Skip to content

Commit

Permalink
fix: change logic to build relative paths for skip-dirs and skip-files (
Browse files Browse the repository at this point in the history 
#3331)

Co-authored-by: knqyf263 <knqyf263@gmail.com>
  • Loading branch information
@DmitriyLewen @knqyf263
DmitriyLewen and knqyf263 authored Dec 26, 2022
1 parent a66d3fe commit b0cebec
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 57 deletions.
4 changes: 2 additions & 2 deletions integration/fs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand All @@ -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",
},
Expand Down
77 changes: 56 additions & 21 deletions pkg/fanal/artifact/local/fs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -51,24 +52,67 @@ 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,

artifactOption: opt,
}, nil
}

func buildAbsPaths(base string, paths []string) []string {
var absPaths []string
// 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
}
for _, path := range paths {
if filepath.IsAbs(path) {
absPaths = append(absPaths, path)
} else {
absPaths = append(absPaths, filepath.Join(base, path))
// 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 absPaths
return relativePaths
}

func (a Artifact) Inspect(ctx context.Context) (types.ArtifactReference, error) {
Expand All @@ -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)
}

// 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)
// instead of a directory, rewrite the file path and directory in this case.
if filePath == "." {
directory, filePath = filepath.Split(a.rootPath)
}

// 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
Expand Down
114 changes: 85 additions & 29 deletions pkg/fanal/artifact/local/fs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 10 additions & 3 deletions pkg/fanal/walker/fs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pkg/fanal/walker/walk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,13 @@ 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
return utils.StringInSlice(filePath, w.skipFiles)
}

func (w *walker) shouldSkipDir(dir string) bool {
dir = filepath.ToSlash(dir)
dir = strings.TrimLeft(dir, "/")

// Skip application dirs (relative path)
Expand Down

0 comments on commit b0cebec

Please sign in to comment.