From 13e72eca5832bdfbd44285bec1bf31894cfaa411 Mon Sep 17 00:00:00 2001 From: Teppei Fukuda Date: Wed, 17 Apr 2024 22:24:18 +0400 Subject: [PATCH] refactor: remove parallel walk (#5180) Signed-off-by: knqyf263 --- .github/workflows/test.yaml | 4 +- .golangci.yaml | 8 +- go.mod | 1 - go.sum | 2 - magefiles/magefile.go | 2 +- pkg/commands/artifact/inject.go | 6 +- pkg/commands/artifact/run.go | 11 +- pkg/commands/artifact/scanner.go | 10 +- pkg/commands/artifact/wire_gen.go | 23 ++- pkg/fanal/analyzer/pkg/dpkg/copyright.go | 2 +- pkg/fanal/artifact/artifact.go | 17 +- pkg/fanal/artifact/image/image.go | 5 +- pkg/fanal/artifact/local/fs.go | 85 +++------ pkg/fanal/artifact/local/fs_test.go | 132 ++------------ pkg/fanal/artifact/repo/git.go | 30 ++- pkg/fanal/artifact/repo/git_test.go | 5 +- pkg/fanal/artifact/vm/vm.go | 15 +- pkg/fanal/artifact/vm/vm_test.go | 4 +- pkg/fanal/cache/key.go | 2 +- pkg/fanal/cache/key_test.go | 8 +- pkg/fanal/external/config_scan.go | 79 -------- pkg/fanal/external/config_scan_test.go | 143 --------------- pkg/fanal/external/testdata/allow/Dockerfile | 3 - pkg/fanal/external/testdata/allow/docker.rego | 20 -- pkg/fanal/external/testdata/deny/Dockerfile | 3 - pkg/fanal/external/testdata/deny/docker.rego | 25 --- pkg/fanal/test/integration/library_test.go | 1 - pkg/fanal/walker/cached_file.go | 11 +- pkg/fanal/walker/fs.go | 171 +++++++++++------- pkg/fanal/walker/fs_test.go | 144 ++++++++++++--- pkg/fanal/walker/tar.go | 23 ++- pkg/fanal/walker/tar_test.go | 17 +- pkg/fanal/walker/vm.go | 36 ++-- pkg/fanal/walker/walk.go | 82 +++------ pkg/fanal/walker/walk_test.go | 141 +++++++++------ pkg/flag/report_flags.go | 2 +- pkg/iac/rules/register.go | 2 +- pkg/iac/scanners/azure/functions/first.go | 2 +- pkg/iac/scanners/azure/functions/last.go | 2 +- pkg/k8s/commands/namespace.go | 2 +- pkg/mapfs/fs.go | 2 +- pkg/parallel/walk.go | 6 +- pkg/report/table/vulnerability.go | 2 +- pkg/sbom/cyclonedx/marshal.go | 2 +- pkg/scanner/scan.go | 12 +- 45 files changed, 504 insertions(+), 801 deletions(-) delete mode 100644 pkg/fanal/external/config_scan.go delete mode 100644 pkg/fanal/external/config_scan_test.go delete mode 100644 pkg/fanal/external/testdata/allow/Dockerfile delete mode 100644 pkg/fanal/external/testdata/allow/docker.rego delete mode 100644 pkg/fanal/external/testdata/deny/Dockerfile delete mode 100644 pkg/fanal/external/testdata/deny/docker.rego diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6884f1c9368b..c9b93fb4e37d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -45,8 +45,8 @@ jobs: id: lint uses: golangci/golangci-lint-action@v4.0.0 with: - version: v1.54 - args: --deadline=30m --out-format=line-number + version: v1.57 + args: --timeout=30m --out-format=line-number skip-cache: true # https://github.com/golangci/golangci-lint-action/issues/244#issuecomment-1052197778 if: matrix.operating-system == 'ubuntu-latest' diff --git a/.golangci.yaml b/.golangci.yaml index 5f959a4cc339..8a4d089f9524 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -89,15 +89,15 @@ linters: run: go: '1.22' - skip-files: + +issues: + exclude-files: - ".*_mock.go$" - ".*_test.go$" - "integration/*" - "examples/*" - skip-dirs: + exclude-dirs: - "pkg/iac/scanners/terraform/parser/funcs" # copies of Terraform functions - -issues: exclude-rules: - linters: - gosec diff --git a/go.mod b/go.mod index ee06f719ac44..5ab8f34700cb 100644 --- a/go.mod +++ b/go.mod @@ -88,7 +88,6 @@ require ( github.com/package-url/packageurl-go v0.1.2 github.com/quasilyte/go-ruleguard/dsl v0.3.22 github.com/samber/lo v1.39.0 - github.com/saracen/walker v0.1.3 github.com/secure-systems-lab/go-securesystemslib v0.8.0 github.com/sigstore/rekor v1.2.2 github.com/sirupsen/logrus v1.9.3 diff --git a/go.sum b/go.sum index 1f149fc1b692..969a394b2da8 100644 --- a/go.sum +++ b/go.sum @@ -1523,8 +1523,6 @@ github.com/samber/lo v1.39.0 h1:4gTz1wUhNYLhFSKl6O+8peW0v2F4BCY034GRpU9WnuA= github.com/samber/lo v1.39.0/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA= github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 h1:lZUw3E0/J3roVtGQ+SCrUrg3ON6NgVqpn3+iol9aGu4= github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY= -github.com/saracen/walker v0.1.3 h1:YtcKKmpRPy6XJTHJ75J2QYXXZYWnZNQxPCVqZSHVV/g= -github.com/saracen/walker v0.1.3/go.mod h1:FU+7qU8DeQQgSZDmmThMJi93kPkLFgy0oVAcLxurjIk= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc= github.com/seccomp/libseccomp-golang v0.9.1/go.mod h1:GbW5+tmTXfcxTToHLXlScSlAvWlF4P2Ca7zGrPiEpWo= diff --git a/magefiles/magefile.go b/magefiles/magefile.go index fbb612b9d855..9e456f9bdf40 100644 --- a/magefiles/magefile.go +++ b/magefiles/magefile.go @@ -61,7 +61,7 @@ func (Tool) Wire() error { // GolangciLint installs golangci-lint func (Tool) GolangciLint() error { - const version = "v1.54.2" + const version = "v1.57.2" if exists(filepath.Join(GOBIN, "golangci-lint")) { return nil } diff --git a/pkg/commands/artifact/inject.go b/pkg/commands/artifact/inject.go index 19d6e8565f02..f0538efdbce3 100644 --- a/pkg/commands/artifact/inject.go +++ b/pkg/commands/artifact/inject.go @@ -5,8 +5,6 @@ package artifact import ( "context" - "github.com/aquasecurity/trivy/pkg/fanal/artifact/vm" - "github.com/google/wire" "github.com/aquasecurity/trivy/pkg/fanal/artifact" @@ -57,7 +55,7 @@ func initializeSBOMScanner(ctx context.Context, filePath string, artifactCache c } func initializeVMScanner(ctx context.Context, filePath string, artifactCache cache.ArtifactCache, - localArtifactCache cache.LocalArtifactCache, walker vm.Walker, artifactOption artifact.Option) ( + localArtifactCache cache.LocalArtifactCache, artifactOption artifact.Option) ( scanner.Scanner, func(), error) { wire.Build(scanner.StandaloneVMSet) return scanner.Scanner{}, nil, nil @@ -108,7 +106,7 @@ func initializeRemoteSBOMScanner(ctx context.Context, path string, artifactCache // initializeRemoteVMScanner is for vm scanning in client/server mode func initializeRemoteVMScanner(ctx context.Context, path string, artifactCache cache.ArtifactCache, - walker vm.Walker, remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { + remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { wire.Build(scanner.RemoteVMSet) return scanner.Scanner{}, nil, nil } diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go index b156ce44754a..d50a39c65ca3 100644 --- a/pkg/commands/artifact/run.go +++ b/pkg/commands/artifact/run.go @@ -19,6 +19,7 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/artifact" "github.com/aquasecurity/trivy/pkg/fanal/cache" ftypes "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/fanal/walker" "github.com/aquasecurity/trivy/pkg/flag" "github.com/aquasecurity/trivy/pkg/javadb" "github.com/aquasecurity/trivy/pkg/log" @@ -650,9 +651,8 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi }, ArtifactOption: artifact.Option{ DisabledAnalyzers: disabledAnalyzers(opts), - SkipFiles: opts.SkipFiles, - SkipDirs: opts.SkipDirs, FilePatterns: opts.FilePatterns, + Parallel: opts.Parallel, Offline: opts.OfflineScan, NoProgress: opts.NoProgress || opts.Quiet, Insecure: opts.Insecure, @@ -662,7 +662,6 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi SBOMSources: opts.SBOMSources, RekorURL: opts.RekorURL, //Platform: opts.Platform, - Parallel: opts.Parallel, AWSRegion: opts.Region, AWSEndpoint: opts.Endpoint, FileChecksum: fileChecksum, @@ -692,6 +691,12 @@ func initScannerConfig(opts flag.Options, cacheClient cache.Cache) (ScannerConfi Full: opts.LicenseFull, ClassifierConfidenceLevel: opts.LicenseConfidenceLevel, }, + + // For file walking + WalkerOption: walker.Option{ + SkipFiles: opts.SkipFiles, + SkipDirs: opts.SkipDirs, + }, }, }, scanOptions, nil } diff --git a/pkg/commands/artifact/scanner.go b/pkg/commands/artifact/scanner.go index 5685572a10c4..cf7a58c52693 100644 --- a/pkg/commands/artifact/scanner.go +++ b/pkg/commands/artifact/scanner.go @@ -5,7 +5,6 @@ import ( "golang.org/x/xerrors" - "github.com/aquasecurity/trivy/pkg/fanal/walker" "github.com/aquasecurity/trivy/pkg/scanner" ) @@ -110,10 +109,7 @@ func sbomRemoteScanner(ctx context.Context, conf ScannerConfig) (scanner.Scanner // vmStandaloneScanner initializes a VM scanner in standalone mode func vmStandaloneScanner(ctx context.Context, conf ScannerConfig) (scanner.Scanner, func(), error) { - // TODO: The walker should be initialized in initializeVMScanner after https://github.com/aquasecurity/trivy/pull/5180 - w := walker.NewVM(conf.ArtifactOption.SkipFiles, conf.ArtifactOption.SkipDirs) - s, cleanup, err := initializeVMScanner(ctx, conf.Target, conf.ArtifactCache, conf.LocalArtifactCache, - w, conf.ArtifactOption) + s, cleanup, err := initializeVMScanner(ctx, conf.Target, conf.ArtifactCache, conf.LocalArtifactCache, conf.ArtifactOption) if err != nil { return scanner.Scanner{}, func() {}, xerrors.Errorf("unable to initialize a vm scanner: %w", err) } @@ -122,9 +118,7 @@ func vmStandaloneScanner(ctx context.Context, conf ScannerConfig) (scanner.Scann // vmRemoteScanner initializes a VM scanner in client/server mode func vmRemoteScanner(ctx context.Context, conf ScannerConfig) (scanner.Scanner, func(), error) { - // TODO: The walker should be initialized in initializeVMScanner after https://github.com/aquasecurity/trivy/pull/5180 - w := walker.NewVM(conf.ArtifactOption.SkipFiles, conf.ArtifactOption.SkipDirs) - s, cleanup, err := initializeRemoteVMScanner(ctx, conf.Target, conf.ArtifactCache, w, conf.ServerOption, conf.ArtifactOption) + s, cleanup, err := initializeRemoteVMScanner(ctx, conf.Target, conf.ArtifactCache, conf.ServerOption, conf.ArtifactOption) if err != nil { return scanner.Scanner{}, func() {}, xerrors.Errorf("unable to initialize a remote vm scanner: %w", err) } diff --git a/pkg/commands/artifact/wire_gen.go b/pkg/commands/artifact/wire_gen.go index e91ae3d5e8c8..5e12c10e54d8 100644 --- a/pkg/commands/artifact/wire_gen.go +++ b/pkg/commands/artifact/wire_gen.go @@ -19,6 +19,7 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/cache" "github.com/aquasecurity/trivy/pkg/fanal/image" "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/fanal/walker" "github.com/aquasecurity/trivy/pkg/rpc/client" "github.com/aquasecurity/trivy/pkg/scanner" "github.com/aquasecurity/trivy/pkg/scanner/langpkg" @@ -82,7 +83,8 @@ func initializeFilesystemScanner(ctx context.Context, path string, artifactCache config := db.Config{} client := vulnerability.NewClient(config) localScanner := local.NewScanner(applierApplier, ospkgScanner, langpkgScanner, client) - artifactArtifact, err := local2.NewArtifact(path, artifactCache, artifactOption) + fs := walker.NewFS() + artifactArtifact, err := local2.NewArtifact(path, artifactCache, fs, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } @@ -98,7 +100,8 @@ func initializeRepositoryScanner(ctx context.Context, url string, artifactCache config := db.Config{} client := vulnerability.NewClient(config) localScanner := local.NewScanner(applierApplier, ospkgScanner, langpkgScanner, client) - artifactArtifact, cleanup, err := repo.NewArtifact(url, artifactCache, artifactOption) + fs := walker.NewFS() + artifactArtifact, cleanup, err := repo.NewArtifact(url, artifactCache, fs, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } @@ -124,14 +127,15 @@ func initializeSBOMScanner(ctx context.Context, filePath string, artifactCache c }, nil } -func initializeVMScanner(ctx context.Context, filePath string, artifactCache cache.ArtifactCache, localArtifactCache cache.LocalArtifactCache, walker vm.Walker, artifactOption artifact.Option) (scanner.Scanner, func(), error) { +func initializeVMScanner(ctx context.Context, filePath string, artifactCache cache.ArtifactCache, localArtifactCache cache.LocalArtifactCache, artifactOption artifact.Option) (scanner.Scanner, func(), error) { applierApplier := applier.NewApplier(localArtifactCache) ospkgScanner := ospkg.NewScanner() langpkgScanner := langpkg.NewScanner() config := db.Config{} client := vulnerability.NewClient(config) localScanner := local.NewScanner(applierApplier, ospkgScanner, langpkgScanner, client) - artifactArtifact, err := vm.NewArtifact(filePath, artifactCache, walker, artifactOption) + walkerVM := walker.NewVM() + artifactArtifact, err := vm.NewArtifact(filePath, artifactCache, walkerVM, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } @@ -185,7 +189,8 @@ func initializeRemoteArchiveScanner(ctx context.Context, filePath string, artifa func initializeRemoteFilesystemScanner(ctx context.Context, path string, artifactCache cache.ArtifactCache, remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { v := _wireValue clientScanner := client.NewScanner(remoteScanOptions, v...) - artifactArtifact, err := local2.NewArtifact(path, artifactCache, artifactOption) + fs := walker.NewFS() + artifactArtifact, err := local2.NewArtifact(path, artifactCache, fs, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } @@ -198,7 +203,8 @@ func initializeRemoteFilesystemScanner(ctx context.Context, path string, artifac func initializeRemoteRepositoryScanner(ctx context.Context, url string, artifactCache cache.ArtifactCache, remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { v := _wireValue clientScanner := client.NewScanner(remoteScanOptions, v...) - artifactArtifact, cleanup, err := repo.NewArtifact(url, artifactCache, artifactOption) + fs := walker.NewFS() + artifactArtifact, cleanup, err := repo.NewArtifact(url, artifactCache, fs, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } @@ -222,10 +228,11 @@ func initializeRemoteSBOMScanner(ctx context.Context, path string, artifactCache } // initializeRemoteVMScanner is for vm scanning in client/server mode -func initializeRemoteVMScanner(ctx context.Context, path string, artifactCache cache.ArtifactCache, walker vm.Walker, remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { +func initializeRemoteVMScanner(ctx context.Context, path string, artifactCache cache.ArtifactCache, remoteScanOptions client.ScannerOption, artifactOption artifact.Option) (scanner.Scanner, func(), error) { v := _wireValue clientScanner := client.NewScanner(remoteScanOptions, v...) - artifactArtifact, err := vm.NewArtifact(path, artifactCache, walker, artifactOption) + walkerVM := walker.NewVM() + artifactArtifact, err := vm.NewArtifact(path, artifactCache, walkerVM, artifactOption) if err != nil { return scanner.Scanner{}, nil, err } diff --git a/pkg/fanal/analyzer/pkg/dpkg/copyright.go b/pkg/fanal/analyzer/pkg/dpkg/copyright.go index 193fd5efe8d6..1f50088f86b7 100644 --- a/pkg/fanal/analyzer/pkg/dpkg/copyright.go +++ b/pkg/fanal/analyzer/pkg/dpkg/copyright.go @@ -88,7 +88,7 @@ func (a *dpkgLicenseAnalyzer) parseCopyright(r xio.ReadSeekerAt) ([]types.Licens l := strings.TrimSpace(line[8:]) l = normalizeLicense(l) - if len(l) > 0 { + if l != "" { for _, lic := range licensing.SplitLicenses(l) { lic = licensing.Normalize(lic) if !slices.Contains(licenses, lic) { diff --git a/pkg/fanal/artifact/artifact.go b/pkg/fanal/artifact/artifact.go index bcb3250f8b7a..b8245f585389 100644 --- a/pkg/fanal/artifact/artifact.go +++ b/pkg/fanal/artifact/artifact.go @@ -14,16 +14,14 @@ type Option struct { AnalyzerGroup analyzer.Group // It is empty in OSS DisabledAnalyzers []analyzer.Type DisabledHandlers []types.HandlerType - SkipFiles []string - SkipDirs []string FilePatterns []string + Parallel int NoProgress bool Insecure bool Offline bool AppDirs []string SBOMSources []string RekorURL string - Parallel int AWSRegion string AWSEndpoint string FileChecksum bool // For SPDX @@ -40,14 +38,7 @@ type Option struct { SecretScannerOption analyzer.SecretScannerOption LicenseScannerOption analyzer.LicenseScannerOption - // File walk - WalkOption WalkOption -} - -// WalkOption is a struct that allows users to define a custom walking behavior. -// This option is only available when using Trivy as an imported library and not through CLI flags. -type WalkOption struct { - ErrorCallback walker.ErrorCallback + WalkerOption walker.Option } func (o *Option) AnalyzerOptions() analyzer.AnalyzerOptions { @@ -75,8 +66,8 @@ func (o *Option) Sort() { sort.Slice(o.DisabledAnalyzers, func(i, j int) bool { return o.DisabledAnalyzers[i] < o.DisabledAnalyzers[j] }) - sort.Strings(o.SkipFiles) - sort.Strings(o.SkipDirs) + sort.Strings(o.WalkerOption.SkipFiles) + sort.Strings(o.WalkerOption.SkipDirs) sort.Strings(o.FilePatterns) } diff --git a/pkg/fanal/artifact/image/image.go b/pkg/fanal/artifact/image/image.go index e22f8d7bb0a2..b0749ad0d1ed 100644 --- a/pkg/fanal/artifact/image/image.go +++ b/pkg/fanal/artifact/image/image.go @@ -64,7 +64,7 @@ func NewArtifact(img types.Image, c cache.ArtifactCache, opt artifact.Option) (a logger: log.WithPrefix("image"), image: img, cache: c, - walker: walker.NewLayerTar(opt.SkipFiles, opt.SkipDirs), + walker: walker.NewLayerTar(opt.WalkerOption), analyzer: a, configAnalyzer: ca, handlerManager: handlerManager, @@ -202,7 +202,8 @@ func (a Artifact) inspect(ctx context.Context, missingImage string, layerKeys, b layerKeyMap map[string]LayerInfo, configFile *v1.ConfigFile) error { var osFound types.OS - p := parallel.NewPipeline(a.artifactOption.Parallel, false, layerKeys, func(ctx context.Context, layerKey string) (any, error) { + p := parallel.NewPipeline(a.artifactOption.Parallel, false, layerKeys, func(ctx context.Context, + layerKey string) (any, error) { layer := layerKeyMap[layerKey] // If it is a base layer, secret scanning should not be performed. diff --git a/pkg/fanal/artifact/local/fs.go b/pkg/fanal/artifact/local/fs.go index ff72d01a72fd..5896196c0e48 100644 --- a/pkg/fanal/artifact/local/fs.go +++ b/pkg/fanal/artifact/local/fs.go @@ -10,6 +10,7 @@ import ( "strings" "sync" + "github.com/google/wire" "github.com/opencontainers/go-digest" "golang.org/x/xerrors" @@ -19,21 +20,34 @@ 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" ) +var ( + ArtifactSet = wire.NewSet( + walker.NewFS, + wire.Bind(new(Walker), new(*walker.FS)), + NewArtifact, + ) + + _ Walker = (*walker.FS)(nil) +) + +type Walker interface { + Walk(root string, opt walker.Option, fn walker.WalkFunc) error +} + type Artifact struct { rootPath string cache cache.ArtifactCache - walker walker.FS + walker Walker analyzer analyzer.AnalyzerGroup handlerManager handler.Manager artifactOption artifact.Option } -func NewArtifact(rootPath string, c cache.ArtifactCache, opt artifact.Option) (artifact.Artifact, error) { +func NewArtifact(rootPath string, c cache.ArtifactCache, w Walker, opt artifact.Option) (artifact.Artifact, error) { handlerManager, err := handler.NewManager(opt) if err != nil { return nil, xerrors.Errorf("handler initialize error: %w", err) @@ -45,72 +59,15 @@ func NewArtifact(rootPath string, c cache.ArtifactCache, opt artifact.Option) (a } return Artifact{ - rootPath: filepath.ToSlash(filepath.Clean(rootPath)), - cache: c, - walker: walker.NewFS(buildPathsToSkip(rootPath, opt.SkipFiles), buildPathsToSkip(rootPath, opt.SkipDirs), - opt.Parallel, opt.WalkOption.ErrorCallback), + rootPath: filepath.ToSlash(filepath.Clean(rootPath)), + cache: c, + walker: w, analyzer: a, handlerManager: handlerManager, - artifactOption: opt, }, nil } -// 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.Warn("Failed to get an absolute path", log.String("base", base), log.Err(err)) - return nil - } - 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.Warn("Failed to get an absolute path", log.String("base", base), log.Err(err)) - continue - } - rel, err := filepath.Rel(absBase, absSkipPath) - if err != nil { - log.Warn("Failed to get an relative path", log.String("base", base), log.Err(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) { var wg sync.WaitGroup result := analyzer.NewAnalysisResult() @@ -126,7 +83,7 @@ func (a Artifact) Inspect(ctx context.Context) (types.ArtifactReference, error) return types.ArtifactReference{}, xerrors.Errorf("failed to prepare filesystem for post analysis: %w", err) } - err = a.walker.Walk(a.rootPath, func(filePath string, info os.FileInfo, opener analyzer.Opener) error { + err = a.walker.Walk(a.rootPath, a.artifactOption.WalkerOption, func(filePath string, info os.FileInfo, opener analyzer.Opener) error { dir := a.rootPath // When the directory is the same as the filePath, a file was given diff --git a/pkg/fanal/artifact/local/fs_test.go b/pkg/fanal/artifact/local/fs_test.go index bbaf684552ae..890679b7f45d 100644 --- a/pkg/fanal/artifact/local/fs_test.go +++ b/pkg/fanal/artifact/local/fs_test.go @@ -3,20 +3,18 @@ package local import ( "context" "errors" + "github.com/aquasecurity/trivy/pkg/fanal/walker" "os" "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/artifact" "github.com/aquasecurity/trivy/pkg/fanal/cache" "github.com/aquasecurity/trivy/pkg/fanal/types" "github.com/aquasecurity/trivy/pkg/misconf" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" _ "github.com/aquasecurity/trivy/pkg/fanal/analyzer/config/all" _ "github.com/aquasecurity/trivy/pkg/fanal/analyzer/language/python/pip" @@ -245,7 +243,7 @@ func TestArtifact_Inspect(t *testing.T) { c := new(cache.MockArtifactCache) c.ApplyPutBlobExpectation(tt.putBlobExpectation) - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -261,107 +259,6 @@ func TestArtifact_Inspect(t *testing.T) { } } -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) - }) - } -} - var terraformPolicyMetadata = types.PolicyMetadata{ ID: "TEST001", AVDID: "AVD-TEST-0001", @@ -816,7 +713,7 @@ func TestTerraformMisconfigurationScan(t *testing.T) { types.SystemFileFilteringPostHandler, } tt.artifactOpt.MisconfScannerOption.DisableEmbeddedPolicies = true - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -1051,7 +948,8 @@ func TestTerraformPlanSnapshotMisconfScan(t *testing.T) { require.NoError(t, err) defer f.Close() - f.WriteString(emptyBucketCheck) + _, err = f.WriteString(emptyBucketCheck) + require.NoError(t, err) c := new(cache.MockArtifactCache) c.ApplyPutBlobExpectation(tt.putBlobExpectation) @@ -1060,7 +958,6 @@ func TestTerraformPlanSnapshotMisconfScan(t *testing.T) { DisabledHandlers: []types.HandlerType{ types.SystemFileFilteringPostHandler, }, - SkipFiles: []string{"*.tf"}, MisconfScannerOption: misconf.ScannerOption{ RegoOnly: true, DisableEmbeddedPolicies: true, @@ -1069,9 +966,12 @@ func TestTerraformPlanSnapshotMisconfScan(t *testing.T) { Namespaces: []string{"user"}, PolicyPaths: []string{tmpDir}, }, + WalkerOption: walker.Option{ + SkipFiles: []string{"*.tf"}, + }, } - a, err := NewArtifact(tt.fields.dir, c, opt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), opt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -1395,7 +1295,7 @@ func TestCloudFormationMisconfigurationScan(t *testing.T) { types.SystemFileFilteringPostHandler, } tt.artifactOpt.MisconfScannerOption.DisableEmbeddedPolicies = true - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -1630,7 +1530,7 @@ func TestDockerfileMisconfigurationScan(t *testing.T) { tt.artifactOpt.DisabledHandlers = []types.HandlerType{ types.SystemFileFilteringPostHandler, } - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -1898,7 +1798,7 @@ func TestKubernetesMisconfigurationScan(t *testing.T) { tt.artifactOpt.DisabledHandlers = []types.HandlerType{ types.SystemFileFilteringPostHandler, } - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -2155,7 +2055,7 @@ func TestAzureARMMisconfigurationScan(t *testing.T) { tt.artifactOpt.DisabledHandlers = []types.HandlerType{ types.SystemFileFilteringPostHandler, } - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) @@ -2271,7 +2171,7 @@ func TestMixedConfigurationScan(t *testing.T) { tt.artifactOpt.DisabledHandlers = []types.HandlerType{ types.SystemFileFilteringPostHandler, } - a, err := NewArtifact(tt.fields.dir, c, tt.artifactOpt) + a, err := NewArtifact(tt.fields.dir, c, walker.NewFS(), tt.artifactOpt) require.NoError(t, err) got, err := a.Inspect(context.Background()) diff --git a/pkg/fanal/artifact/repo/git.go b/pkg/fanal/artifact/repo/git.go index de1a528f65dc..977c75d71bb7 100644 --- a/pkg/fanal/artifact/repo/git.go +++ b/pkg/fanal/artifact/repo/git.go @@ -8,6 +8,7 @@ import ( "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/transport/http" + "github.com/google/wire" "github.com/hashicorp/go-multierror" "golang.org/x/xerrors" @@ -15,28 +16,43 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/artifact/local" "github.com/aquasecurity/trivy/pkg/fanal/cache" "github.com/aquasecurity/trivy/pkg/fanal/types" + "github.com/aquasecurity/trivy/pkg/fanal/walker" ) +var ( + ArtifactSet = wire.NewSet( + walker.NewFS, + wire.Bind(new(Walker), new(*walker.FS)), + NewArtifact, + ) + + _ Walker = (*walker.FS)(nil) +) + +type Walker interface { + Walk(root string, opt walker.Option, fn walker.WalkFunc) error +} + type Artifact struct { url string local artifact.Artifact } -func NewArtifact(target string, c cache.ArtifactCache, artifactOpt artifact.Option) ( +func NewArtifact(target string, c cache.ArtifactCache, w Walker, artifactOpt artifact.Option) ( artifact.Artifact, func(), error) { var cleanup func() var errs error // Try the local repository - art, err := tryLocalRepo(target, c, artifactOpt) + art, err := tryLocalRepo(target, c, w, artifactOpt) if err == nil { return art, func() {}, nil } errs = multierror.Append(errs, err) // Try the remote git repository - art, cleanup, err = tryRemoteRepo(target, c, artifactOpt) + art, cleanup, err = tryRemoteRepo(target, c, w, artifactOpt) if err == nil { return art, cleanup, nil } @@ -64,12 +80,12 @@ func (Artifact) Clean(_ types.ArtifactReference) error { return nil } -func tryLocalRepo(target string, c cache.ArtifactCache, artifactOpt artifact.Option) (artifact.Artifact, error) { +func tryLocalRepo(target string, c cache.ArtifactCache, w Walker, artifactOpt artifact.Option) (artifact.Artifact, error) { if _, err := os.Stat(target); err != nil { return nil, xerrors.Errorf("no such path: %w", err) } - art, err := local.NewArtifact(target, c, artifactOpt) + art, err := local.NewArtifact(target, c, w, artifactOpt) if err != nil { return nil, xerrors.Errorf("local repo artifact error: %w", err) } @@ -78,7 +94,7 @@ func tryLocalRepo(target string, c cache.ArtifactCache, artifactOpt artifact.Opt }, nil } -func tryRemoteRepo(target string, c cache.ArtifactCache, artifactOpt artifact.Option) (artifact.Artifact, func(), error) { +func tryRemoteRepo(target string, c cache.ArtifactCache, w Walker, artifactOpt artifact.Option) (artifact.Artifact, func(), error) { cleanup := func() {} u, err := newURL(target) if err != nil { @@ -92,7 +108,7 @@ func tryRemoteRepo(target string, c cache.ArtifactCache, artifactOpt artifact.Op cleanup = func() { _ = os.RemoveAll(tmpDir) } - art, err := local.NewArtifact(tmpDir, c, artifactOpt) + art, err := local.NewArtifact(tmpDir, c, w, artifactOpt) if err != nil { return nil, cleanup, xerrors.Errorf("fs artifact: %w", err) } diff --git a/pkg/fanal/artifact/repo/git_test.go b/pkg/fanal/artifact/repo/git_test.go index ab8221c2f9e8..de484e64d221 100644 --- a/pkg/fanal/artifact/repo/git_test.go +++ b/pkg/fanal/artifact/repo/git_test.go @@ -4,6 +4,7 @@ package repo import ( "context" + "github.com/aquasecurity/trivy/pkg/fanal/walker" "net/http/httptest" "testing" @@ -165,7 +166,7 @@ func TestNewArtifact(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, cleanup, err := NewArtifact(tt.args.target, tt.args.c, artifact.Option{ + _, cleanup, err := NewArtifact(tt.args.target, tt.args.c, walker.NewFS(), artifact.Option{ NoProgress: tt.args.noProgress, RepoBranch: tt.args.repoBranch, RepoTag: tt.args.repoTag, @@ -207,7 +208,7 @@ func TestArtifact_Inspect(t *testing.T) { fsCache, err := cache.NewFSCache(t.TempDir()) require.NoError(t, err) - art, cleanup, err := NewArtifact(tt.rawurl, fsCache, artifact.Option{}) + art, cleanup, err := NewArtifact(tt.rawurl, fsCache, walker.NewFS(), artifact.Option{}) require.NoError(t, err) defer cleanup() diff --git a/pkg/fanal/artifact/vm/vm.go b/pkg/fanal/artifact/vm/vm.go index 5f0d1f5622cc..5b9aae130f4c 100644 --- a/pkg/fanal/artifact/vm/vm.go +++ b/pkg/fanal/artifact/vm/vm.go @@ -7,6 +7,7 @@ import ( "strings" "sync" + "github.com/google/wire" "golang.org/x/xerrors" "github.com/aquasecurity/trivy/pkg/fanal/analyzer" @@ -30,8 +31,18 @@ const ( TypeFile Type = "file" ) +var ( + ArtifactSet = wire.NewSet( + walker.NewVM, + wire.Bind(new(Walker), new(*walker.VM)), + NewArtifact, + ) + + _ Walker = (*walker.VM)(nil) +) + type Walker interface { - Walk(*io.SectionReader, string, walker.WalkFunc) error + Walk(*io.SectionReader, string, walker.Option, walker.WalkFunc) error } func NewArtifact(target string, c cache.ArtifactCache, w Walker, opt artifact.Option) (artifact.Artifact, error) { @@ -98,7 +109,7 @@ func (a *Storage) Analyze(ctx context.Context, r *io.SectionReader) (types.BlobI defer composite.Cleanup() // TODO: Always walk from the root directory. Consider whether there is a need to be able to set optional - err = a.walker.Walk(r, "/", func(filePath string, info os.FileInfo, opener analyzer.Opener) error { + err = a.walker.Walk(r, "/", a.artifactOption.WalkerOption, func(filePath string, info os.FileInfo, opener analyzer.Opener) error { path := strings.TrimPrefix(filePath, "/") if err := a.analyzer.AnalyzeFile(ctx, &wg, limit, result, "/", path, info, opener, nil, opts); err != nil { return xerrors.Errorf("analyze file (%s): %w", path, err) diff --git a/pkg/fanal/artifact/vm/vm_test.go b/pkg/fanal/artifact/vm/vm_test.go index 2c47c756e6e5..445707c8739b 100644 --- a/pkg/fanal/artifact/vm/vm_test.go +++ b/pkg/fanal/artifact/vm/vm_test.go @@ -36,7 +36,7 @@ type mockWalker struct { root string } -func (m *mockWalker) Walk(_ *io.SectionReader, _ string, fn walker.WalkFunc) error { +func (m *mockWalker) Walk(_ *io.SectionReader, _ string, _ walker.Option, fn walker.WalkFunc) error { return filepath.WalkDir(m.root, func(path string, d fs.DirEntry, err error) error { if err != nil { return err @@ -94,7 +94,7 @@ func TestNewArtifact(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { w := &mockWalker{root: "testdata"} - _, err := vm.NewArtifact(tt.target, nil, w, artifact.Option{}) + _, err := vm.NewArtifact(tt.target, nil, w, artifact.Option{Parallel: 3}) tt.wantErr(t, err, fmt.Sprintf("NewArtifact(%v, nil, nil)", tt.target)) }) } diff --git a/pkg/fanal/cache/key.go b/pkg/fanal/cache/key.go index f9258caa866b..b11cfc4be1df 100644 --- a/pkg/fanal/cache/key.go +++ b/pkg/fanal/cache/key.go @@ -30,7 +30,7 @@ func CalcKey(id string, analyzerVersions analyzer.Versions, hookVersions map[str SkipFiles []string SkipDirs []string FilePatterns []string `json:",omitempty"` - }{id, analyzerVersions, hookVersions, artifactOpt.SkipFiles, artifactOpt.SkipDirs, artifactOpt.FilePatterns} + }{id, analyzerVersions, hookVersions, artifactOpt.WalkerOption.SkipFiles, artifactOpt.WalkerOption.SkipDirs, artifactOpt.FilePatterns} if err := json.NewEncoder(h).Encode(keyBase); err != nil { return "", xerrors.Errorf("json encode error: %w", err) diff --git a/pkg/fanal/cache/key_test.go b/pkg/fanal/cache/key_test.go index 94828e5d485e..f21910b275ae 100644 --- a/pkg/fanal/cache/key_test.go +++ b/pkg/fanal/cache/key_test.go @@ -1,6 +1,7 @@ package cache import ( + "github.com/aquasecurity/trivy/pkg/fanal/walker" "testing" "github.com/stretchr/testify/assert" @@ -230,8 +231,6 @@ func TestCalcKey(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { artifactOpt := artifact.Option{ - SkipFiles: tt.args.skipFiles, - SkipDirs: tt.args.skipDirs, FilePatterns: tt.args.patterns, MisconfScannerOption: misconf.ScannerOption{ @@ -242,6 +241,11 @@ func TestCalcKey(t *testing.T) { SecretScannerOption: analyzer.SecretScannerOption{ ConfigPath: tt.args.secretConfigPath, }, + + WalkerOption: walker.Option{ + SkipFiles: tt.args.skipFiles, + SkipDirs: tt.args.skipDirs, + }, } got, err := CalcKey(tt.args.key, tt.args.analyzerVersions, tt.args.hookVersions, artifactOpt) if tt.wantErr != "" { diff --git a/pkg/fanal/external/config_scan.go b/pkg/fanal/external/config_scan.go deleted file mode 100644 index 43f9ede430f4..000000000000 --- a/pkg/fanal/external/config_scan.go +++ /dev/null @@ -1,79 +0,0 @@ -package external - -import ( - "context" - "errors" - - "github.com/aquasecurity/trivy/pkg/fanal/analyzer" - "github.com/aquasecurity/trivy/pkg/fanal/applier" - "github.com/aquasecurity/trivy/pkg/fanal/artifact" - "github.com/aquasecurity/trivy/pkg/fanal/artifact/local" - "github.com/aquasecurity/trivy/pkg/fanal/cache" - "github.com/aquasecurity/trivy/pkg/fanal/types" - "github.com/aquasecurity/trivy/pkg/misconf" - - _ "github.com/aquasecurity/trivy/pkg/fanal/analyzer/config/all" -) - -type ConfigScanner struct { - cache cache.FSCache - policyPaths []string - dataPaths []string - namespaces []string - allowEmbedded bool -} - -func NewConfigScanner(cacheDir string, policyPaths, dataPaths, namespaces []string, allowEmbedded bool) (*ConfigScanner, error) { - // Initialize local cache - cacheClient, err := cache.NewFSCache(cacheDir) - if err != nil { - return nil, err - } - - return &ConfigScanner{ - cache: cacheClient, - policyPaths: policyPaths, - dataPaths: dataPaths, - namespaces: namespaces, - allowEmbedded: allowEmbedded, - }, nil -} - -func (s ConfigScanner) Scan(dir string) ([]types.Misconfiguration, error) { - art, err := local.NewArtifact(dir, s.cache, artifact.Option{ - MisconfScannerOption: misconf.ScannerOption{ - PolicyPaths: s.policyPaths, - DataPaths: s.dataPaths, - Namespaces: s.namespaces, - DisableEmbeddedPolicies: !s.allowEmbedded, - DisableEmbeddedLibraries: !s.allowEmbedded, - }, - }) - if err != nil { - return nil, err - } - - // Scan config files - result, err := art.Inspect(context.Background()) - if err != nil { - return nil, err - } - - // Merge layers - a := applier.NewApplier(s.cache) - mergedLayer, err := a.ApplyLayers(result.ID, result.BlobIDs) - if !errors.Is(err, analyzer.ErrUnknownOS) && !errors.Is(err, analyzer.ErrNoPkgsDetected) { - return nil, err - } - - // Do not assert successes and layer - for i := range mergedLayer.Misconfigurations { - mergedLayer.Misconfigurations[i].Layer = types.Layer{} - } - - return mergedLayer.Misconfigurations, nil -} - -func (s ConfigScanner) Close() error { - return s.cache.Close() -} diff --git a/pkg/fanal/external/config_scan_test.go b/pkg/fanal/external/config_scan_test.go deleted file mode 100644 index 094fdf41accf..000000000000 --- a/pkg/fanal/external/config_scan_test.go +++ /dev/null @@ -1,143 +0,0 @@ -package external_test - -import ( - "path/filepath" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/aquasecurity/trivy/pkg/fanal/external" - "github.com/aquasecurity/trivy/pkg/fanal/types" - - _ "github.com/aquasecurity/trivy/pkg/fanal/analyzer/config/all" -) - -func TestConfigScanner_Scan(t *testing.T) { - type fields struct { - policyPaths []string - dataPaths []string - namespaces []string - } - tests := []struct { - name string - fields fields - inputDir string - want []types.Misconfiguration - }{ - { - name: "deny", - fields: fields{ - policyPaths: []string{filepath.Join("testdata", "deny")}, - namespaces: []string{"testdata"}, - }, - inputDir: filepath.Join("testdata", "deny"), - want: []types.Misconfiguration{ - { - FileType: "dockerfile", - FilePath: "Dockerfile", - Failures: types.MisconfResults{ - types.MisconfResult{ - Namespace: "testdata.xyz_200", - Query: "data.testdata.xyz_200.deny", - Message: "Old image", - PolicyMetadata: types.PolicyMetadata{ - ID: "XYZ-200", - Type: "Dockerfile Security Check", - Title: "Old FROM", - Description: "Rego module: data.testdata.xyz_200", - Severity: "LOW", - RecommendedActions: "", - References: []string(nil), - }, - CauseMetadata: types.CauseMetadata{ - Resource: "", - Provider: "Dockerfile", - Service: "general", - StartLine: 1, - EndLine: 2, - Code: types.Code{ - Lines: []types.Line{ - { - Number: 1, - Content: "FROM alpine:3.10", - Highlighted: "\x1b[38;5;64mFROM\x1b[0m\x1b[38;5;37m alpine:3.10", - IsCause: true, - Annotation: "", - Truncated: false, - FirstCause: true, - LastCause: false, - }, - { - Number: 2, - Content: "", - Highlighted: "\x1b[0m", - IsCause: true, - Annotation: "", - Truncated: false, - FirstCause: false, - LastCause: true, - }, - }, - }, - }, Traces: []string(nil), - }, - }, Warnings: types.MisconfResults(nil), - Successes: types.MisconfResults(nil), - Exceptions: types.MisconfResults(nil), - Layer: types.Layer{ - Digest: "", - DiffID: "", - }, - }, - }, - }, - { - name: "allow", - fields: fields{ - policyPaths: []string{filepath.Join("testdata", "allow")}, - namespaces: []string{"testdata"}, - }, - inputDir: filepath.Join("testdata", "allow"), - want: []types.Misconfiguration{ - { - FileType: "dockerfile", - FilePath: "Dockerfile", - Successes: types.MisconfResults{ - { - Namespace: "testdata.xyz_200", - Query: "data.testdata.xyz_200.deny", - PolicyMetadata: types.PolicyMetadata{ - ID: "XYZ-200", - Type: "Dockerfile Security Check", - Title: "Old FROM", - Description: "Rego module: data.testdata.xyz_200", - Severity: "LOW", - }, - CauseMetadata: types.CauseMetadata{ - Resource: "", - Provider: "Dockerfile", - Service: "general", - StartLine: 0, - EndLine: 0, - }, - }, - }, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - s, err := external.NewConfigScanner(t.TempDir(), - tt.fields.policyPaths, tt.fields.dataPaths, tt.fields.namespaces, false) - require.NoError(t, err) - - defer func() { _ = s.Close() }() - - got, err := s.Scan(tt.inputDir) - require.NoError(t, err) - assert.Equal(t, tt.want, got) - }) - } -} diff --git a/pkg/fanal/external/testdata/allow/Dockerfile b/pkg/fanal/external/testdata/allow/Dockerfile deleted file mode 100644 index fb7daf440fc6..000000000000 --- a/pkg/fanal/external/testdata/allow/Dockerfile +++ /dev/null @@ -1,3 +0,0 @@ -FROM alpine:3.14 - -ADD foo.txt . \ No newline at end of file diff --git a/pkg/fanal/external/testdata/allow/docker.rego b/pkg/fanal/external/testdata/allow/docker.rego deleted file mode 100644 index 0ef57f138813..000000000000 --- a/pkg/fanal/external/testdata/allow/docker.rego +++ /dev/null @@ -1,20 +0,0 @@ -package testdata.xyz_200 - -__rego_metadata__ := { - "id": "XYZ-200", - "title": "Old FROM", - "version": "v1.0.0", - "severity": "LOW", - "type": "Docker Security Check", -} - -__rego_input__ := { - "combine": false, - "selector": [{"type": "dockerfile"}], -} - -deny[msg] { - input.stages[from] - from == "alpine:3.10" - msg := "Old image" -} diff --git a/pkg/fanal/external/testdata/deny/Dockerfile b/pkg/fanal/external/testdata/deny/Dockerfile deleted file mode 100644 index b3be16e28eac..000000000000 --- a/pkg/fanal/external/testdata/deny/Dockerfile +++ /dev/null @@ -1,3 +0,0 @@ -FROM alpine:3.10 - -ADD foo.txt . \ No newline at end of file diff --git a/pkg/fanal/external/testdata/deny/docker.rego b/pkg/fanal/external/testdata/deny/docker.rego deleted file mode 100644 index 6be4af8ee57e..000000000000 --- a/pkg/fanal/external/testdata/deny/docker.rego +++ /dev/null @@ -1,25 +0,0 @@ -package testdata.xyz_200 - -__rego_metadata__ := { - "id": "XYZ-200", - "title": "Old FROM", - "version": "v1.0.0", - "severity": "LOW", - "type": "Docker Security Check", -} - -__rego_input__ := { - "combine": false, - "selector": [{"type": "dockerfile"}], -} - -deny[res] { - stage := input.Stages[_] - stage.Name == "alpine:3.10" - msg := "Old image" - res := { - "msg": msg, - "startline": 1, - "endline": 2, - } -} diff --git a/pkg/fanal/test/integration/library_test.go b/pkg/fanal/test/integration/library_test.go index b802084ba314..f6a863fc82bd 100644 --- a/pkg/fanal/test/integration/library_test.go +++ b/pkg/fanal/test/integration/library_test.go @@ -1,5 +1,4 @@ //go:build integration -// +build integration package integration diff --git a/pkg/fanal/walker/cached_file.go b/pkg/fanal/walker/cached_file.go index 05cfa1443c39..fc963c010409 100644 --- a/pkg/fanal/walker/cached_file.go +++ b/pkg/fanal/walker/cached_file.go @@ -19,17 +19,14 @@ type cachedFile struct { size int64 reader io.Reader - threshold int64 //　Files larger than this threshold are written to file without being read into memory. - content []byte // It will be populated if this file is small filePath string // It will be populated if this file is large } -func newCachedFile(size int64, r io.Reader, threshold int64) *cachedFile { +func newCachedFile(size int64, r io.Reader) *cachedFile { return &cachedFile{ - size: size, - reader: r, - threshold: threshold, + size: size, + reader: r, } } @@ -39,7 +36,7 @@ func newCachedFile(size int64, r io.Reader, threshold int64) *cachedFile { func (o *cachedFile) Open() (xio.ReadSeekCloserAt, error) { o.once.Do(func() { // When the file is large, it will be written down to a temp file. - if o.size >= o.threshold { + if o.size >= defaultSizeThreshold { f, err := os.CreateTemp("", "fanal-*") if err != nil { o.err = xerrors.Errorf("failed to create the temp file: %w", err) diff --git a/pkg/fanal/walker/fs.go b/pkg/fanal/walker/fs.go index 88a623ed1792..8255585192d9 100644 --- a/pkg/fanal/walker/fs.go +++ b/pkg/fanal/walker/fs.go @@ -1,121 +1,160 @@ package walker import ( + "errors" "io/fs" "os" "path/filepath" + "strings" - swalker "github.com/saracen/walker" "golang.org/x/xerrors" "github.com/aquasecurity/trivy/pkg/log" xio "github.com/aquasecurity/trivy/pkg/x/io" ) -type ErrorCallback func(pathname string, err error) error +// FS is the filesystem walker +type FS struct{} -type FS struct { - walker - parallel int - errCallback ErrorCallback +func NewFS() *FS { + return &FS{} } -func NewFS(skipFiles, skipDirs []string, parallel int, errCallback ErrorCallback) FS { - if errCallback == nil { - errCallback = func(pathname string, err error) error { - // ignore permission errors - if os.IsPermission(err) { - return nil - } - // halt traversal on any other error - return xerrors.Errorf("unknown error with %s: %w", pathname, err) - } - } +// Walk walks the filesystem rooted at root, calling fn for each unfiltered file. +func (w *FS) Walk(root string, opt Option, fn WalkFunc) error { + opt.SkipFiles = w.BuildSkipPaths(root, opt.SkipFiles) + opt.SkipDirs = append(opt.SkipDirs, defaultSkipDirs...) + opt.SkipDirs = w.BuildSkipPaths(root, opt.SkipDirs) + + walkDirFunc := w.WalkDirFunc(root, fn, opt) + walkDirFunc = w.onError(walkDirFunc) - return FS{ - walker: newWalker(skipFiles, skipDirs), - parallel: parallel, - errCallback: errCallback, + // Walk the filesystem + if err := filepath.WalkDir(root, walkDirFunc); err != nil { + return xerrors.Errorf("walk dir error: %w", err) } + + return nil } -// Walk walks the file tree rooted at root, calling WalkFunc for each file or -// directory in the tree, including root, but a directory to be ignored will be skipped. -func (w FS) Walk(root string, fn WalkFunc) error { - // walk function called for every path found - walkFn := func(pathname string, fi os.FileInfo) error { - pathname = filepath.Clean(pathname) +func (w *FS) WalkDirFunc(root string, fn WalkFunc, opt Option) fs.WalkDirFunc { + return func(filePath string, d fs.DirEntry, err error) error { + if err != nil { + return err + } // For exported rootfs (e.g. images/alpine/etc/alpine-release) - relPath, err := filepath.Rel(root, pathname) + relPath, err := filepath.Rel(root, filePath) if err != nil { return xerrors.Errorf("filepath rel (%s): %w", relPath, err) } relPath = filepath.ToSlash(relPath) + info, err := d.Info() + if err != nil { + return xerrors.Errorf("file info error: %w", err) + } + + // Skip unnecessary files switch { - case fi.IsDir(): - if w.shouldSkipDir(relPath) { + case info.IsDir(): + if SkipPath(relPath, opt.SkipDirs) { return filepath.SkipDir } return nil - case !fi.Mode().IsRegular(): + case !info.Mode().IsRegular(): return nil - case w.shouldSkipFile(relPath): + case SkipPath(relPath, opt.SkipFiles): return nil } - if err = fn(relPath, fi, w.fileOpener(pathname)); err != nil { + if err = fn(relPath, info, fileOpener(filePath)); err != nil { return xerrors.Errorf("failed to analyze file: %w", err) } - return nil - } - if w.parallel <= 1 { - // In series: fast, with higher CPU/memory - return w.walkSlow(root, walkFn) + return nil } - - // In parallel: slow, with lower CPU/memory - return w.walkFast(root, walkFn) } -type fastWalkFunc func(pathname string, fi os.FileInfo) error - -func (w FS) walkFast(root string, walkFn fastWalkFunc) error { - // error function called for every error encountered - errorCallbackOption := swalker.WithErrorCallback(w.errCallback) - - // Multiple goroutines stat the filesystem concurrently. The provided - // walkFn must be safe for concurrent use. - log.Debug("Walking the file tree in parallel", log.String("root", root)) - if err := swalker.Walk(root, walkFn, errorCallbackOption); err != nil { - return xerrors.Errorf("walk error: %w", err) +func (w *FS) onError(wrapped fs.WalkDirFunc) fs.WalkDirFunc { + return func(filePath string, d fs.DirEntry, err error) error { + err = wrapped(filePath, d, err) + switch { + // Unwrap fs.SkipDir error + case errors.Is(err, fs.SkipDir): + return fs.SkipDir + // ignore permission errors + case os.IsPermission(err): + return nil + case err != nil: + // halt traversal on any other error + return xerrors.Errorf("unknown error with %s: %w", filePath, err) + } + return nil } - return nil } -func (w FS) walkSlow(root string, walkFn fastWalkFunc) error { - log.Debug("Walking the file tree in series", log.String("root", root)) - err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { +// BuildSkipPaths builds correct patch for defaultSkipDirs and skipFiles +func (w *FS) BuildSkipPaths(base string, paths []string) []string { + var relativePaths []string + absBase, err := filepath.Abs(base) + if err != nil { + log.Warn("Failed to get an absolute path", log.String("base", base), log.Err(err)) + return nil + } + 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 { - return w.errCallback(path, err) + log.Warn("Failed to get an absolute path", log.String("base", base), log.Err(err)) + continue } - info, err := d.Info() + rel, err := filepath.Rel(absBase, absSkipPath) if err != nil { - return xerrors.Errorf("file info error: %w", err) + log.Warn("Failed to get a relative path", log.String("from", base), + log.String("to", path), log.Err(err)) + continue } - return walkFn(path, info) - }) - if err != nil { - return xerrors.Errorf("walk dir error: %w", err) + + 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 nil + + relativePaths = CleanSkipPaths(relativePaths) + return relativePaths } // fileOpener returns a function opening a file. -func (w *walker) fileOpener(pathname string) func() (xio.ReadSeekCloserAt, error) { +func fileOpener(filePath string) func() (xio.ReadSeekCloserAt, error) { return func() (xio.ReadSeekCloserAt, error) { - return os.Open(pathname) + return os.Open(filePath) } } diff --git a/pkg/fanal/walker/fs_test.go b/pkg/fanal/walker/fs_test.go index aa84f6d23503..7af5b52020f8 100644 --- a/pkg/fanal/walker/fs_test.go +++ b/pkg/fanal/walker/fs_test.go @@ -2,8 +2,11 @@ package walker_test import ( "errors" + "golang.org/x/exp/slices" "io" "os" + "path/filepath" + "runtime" "strings" "testing" @@ -14,15 +17,10 @@ import ( "github.com/aquasecurity/trivy/pkg/fanal/walker" ) -func TestDir_Walk(t *testing.T) { - type fields struct { - skipFiles []string - skipDirs []string - errCallback walker.ErrorCallback - } +func TestFS_Walk(t *testing.T) { tests := []struct { name string - fields fields + option walker.Option rootDir string analyzeFn walker.WalkFunc wantErr string @@ -46,8 +44,8 @@ func TestDir_Walk(t *testing.T) { { name: "skip file", rootDir: "testdata/fs", - fields: fields{ - skipFiles: []string{"testdata/fs/bar"}, + option: walker.Option{ + SkipFiles: []string{"testdata/fs/bar"}, }, analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { if filePath == "testdata/fs/bar" { @@ -59,8 +57,8 @@ func TestDir_Walk(t *testing.T) { { name: "skip dir", rootDir: "testdata/fs/", - fields: fields{ - skipDirs: []string{"/testdata/fs/app"}, + option: walker.Option{ + SkipDirs: []string{"/testdata/fs/app"}, }, analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { if strings.HasPrefix(filePath, "testdata/fs/app") { @@ -69,23 +67,10 @@ func TestDir_Walk(t *testing.T) { return nil }, }, - { - name: "ignore all errors", - rootDir: "testdata/fs/nosuch", - fields: fields{ - errCallback: func(pathname string, err error) error { - return nil - }, - }, - analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { - // Ignore errors - return nil - }, - }, { name: "sad path", rootDir: "testdata/fs", - analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { + analyzeFn: func(string, os.FileInfo, analyzer.Opener) error { return errors.New("error") }, wantErr: "failed to analyze file", @@ -93,15 +78,114 @@ func TestDir_Walk(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - w := walker.NewFS(tt.fields.skipFiles, tt.fields.skipDirs, 1, tt.fields.errCallback) - - err := w.Walk(tt.rootDir, tt.analyzeFn) + w := walker.NewFS() + err := w.Walk(tt.rootDir, tt.option, tt.analyzeFn) if tt.wantErr != "" { - require.Error(t, err) - assert.Contains(t, err.Error(), tt.wantErr) + assert.ErrorContains(t, err, tt.wantErr) return } assert.NoError(t, err) }) } } + +func TestFS_BuildSkipPaths(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 := walker.NewFS().BuildSkipPaths(tt.base, tt.paths) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/pkg/fanal/walker/tar.go b/pkg/fanal/walker/tar.go index d548d06f9c0a..51f185d89c50 100644 --- a/pkg/fanal/walker/tar.go +++ b/pkg/fanal/walker/tar.go @@ -21,20 +21,19 @@ const ( var parentDir = ".." + utils.PathSeparator type LayerTar struct { - walker - threshold int64 + skipFiles []string + skipDirs []string } -func NewLayerTar(skipFiles, skipDirs []string) LayerTar { - threshold := defaultSizeThreshold +func NewLayerTar(opt Option) LayerTar { return LayerTar{ - walker: newWalker(skipFiles, skipDirs), - threshold: threshold, + skipFiles: CleanSkipPaths(opt.SkipFiles), + skipDirs: CleanSkipPaths(opt.SkipDirs), } } func (w LayerTar) Walk(layer io.Reader, analyzeFn WalkFunc) ([]string, []string, error) { - var opqDirs, whFiles, skipDirs []string + var opqDirs, whFiles, skippedDirs []string tr := tar.NewReader(layer) for { hdr, err := tr.Next() @@ -64,12 +63,12 @@ func (w LayerTar) Walk(layer io.Reader, analyzeFn WalkFunc) ([]string, []string, switch hdr.Typeflag { case tar.TypeDir: - if w.shouldSkipDir(filePath) { - skipDirs = append(skipDirs, filePath) + if SkipPath(filePath, w.skipDirs) { + skippedDirs = append(skippedDirs, filePath) continue } case tar.TypeReg: - if w.shouldSkipFile(filePath) { + if SkipPath(filePath, w.skipFiles) { continue } // symlinks and hardlinks have no content in reader, skip them @@ -77,7 +76,7 @@ func (w LayerTar) Walk(layer io.Reader, analyzeFn WalkFunc) ([]string, []string, continue } - if underSkippedDir(filePath, skipDirs) { + if underSkippedDir(filePath, skippedDirs) { continue } @@ -90,7 +89,7 @@ func (w LayerTar) Walk(layer io.Reader, analyzeFn WalkFunc) ([]string, []string, } func (w LayerTar) processFile(filePath string, tr *tar.Reader, fi fs.FileInfo, analyzeFn WalkFunc) error { - cf := newCachedFile(fi.Size(), tr, w.threshold) + cf := newCachedFile(fi.Size(), tr) defer func() { // nolint _ = cf.Clean() diff --git a/pkg/fanal/walker/tar_test.go b/pkg/fanal/walker/tar_test.go index fed2f9efea7d..79dc878e0118 100644 --- a/pkg/fanal/walker/tar_test.go +++ b/pkg/fanal/walker/tar_test.go @@ -15,13 +15,9 @@ import ( ) func TestLayerTar_Walk(t *testing.T) { - type fields struct { - skipFiles []string - skipDirs []string - } tests := []struct { name string - fields fields + option walker.Option inputFile string analyzeFn walker.WalkFunc wantOpqDirs []string @@ -40,8 +36,8 @@ func TestLayerTar_Walk(t *testing.T) { { name: "skip file", inputFile: filepath.Join("testdata", "test.tar"), - fields: fields{ - skipFiles: []string{"/app/myweb/index.html"}, + option: walker.Option{ + SkipFiles: []string{"/app/myweb/index.html"}, }, analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { if filePath == "app/myweb/index.html" { @@ -55,8 +51,8 @@ func TestLayerTar_Walk(t *testing.T) { { name: "skip dir", inputFile: filepath.Join("testdata", "test.tar"), - fields: fields{ - skipDirs: []string{"/app"}, + option: walker.Option{ + SkipDirs: []string{"/app"}, }, analyzeFn: func(filePath string, info os.FileInfo, opener analyzer.Opener) error { if strings.HasPrefix(filePath, "app") { @@ -81,8 +77,7 @@ func TestLayerTar_Walk(t *testing.T) { f, err := os.Open("testdata/test.tar") require.NoError(t, err) - w := walker.NewLayerTar(tt.fields.skipFiles, tt.fields.skipDirs) - + w := walker.NewLayerTar(tt.option) gotOpqDirs, gotWhFiles, err := w.Walk(f, tt.analyzeFn) if tt.wantErr != "" { require.Error(t, err) diff --git a/pkg/fanal/walker/vm.go b/pkg/fanal/walker/vm.go index 42206fa5efcc..6ff32564cc0d 100644 --- a/pkg/fanal/walker/vm.go +++ b/pkg/fanal/walker/vm.go @@ -34,22 +34,22 @@ func AppendPermitDiskName(s ...string) { } type VM struct { - walker logger *log.Logger - threshold int64 + skipFiles []string + skipDirs []string analyzeFn WalkFunc } -func NewVM(skipFiles, skipDirs []string) *VM { - threshold := defaultSizeThreshold +func NewVM() *VM { return &VM{ - logger: log.WithPrefix("vm"), - walker: newWalker(skipFiles, skipDirs), - threshold: threshold, + logger: log.WithPrefix("vm"), } } -func (w *VM) Walk(vreader *io.SectionReader, root string, fn WalkFunc) error { +func (w *VM) Walk(vreader *io.SectionReader, root string, opt Option, fn WalkFunc) error { + w.skipFiles = opt.SkipFiles + w.skipDirs = append(opt.SkipDirs, defaultSkipDirs...) + // This function will be called on each file. w.analyzeFn = fn @@ -123,13 +123,13 @@ func (w *VM) fsWalk(fsys fs.FS, path string, d fs.DirEntry, err error) error { pathName := strings.TrimPrefix(filepath.Clean(path), "/") switch { case fi.IsDir(): - if w.shouldSkipDir(pathName) { + if SkipPath(pathName, w.skipDirs) { return filepath.SkipDir } return nil case !fi.Mode().IsRegular(): return nil - case w.shouldSkipFile(pathName): + case SkipPath(pathName, w.skipFiles): return nil case fi.Mode()&0x1000 == 0x1000 || fi.Mode()&0x2000 == 0x2000 || @@ -144,7 +144,7 @@ func (w *VM) fsWalk(fsys fs.FS, path string, d fs.DirEntry, err error) error { return nil } - cvf := newCachedVMFile(fsys, pathName, w.threshold) + cvf := newCachedVMFile(fsys, pathName) defer cvf.Clean() if err = w.analyzeFn(path, fi, cvf.Open); err != nil { @@ -154,18 +154,16 @@ func (w *VM) fsWalk(fsys fs.FS, path string, d fs.DirEntry, err error) error { } type cachedVMFile struct { - fs fs.FS - filePath string - threshold int64 + fs fs.FS + filePath string cf *cachedFile } -func newCachedVMFile(fsys fs.FS, filePath string, threshold int64) *cachedVMFile { +func newCachedVMFile(fsys fs.FS, filePath string) *cachedVMFile { return &cachedVMFile{ - fs: fsys, - filePath: filePath, - threshold: threshold, + fs: fsys, + filePath: filePath, } } @@ -183,7 +181,7 @@ func (cvf *cachedVMFile) Open() (xio.ReadSeekCloserAt, error) { return nil, xerrors.Errorf("file stat error: %w", err) } - cvf.cf = newCachedFile(fi.Size(), f, cvf.threshold) + cvf.cf = newCachedFile(fi.Size(), f) return cvf.cf.Open() } diff --git a/pkg/fanal/walker/walk.go b/pkg/fanal/walker/walk.go index 7d7d71e702f7..52c13808d86b 100644 --- a/pkg/fanal/walker/walk.go +++ b/pkg/fanal/walker/walk.go @@ -6,85 +6,47 @@ import ( "strings" "github.com/bmatcuk/doublestar/v4" + "github.com/samber/lo" "github.com/aquasecurity/trivy/pkg/fanal/analyzer" - "github.com/aquasecurity/trivy/pkg/fanal/utils" "github.com/aquasecurity/trivy/pkg/log" ) -var ( - // These variables are exported so that a tool importing Trivy as a library can override these values. - AppDirs = []string{".git"} - SystemDirs = []string{ - "proc", - "sys", - "dev", - } -) - -const defaultSizeThreshold = int64(200) << 20 // 200MB +const defaultSizeThreshold = int64(100) << 20 // 200MB -type WalkFunc func(filePath string, info os.FileInfo, opener analyzer.Opener) error - -type walker struct { - skipFiles []string - skipDirs []string +var defaultSkipDirs = []string{ + "**/.git", + "proc", + "sys", + "dev", } -func newWalker(skipFiles, skipDirs []string) walker { - var cleanSkipFiles, cleanSkipDirs []string - for _, skipFile := range skipFiles { - skipFile = filepath.ToSlash(filepath.Clean(skipFile)) - skipFile = strings.TrimLeft(skipFile, "/") - cleanSkipFiles = append(cleanSkipFiles, skipFile) - } +type Option struct { + SkipFiles []string + SkipDirs []string +} - for _, skipDir := range append(skipDirs, SystemDirs...) { - skipDir = filepath.ToSlash(filepath.Clean(skipDir)) - skipDir = strings.TrimLeft(skipDir, "/") - cleanSkipDirs = append(cleanSkipDirs, skipDir) - } +type WalkFunc func(filePath string, info os.FileInfo, opener analyzer.Opener) error - return walker{ - skipFiles: cleanSkipFiles, - skipDirs: cleanSkipDirs, - } +func CleanSkipPaths(skipPaths []string) []string { + return lo.Map(skipPaths, func(skipPath string, index int) string { + skipPath = filepath.ToSlash(filepath.Clean(skipPath)) + return strings.TrimLeft(skipPath, "/") + }) } -func (w *walker) shouldSkipFile(filePath string) bool { - filePath = strings.TrimLeft(filePath, "/") +func SkipPath(path string, skipPaths []string) bool { + path = strings.TrimLeft(path, "/") // skip files - for _, pattern := range w.skipFiles { - match, err := doublestar.Match(pattern, filePath) + for _, pattern := range skipPaths { + match, err := doublestar.Match(pattern, path) if err != nil { return false // return early if bad pattern } else if match { - log.Debug("Skipping file", log.String("file_path", filePath)) - return true - } - } - return false -} - -func (w *walker) shouldSkipDir(dir string) bool { - dir = strings.TrimLeft(dir, "/") - - // Skip application dirs (relative path) - base := filepath.Base(dir) - if utils.StringInSlice(base, AppDirs) { - return true - } - - // Skip system dirs and specified dirs (absolute path) - for _, pattern := range w.skipDirs { - if match, err := doublestar.Match(pattern, dir); err != nil { - return false // return early if bad pattern - } else if match { - log.Debug("Skipping directory", log.String("dir", dir)) + log.Debug("Skipping path", log.String("path", path)) return true } } - return false } diff --git a/pkg/fanal/walker/walk_test.go b/pkg/fanal/walker/walk_test.go index 4d52117aa1ce..765f5c07d7b3 100644 --- a/pkg/fanal/walker/walk_test.go +++ b/pkg/fanal/walker/walk_test.go @@ -1,123 +1,144 @@ -package walker +package walker_test import ( "fmt" + "github.com/aquasecurity/trivy/pkg/fanal/walker" "path/filepath" "testing" "github.com/stretchr/testify/assert" ) -func Test_shouldSkipFile(t *testing.T) { - testCases := []struct { +func TestSkipFile(t *testing.T) { + tests := []struct { + name string skipFiles []string - skipMap map[string]bool + wants map[string]bool }{ { - skipFiles: []string{filepath.Join("/etc/*")}, - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): true, - filepath.Join("/etc/foo/bar"): false, + name: "single star", + skipFiles: []string{"/etc/*"}, + wants: map[string]bool{ + "/etc/foo": true, + "/etc/foo/bar": false, }, }, { - skipFiles: []string{filepath.Join("/etc/*/*")}, - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): true, + name: "two stars", + skipFiles: []string{"/etc/*/*"}, + wants: map[string]bool{ + "/etc/foo": false, + "/etc/foo/bar": true, }, }, { - skipFiles: []string{filepath.Join("**/*.txt")}, - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): false, - filepath.Join("/var/log/bar.txt"): true, + name: "double star", + skipFiles: []string{"**/*.txt"}, + wants: map[string]bool{ + "/etc/foo": false, + "/etc/foo/bar": false, + "/var/log/bar.txt": true, }, }, { - skipFiles: []string{filepath.Join("/etc/*/*"), filepath.Join("/var/log/*.txt")}, - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): true, - filepath.Join("/var/log/bar.txt"): true, + name: "multiple skip files", + skipFiles: []string{"/etc/*/*", "/var/log/*.txt"}, + wants: map[string]bool{ + "/etc/foo": false, + "/etc/foo/bar": true, + "/var/log/bar.txt": true, }, }, { - skipFiles: []string{filepath.Join(`[^etc`)}, // filepath.Match returns ErrBadPattern - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): false, + name: "error bad pattern", + skipFiles: []string{`[^etc`}, // filepath.Match returns ErrBadPattern + wants: map[string]bool{ + "/etc/foo": false, + "/etc/foo/bar": false, }, }, } - for i, tc := range testCases { - t.Run(fmt.Sprint(i), func(t *testing.T) { - w := newWalker(tc.skipFiles, nil) - for file, skipResult := range tc.skipMap { - assert.Equal(t, skipResult, w.shouldSkipFile(filepath.ToSlash(filepath.Clean(file))), fmt.Sprintf("skipFiles: %s, file: %s", tc.skipFiles, file)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for file, want := range tt.wants { + file = filepath.ToSlash(filepath.Clean(file)) + got := walker.SkipPath(file, walker.CleanSkipPaths(tt.skipFiles)) + assert.Equal(t, want, got, fmt.Sprintf("skipFiles: %s, file: %s", tt.skipFiles, file)) } }) } } -func Test_shouldSkipDir(t *testing.T) { - testCases := []struct { +func TestSkipDir(t *testing.T) { + tests := []struct { + name string skipDirs []string - skipMap map[string]bool + wants map[string]bool }{ { - skipDirs: nil, - skipMap: map[string]bool{ - ".git": true, // AppDir - "proc": true, // SystemDir - "foo.bar": false, // random directory + name: "default skip dirs", + skipDirs: []string{ + "**/.git", + "proc", + "sys", + "dev", + }, + wants: map[string]bool{ + ".git": true, + "proc": true, + "foo.bar": false, }, }, { - skipDirs: []string{filepath.Join("/*")}, - skipMap: map[string]bool{ - filepath.Join("/etc"): true, - filepath.Join("/etc/foo/bar"): false, + name: "single star", + skipDirs: []string{"/*"}, + wants: map[string]bool{ + "/etc": true, + "/etc/foo/bar": false, }, }, { + name: "two stars", skipDirs: []string{filepath.Join("/etc/*/*")}, - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): true, + wants: map[string]bool{ + "/etc/foo": false, + "/etc/foo/bar": true, }, }, { + name: "multiple dirs", skipDirs: []string{filepath.Join("/etc/*/*"), filepath.Join("/var/log/*")}, - skipMap: map[string]bool{ + wants: map[string]bool{ filepath.Join("/etc/foo"): false, filepath.Join("/etc/foo/bar"): true, filepath.Join("/var/log/bar"): true, }, }, { - skipDirs: []string{filepath.Join(`[^etc`)}, // filepath.Match returns ErrBadPattern - skipMap: map[string]bool{ - filepath.Join("/etc/foo"): false, - filepath.Join("/etc/foo/bar"): false, - }, - }, - { + name: "double star", skipDirs: []string{"**/.terraform"}, - skipMap: map[string]bool{ + wants: map[string]bool{ ".terraform": true, "test/foo/bar/.terraform": true, }, }, + { + name: "error bad pattern", + skipDirs: []string{filepath.Join(`[^etc`)}, // filepath.Match returns ErrBadPattern + wants: map[string]bool{ + filepath.Join("/etc/foo"): false, + filepath.Join("/etc/foo/bar"): false, + }, + }, } - for i, tc := range testCases { - t.Run(fmt.Sprint(i), func(t *testing.T) { - w := newWalker(nil, tc.skipDirs) - for dir, skipResult := range tc.skipMap { - assert.Equal(t, skipResult, w.shouldSkipDir(filepath.ToSlash(filepath.Clean(dir))), fmt.Sprintf("skipDirs: %s, dir: %s", tc.skipDirs, dir)) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for dir, want := range tt.wants { + dir = filepath.ToSlash(filepath.Clean(dir)) + got := walker.SkipPath(dir, walker.CleanSkipPaths(tt.skipDirs)) + assert.Equal(t, want, got, fmt.Sprintf("defaultSkipDirs: %s, dir: %s", tt.skipDirs, dir)) } }) } diff --git a/pkg/flag/report_flags.go b/pkg/flag/report_flags.go index c079e6bb256a..852710248972 100644 --- a/pkg/flag/report_flags.go +++ b/pkg/flag/report_flags.go @@ -261,7 +261,7 @@ func (f *ReportFlagGroup) ToOptions() (ReportOptions, error) { } func loadComplianceTypes(compliance string) (spec.ComplianceSpec, error) { - if len(compliance) > 0 && !slices.Contains(types.SupportedCompliances, compliance) && !strings.HasPrefix(compliance, "@") { + if compliance != "" && !slices.Contains(types.SupportedCompliances, compliance) && !strings.HasPrefix(compliance, "@") { return spec.ComplianceSpec{}, xerrors.Errorf("unknown compliance : %v", compliance) } diff --git a/pkg/iac/rules/register.go b/pkg/iac/rules/register.go index f34cdf904da3..0170a8d63979 100755 --- a/pkg/iac/rules/register.go +++ b/pkg/iac/rules/register.go @@ -125,7 +125,7 @@ func GetFrameworkRules(fw ...framework.Framework) []ruleTypes.RegisteredRule { } func GetSpecRules(spec string) []ruleTypes.RegisteredRule { - if len(spec) > 0 { + if spec != "" { return coreRegistry.getSpecRules(spec) } diff --git a/pkg/iac/scanners/azure/functions/first.go b/pkg/iac/scanners/azure/functions/first.go index 3415b453ffe3..91e2aece8e26 100644 --- a/pkg/iac/scanners/azure/functions/first.go +++ b/pkg/iac/scanners/azure/functions/first.go @@ -9,7 +9,7 @@ func First(args ...interface{}) interface{} { switch cType := container.(type) { case string: - if len(cType) > 0 { + if cType != "" { return string(cType[0]) } case interface{}: diff --git a/pkg/iac/scanners/azure/functions/last.go b/pkg/iac/scanners/azure/functions/last.go index 8466ec6b669f..84f54fb335fa 100644 --- a/pkg/iac/scanners/azure/functions/last.go +++ b/pkg/iac/scanners/azure/functions/last.go @@ -9,7 +9,7 @@ func Last(args ...interface{}) interface{} { switch cType := container.(type) { case string: - if len(cType) > 0 { + if cType != "" { return string(cType[len(cType)-1]) } case interface{}: diff --git a/pkg/k8s/commands/namespace.go b/pkg/k8s/commands/namespace.go index 6d828d4efdf9..b6c85e8dae00 100644 --- a/pkg/k8s/commands/namespace.go +++ b/pkg/k8s/commands/namespace.go @@ -36,7 +36,7 @@ func namespaceRun(ctx context.Context, opts flag.Options, cluster k8s.Cluster) e } func getNamespace(opts flag.Options, currentNamespace string) string { - if len(opts.K8sOptions.Namespace) > 0 { + if opts.K8sOptions.Namespace != "" { return opts.K8sOptions.Namespace } diff --git a/pkg/mapfs/fs.go b/pkg/mapfs/fs.go index 471730cc533e..c809f394b6fd 100644 --- a/pkg/mapfs/fs.go +++ b/pkg/mapfs/fs.go @@ -235,7 +235,7 @@ func (m *FS) RemoveAll(path string) error { func cleanPath(path string) string { // Convert the volume name like 'C:' into dir like 'C\' - if vol := filepath.VolumeName(path); len(vol) > 0 { + if vol := filepath.VolumeName(path); vol != "" { newVol := strings.TrimSuffix(vol, ":") newVol = fmt.Sprintf("%s%c", newVol, filepath.Separator) path = strings.Replace(path, vol, newVol, 1) diff --git a/pkg/parallel/walk.go b/pkg/parallel/walk.go index 2797bf98391a..a5e2523f8920 100644 --- a/pkg/parallel/walk.go +++ b/pkg/parallel/walk.go @@ -18,6 +18,9 @@ type onWalkResult[T any] func(T) error func WalkDir[T any](ctx context.Context, fsys fs.FS, root string, parallel int, onFile onFile[T], onResult onWalkResult[T]) error { + if parallel == 0 { + parallel = defaultParallel // Set the default value + } g, ctx := errgroup.WithContext(ctx) paths := make(chan string) @@ -55,9 +58,6 @@ func WalkDir[T any](ctx context.Context, fsys fs.FS, root string, parallel int, // Start a fixed number of goroutines to read and digest files. c := make(chan T) - if parallel == 0 { - parallel = defaultParallel - } for i := 0; i < parallel; i++ { g.Go(func() error { for path := range paths { diff --git a/pkg/report/table/vulnerability.go b/pkg/report/table/vulnerability.go index 9b85a1f4aab1..1984838d3709 100644 --- a/pkg/report/table/vulnerability.go +++ b/pkg/report/table/vulnerability.go @@ -124,7 +124,7 @@ func (r *vulnerabilityRenderer) setVulnerabilityRows(tw *table.Table, vulns []ty title = strings.Join(splitTitle[:12], " ") + "..." } - if len(v.PrimaryURL) > 0 { + if v.PrimaryURL != "" { if r.isTerminal { title = tml.Sprintf("%s\n%s", title, v.PrimaryURL) } else { diff --git a/pkg/sbom/cyclonedx/marshal.go b/pkg/sbom/cyclonedx/marshal.go index 9465b790bdd2..a8d96e4b12e3 100644 --- a/pkg/sbom/cyclonedx/marshal.go +++ b/pkg/sbom/cyclonedx/marshal.go @@ -298,7 +298,7 @@ func (*Marshaler) Properties(properties []core.Property) *[]cdx.Property { cdxProps := make([]cdx.Property, 0, len(properties)) for _, property := range properties { namespace := Namespace - if len(property.Namespace) > 0 { + if property.Namespace != "" { namespace = property.Namespace } diff --git a/pkg/scanner/scan.go b/pkg/scanner/scan.go index 4964ef1f2ad5..c5451167c709 100644 --- a/pkg/scanner/scan.go +++ b/pkg/scanner/scan.go @@ -49,13 +49,13 @@ var StandaloneArchiveSet = wire.NewSet( // StandaloneFilesystemSet binds filesystem dependencies var StandaloneFilesystemSet = wire.NewSet( - flocal.NewArtifact, + flocal.ArtifactSet, StandaloneSuperSet, ) // StandaloneRepositorySet binds repository dependencies var StandaloneRepositorySet = wire.NewSet( - repo.NewArtifact, + repo.ArtifactSet, StandaloneSuperSet, ) @@ -67,7 +67,7 @@ var StandaloneSBOMSet = wire.NewSet( // StandaloneVMSet binds vm dependencies var StandaloneVMSet = wire.NewSet( - vm.NewArtifact, + vm.ArtifactSet, StandaloneSuperSet, ) @@ -85,13 +85,13 @@ var RemoteSuperSet = wire.NewSet( // RemoteFilesystemSet binds filesystem dependencies for client/server mode var RemoteFilesystemSet = wire.NewSet( - flocal.NewArtifact, + flocal.ArtifactSet, RemoteSuperSet, ) // RemoteRepositorySet binds repository dependencies for client/server mode var RemoteRepositorySet = wire.NewSet( - repo.NewArtifact, + repo.ArtifactSet, RemoteSuperSet, ) @@ -103,7 +103,7 @@ var RemoteSBOMSet = wire.NewSet( // RemoteVMSet binds vm dependencies for client/server mode var RemoteVMSet = wire.NewSet( - vm.NewArtifact, + vm.ArtifactSet, RemoteSuperSet, )