From c0e8da3828e9d3a0b30d1f6568037db8dc827765 Mon Sep 17 00:00:00 2001 From: simar7 <1254783+simar7@users.noreply.github.com> Date: Tue, 1 Oct 2024 23:20:03 -0600 Subject: [PATCH] feat(misconf): Support `--skip-*` for all included modules (#7579) Signed-off-by: nikpivkin Co-authored-by: nikpivkin --- pkg/commands/artifact/run.go | 2 + pkg/fanal/utils/utils.go | 27 ++++++ pkg/fanal/walker/fs.go | 7 +- pkg/fanal/walker/tar.go | 8 +- pkg/fanal/walker/vm.go | 5 +- pkg/fanal/walker/walk.go | 29 ------ pkg/fanal/walker/walk_test.go | 6 +- pkg/iac/scanners/terraform/options.go | 16 ++++ pkg/iac/scanners/terraform/parser/option.go | 12 +++ pkg/iac/scanners/terraform/parser/parser.go | 7 ++ pkg/iac/scanners/terraform/scanner_test.go | 99 +++++++++++++++++++++ pkg/misconf/scanner.go | 4 + 12 files changed, 181 insertions(+), 41 deletions(-) diff --git a/pkg/commands/artifact/run.go b/pkg/commands/artifact/run.go index 11c6ef6a3090..6f926519b91a 100644 --- a/pkg/commands/artifact/run.go +++ b/pkg/commands/artifact/run.go @@ -659,5 +659,7 @@ func initMisconfScannerOption(opts flag.Options) (misconf.ScannerOption, error) TfExcludeDownloaded: opts.TfExcludeDownloaded, FilePatterns: opts.FilePatterns, ConfigFileSchemas: configSchemas, + SkipFiles: opts.SkipFiles, + SkipDirs: opts.SkipDirs, }, nil } diff --git a/pkg/fanal/utils/utils.go b/pkg/fanal/utils/utils.go index 58b44510db89..463c8dd1f255 100644 --- a/pkg/fanal/utils/utils.go +++ b/pkg/fanal/utils/utils.go @@ -9,10 +9,14 @@ import ( "os" "os/exec" "path/filepath" + "strings" "unicode" + "github.com/bmatcuk/doublestar/v4" + "github.com/samber/lo" "golang.org/x/xerrors" + "github.com/aquasecurity/trivy/pkg/log" xio "github.com/aquasecurity/trivy/pkg/x/io" ) @@ -98,6 +102,29 @@ func IsBinary(content xio.ReadSeekerAt, fileSize int64) (bool, error) { return false, nil } +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 SkipPath(path string, skipPaths []string) bool { + path = strings.TrimLeft(path, "/") + + // skip files + 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 path", log.String("path", path)) + return true + } + } + return false +} + func ExtractPrintableBytes(content xio.ReadSeekerAt) ([]byte, error) { const minLength = 4 // Minimum length of strings to extract var result []byte diff --git a/pkg/fanal/walker/fs.go b/pkg/fanal/walker/fs.go index f6bec84d1fc2..3ebfb7258459 100644 --- a/pkg/fanal/walker/fs.go +++ b/pkg/fanal/walker/fs.go @@ -9,6 +9,7 @@ import ( "golang.org/x/xerrors" + "github.com/aquasecurity/trivy/pkg/fanal/utils" "github.com/aquasecurity/trivy/pkg/log" xio "github.com/aquasecurity/trivy/pkg/x/io" ) @@ -53,13 +54,13 @@ func (w *FS) WalkDirFunc(root string, fn WalkFunc, opt Option) fs.WalkDirFunc { // Skip unnecessary files switch { case d.IsDir(): - if SkipPath(relPath, opt.SkipDirs) { + if utils.SkipPath(relPath, opt.SkipDirs) { return filepath.SkipDir } return nil case !d.Type().IsRegular(): return nil - case SkipPath(relPath, opt.SkipFiles): + case utils.SkipPath(relPath, opt.SkipFiles): return nil } @@ -148,7 +149,7 @@ func (w *FS) BuildSkipPaths(base string, paths []string) []string { relativePaths = append(relativePaths, relPath) } - relativePaths = CleanSkipPaths(relativePaths) + relativePaths = utils.CleanSkipPaths(relativePaths) return relativePaths } diff --git a/pkg/fanal/walker/tar.go b/pkg/fanal/walker/tar.go index 51f185d89c50..0c7d0bee9d5d 100644 --- a/pkg/fanal/walker/tar.go +++ b/pkg/fanal/walker/tar.go @@ -27,8 +27,8 @@ type LayerTar struct { func NewLayerTar(opt Option) LayerTar { return LayerTar{ - skipFiles: CleanSkipPaths(opt.SkipFiles), - skipDirs: CleanSkipPaths(opt.SkipDirs), + skipFiles: utils.CleanSkipPaths(opt.SkipFiles), + skipDirs: utils.CleanSkipPaths(opt.SkipDirs), } } @@ -63,12 +63,12 @@ func (w LayerTar) Walk(layer io.Reader, analyzeFn WalkFunc) ([]string, []string, switch hdr.Typeflag { case tar.TypeDir: - if SkipPath(filePath, w.skipDirs) { + if utils.SkipPath(filePath, w.skipDirs) { skippedDirs = append(skippedDirs, filePath) continue } case tar.TypeReg: - if SkipPath(filePath, w.skipFiles) { + if utils.SkipPath(filePath, w.skipFiles) { continue } // symlinks and hardlinks have no content in reader, skip them diff --git a/pkg/fanal/walker/vm.go b/pkg/fanal/walker/vm.go index 0bc3e87ac86d..f8ef63ce0252 100644 --- a/pkg/fanal/walker/vm.go +++ b/pkg/fanal/walker/vm.go @@ -17,6 +17,7 @@ import ( "github.com/masahiro331/go-xfs-filesystem/xfs" "golang.org/x/xerrors" + "github.com/aquasecurity/trivy/pkg/fanal/utils" "github.com/aquasecurity/trivy/pkg/fanal/vm/filesystem" "github.com/aquasecurity/trivy/pkg/log" xio "github.com/aquasecurity/trivy/pkg/x/io" @@ -131,13 +132,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 SkipPath(pathName, w.skipDirs) { + if utils.SkipPath(pathName, w.skipDirs) { return filepath.SkipDir } return nil case !fi.Mode().IsRegular(): return nil - case SkipPath(pathName, w.skipFiles): + case utils.SkipPath(pathName, w.skipFiles): return nil case fi.Mode()&0x1000 == 0x1000 || fi.Mode()&0x2000 == 0x2000 || diff --git a/pkg/fanal/walker/walk.go b/pkg/fanal/walker/walk.go index 52c13808d86b..0785cef41606 100644 --- a/pkg/fanal/walker/walk.go +++ b/pkg/fanal/walker/walk.go @@ -2,14 +2,8 @@ package walker import ( "os" - "path/filepath" - "strings" - - "github.com/bmatcuk/doublestar/v4" - "github.com/samber/lo" "github.com/aquasecurity/trivy/pkg/fanal/analyzer" - "github.com/aquasecurity/trivy/pkg/log" ) const defaultSizeThreshold = int64(100) << 20 // 200MB @@ -27,26 +21,3 @@ type Option struct { } type WalkFunc func(filePath string, info os.FileInfo, opener analyzer.Opener) error - -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 SkipPath(path string, skipPaths []string) bool { - path = strings.TrimLeft(path, "/") - - // skip files - 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 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 0bac6009d04f..09d55527c0fe 100644 --- a/pkg/fanal/walker/walk_test.go +++ b/pkg/fanal/walker/walk_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/aquasecurity/trivy/pkg/fanal/walker" + "github.com/aquasecurity/trivy/pkg/fanal/utils" ) func TestSkipFile(t *testing.T) { @@ -64,7 +64,7 @@ func TestSkipFile(t *testing.T) { 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)) + got := utils.SkipPath(file, utils.CleanSkipPaths(tt.skipFiles)) assert.Equal(t, want, got, fmt.Sprintf("skipFiles: %s, file: %s", tt.skipFiles, file)) } }) @@ -138,7 +138,7 @@ func TestSkipDir(t *testing.T) { 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)) + got := utils.SkipPath(dir, utils.CleanSkipPaths(tt.skipDirs)) assert.Equal(t, want, got, fmt.Sprintf("defaultSkipDirs: %s, dir: %s", tt.skipDirs, dir)) } }) diff --git a/pkg/iac/scanners/terraform/options.go b/pkg/iac/scanners/terraform/options.go index d256f0a34daa..5a665a05638f 100644 --- a/pkg/iac/scanners/terraform/options.go +++ b/pkg/iac/scanners/terraform/options.go @@ -84,3 +84,19 @@ func ScannerWithConfigsFileSystem(fsys fs.FS) options.ScannerOption { } } } + +func ScannerWithSkipFiles(files []string) options.ScannerOption { + return func(s options.ConfigurableScanner) { + if tf, ok := s.(ConfigurableTerraformScanner); ok { + tf.AddParserOptions(parser.OptionWithSkipFiles(files)) + } + } +} + +func ScannerWithSkipDirs(dirs []string) options.ScannerOption { + return func(s options.ConfigurableScanner) { + if tf, ok := s.(ConfigurableTerraformScanner); ok { + tf.AddParserOptions(parser.OptionWithSkipDirs(dirs)) + } + } +} diff --git a/pkg/iac/scanners/terraform/parser/option.go b/pkg/iac/scanners/terraform/parser/option.go index 277cf2a58c61..f9ada6545225 100644 --- a/pkg/iac/scanners/terraform/parser/option.go +++ b/pkg/iac/scanners/terraform/parser/option.go @@ -49,3 +49,15 @@ func OptionWithConfigsFS(fsys fs.FS) Option { p.configsFS = fsys } } + +func OptionWithSkipFiles(files []string) Option { + return func(p *Parser) { + p.skipPaths = files + } +} + +func OptionWithSkipDirs(dirs []string) Option { + return func(p *Parser) { + p.skipPaths = dirs + } +} diff --git a/pkg/iac/scanners/terraform/parser/parser.go b/pkg/iac/scanners/terraform/parser/parser.go index 60940d6c6241..695cbee4fb25 100644 --- a/pkg/iac/scanners/terraform/parser/parser.go +++ b/pkg/iac/scanners/terraform/parser/parser.go @@ -15,6 +15,7 @@ import ( "github.com/hashicorp/hcl/v2/hclparse" "github.com/zclconf/go-cty/cty" + "github.com/aquasecurity/trivy/pkg/fanal/utils" "github.com/aquasecurity/trivy/pkg/iac/ignore" "github.com/aquasecurity/trivy/pkg/iac/terraform" tfcontext "github.com/aquasecurity/trivy/pkg/iac/terraform/context" @@ -47,6 +48,7 @@ type Parser struct { skipCachedModules bool fsMap map[string]fs.FS configsFS fs.FS + skipPaths []string } // New creates a new Parser @@ -78,6 +80,7 @@ func (p *Parser) newModuleParser(moduleFS fs.FS, moduleSource, modulePath, modul mp.moduleName = moduleName mp.logger = log.WithPrefix("terraform parser").With("module", moduleName) mp.projectRoot = p.projectRoot + mp.skipPaths = p.skipPaths p.children = append(p.children, mp) for _, option := range p.options { option(mp) @@ -155,6 +158,10 @@ func (p *Parser) ParseFS(ctx context.Context, dir string) error { if info.IsDir() { continue } + if utils.SkipPath(realPath, utils.CleanSkipPaths(p.skipPaths)) { + p.logger.Debug("Skipping path based on input glob", log.FilePath(realPath), log.Any("glob", p.skipPaths)) + continue + } paths = append(paths, realPath) } sort.Strings(paths) diff --git a/pkg/iac/scanners/terraform/scanner_test.go b/pkg/iac/scanners/terraform/scanner_test.go index 39ee35e81321..a7020b4ddd06 100644 --- a/pkg/iac/scanners/terraform/scanner_test.go +++ b/pkg/iac/scanners/terraform/scanner_test.go @@ -1118,3 +1118,102 @@ func TestSkipDeprecatedGoChecks(t *testing.T) { require.Len(t, results, 1) }) } + +func TestSkipDir(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "deployments/main.tf": ` +module "use_bad_configuration" { + source = "../modules" +} + +module "use_bad_configuration_2" { + source = "../modules/modules2" +} +`, + "modules/misconfig.tf": `data "aws_iam_policy_document" "bad" { + statement { + actions = [ + "apigateway:*", + ] + + resources = [ + "*", + ] + } +} + +resource "aws_iam_policy" "bad_configuration" { + name_prefix = local.setup_role_name + policy = data.aws_iam_policy_document.bad.json +} +`, + "modules/modules2/misconfig.tf": `data "aws_iam_policy_document" "bad" { + statement { + actions = [ + "apigateway:*", + ] + + resources = [ + "*", + ] + } +} + +resource "aws_iam_policy" "bad_configuration" { + name_prefix = local.setup_role_name + policy = data.aws_iam_policy_document.bad.json +} +`, + }) + + t.Run("use skip-dir option", func(t *testing.T) { + scanner := New( + options.ScannerWithIncludeDeprecatedChecks(true), + ScannerWithSkipDirs([]string{"**/modules/**"}), + ScannerWithAllDirectories(true), + ) + + results, err := scanner.ScanFS(context.TODO(), fs, "deployments") + require.NoError(t, err) + + assert.Empty(t, results) + }) + + t.Run("use skip-files option", func(t *testing.T) { + scanner := New( + options.ScannerWithIncludeDeprecatedChecks(true), + ScannerWithSkipFiles([]string{"**/modules/**/*.tf"}), + ScannerWithAllDirectories(true), + ) + + results, err := scanner.ScanFS(context.TODO(), fs, "deployments") + require.NoError(t, err) + + assert.Empty(t, results) + }) + + t.Run("non existing value for skip-files option", func(t *testing.T) { + scanner := New( + options.ScannerWithIncludeDeprecatedChecks(true), + ScannerWithSkipFiles([]string{"foo/bar*.tf"}), + ScannerWithAllDirectories(true), + ) + + results, err := scanner.ScanFS(context.TODO(), fs, "deployments") + require.NoError(t, err) + + assert.Len(t, results, 4) + }) + + t.Run("empty skip-files option", func(t *testing.T) { + scanner := New( + options.ScannerWithIncludeDeprecatedChecks(true), + ScannerWithAllDirectories(true), + ) + + results, err := scanner.ScanFS(context.TODO(), fs, "deployments") + require.NoError(t, err) + + assert.Len(t, results, 4) + }) +} diff --git a/pkg/misconf/scanner.go b/pkg/misconf/scanner.go index 78f4f4bdb158..7bec3ed6fe14 100644 --- a/pkg/misconf/scanner.go +++ b/pkg/misconf/scanner.go @@ -76,6 +76,8 @@ type ScannerOption struct { ConfigFileSchemas []*ConfigFileSchema DisabledCheckIDs []string + SkipFiles []string + SkipDirs []string } func (o *ScannerOption) Sort() { @@ -297,6 +299,8 @@ func addTFOpts(opts []options.ScannerOption, scannerOption ScannerOption) ([]opt opts = append(opts, terraform.ScannerWithAllDirectories(true), terraform.ScannerWithSkipDownloaded(scannerOption.TfExcludeDownloaded), + terraform.ScannerWithSkipFiles(scannerOption.SkipFiles), + terraform.ScannerWithSkipDirs(scannerOption.SkipDirs), ) return opts, nil