From 7a8a2f7fc136299f300fa392bcd6e8ff1affce29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Cort=C3=AAs?= <593860+mrfyda@users.noreply.github.com> Date: Fri, 22 Mar 2024 06:52:43 -0700 Subject: [PATCH] fix: scan secrets only for changed files (#35) --- docs/multiple-tests/all-patterns/patterns.xml | 5 + docs/multiple-tests/all-patterns/results.xml | 10 + .../all-patterns/src/aws-config.txt | 4 + .../all-patterns/src/dart/pubspec.lock | 85 +++++++++ internal/tool/tool.go | 175 ++++++++++++------ internal/tool/tool_test.go | 21 ++- 6 files changed, 232 insertions(+), 68 deletions(-) create mode 100644 docs/multiple-tests/all-patterns/patterns.xml create mode 100644 docs/multiple-tests/all-patterns/results.xml create mode 100644 docs/multiple-tests/all-patterns/src/aws-config.txt create mode 100644 docs/multiple-tests/all-patterns/src/dart/pubspec.lock diff --git a/docs/multiple-tests/all-patterns/patterns.xml b/docs/multiple-tests/all-patterns/patterns.xml new file mode 100644 index 0000000..8db134c --- /dev/null +++ b/docs/multiple-tests/all-patterns/patterns.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/docs/multiple-tests/all-patterns/results.xml b/docs/multiple-tests/all-patterns/results.xml new file mode 100644 index 0000000..6d9d062 --- /dev/null +++ b/docs/multiple-tests/all-patterns/results.xml @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/docs/multiple-tests/all-patterns/src/aws-config.txt b/docs/multiple-tests/all-patterns/src/aws-config.txt new file mode 100644 index 0000000..5c255c7 --- /dev/null +++ b/docs/multiple-tests/all-patterns/src/aws-config.txt @@ -0,0 +1,4 @@ +'AWS_secret_KEY'="12ASD34qwe56CXZ78tyH10Tna543VBokN85RHCas" +AWS_ACCESS_KEY_ID=AKIA0123456789ABCDEF +"aws_account_ID":'1234-5678-9123' +AWS_example=AKIAIOSFODNN7EXAMPLE% diff --git a/docs/multiple-tests/all-patterns/src/dart/pubspec.lock b/docs/multiple-tests/all-patterns/src/dart/pubspec.lock new file mode 100644 index 0000000..c74de40 --- /dev/null +++ b/docs/multiple-tests/all-patterns/src/dart/pubspec.lock @@ -0,0 +1,85 @@ +# Generated by pub +# See https://dart.dev/tools/pub/glossary#lockfile +packages: + async: + dependency: transitive + description: + name: async + sha256: "947bfcf187f74dbc5e146c9eb9c0f10c9f8b30743e341481c1e2ed3ecc18c20c" + url: "https://pub.dev" + source: hosted + version: "2.11.0" + collection: + dependency: transitive + description: + name: collection + sha256: ee67cb0715911d28db6bf4af1026078bd6f0128b07a5f66fb2ed94ec6783c09a + url: "https://pub.dev" + source: hosted + version: "1.18.0" + dio: + dependency: "direct main" + description: + name: http + sha256: "5895291c13fa8a3bd82e76d5627f69e0d85ca6a30dcac95c4ea19a5d555879c2" + url: "https://pub.dev" + source: hosted + version: "4.0.0" + http_parser: + dependency: transitive + description: + name: http_parser + sha256: "2aa08ce0341cc9b354a498388e30986515406668dbcc4f7c950c3e715496693b" + url: "https://pub.dev" + source: hosted + version: "4.0.2" + meta: + dependency: transitive + description: + name: meta + sha256: d584fa6707a52763a52446f02cc621b077888fb63b93bbcb1143a7be5a0c0c04 + url: "https://pub.dev" + source: hosted + version: "1.11.0" + path: + dependency: transitive + description: + name: path + sha256: "8829d8a55c13fc0e37127c29fedf290c102f4e40ae94ada574091fe0ff96c917" + url: "https://pub.dev" + source: hosted + version: "1.8.3" + source_span: + dependency: transitive + description: + name: source_span + sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" + url: "https://pub.dev" + source: hosted + version: "1.10.0" + string_scanner: + dependency: transitive + description: + name: string_scanner + sha256: "556692adab6cfa87322a115640c11f13cb77b3f076ddcc5d6ae3c20242bedcde" + url: "https://pub.dev" + source: hosted + version: "1.2.0" + term_glyph: + dependency: transitive + description: + name: term_glyph + sha256: a29248a84fbb7c79282b40b8c72a1209db169a2e0542bce341da992fe1bc7e84 + url: "https://pub.dev" + source: hosted + version: "1.2.1" + typed_data: + dependency: transitive + description: + name: typed_data + sha256: facc8d6582f16042dd49f2463ff1bd6e2c9ef9f3d5da3d9b087e244a7b564b3c + url: "https://pub.dev" + source: hosted + version: "1.3.2" +sdks: + dart: ">=3.1.0 <4.0.0" diff --git a/internal/tool/tool.go b/internal/tool/tool.go index 6c586bb..324ec3b 100644 --- a/internal/tool/tool.go +++ b/internal/tool/tool.go @@ -1,12 +1,16 @@ package tool import ( + "bytes" "context" "fmt" + "os" + "path" + "github.com/aquasecurity/trivy/pkg/fanal/secret" "github.com/aquasecurity/trivy/pkg/flag" "github.com/aquasecurity/trivy/pkg/log" - "github.com/aquasecurity/trivy/pkg/types" + types "github.com/aquasecurity/trivy/pkg/types" codacy "github.com/codacy/codacy-engine-golang-seed/v6" "github.com/samber/lo" ) @@ -38,21 +42,63 @@ func (t codacyTrivy) Run(ctx context.Context, toolExecution codacy.ToolExecution return []codacy.Result{}, nil } - config, err := newConfiguration(*toolExecution.Patterns, toolExecution.SourceDir) + err := newConfiguration(*toolExecution.Patterns) if err != nil { return nil, err } - issues, err := t.run(ctx, *config) + vulnerabilityScanningIssues, err := t.runVulnerabilityScanning(ctx, toolExecution) if err != nil { return nil, err } - // Filter only valid results - return mapIssuesWithoutLineNumber(filterIssuesFromKnownFiles(issues, *toolExecution.Files)), nil + secretScanningIssues, err := t.runSecretScanning(*toolExecution.Patterns, toolExecution.Files, toolExecution.SourceDir) + if err != nil { + return nil, err + } + + allIssues := append(vulnerabilityScanningIssues, secretScanningIssues...) + + return allIssues, nil } -func (t codacyTrivy) run(ctx context.Context, config flag.Options) ([]codacy.Issue, error) { +func (t codacyTrivy) runVulnerabilityScanning(ctx context.Context, toolExecution codacy.ToolExecution) ([]codacy.Result, error) { + vulnerabilityScanningEnabled := lo.SomeBy(*toolExecution.Patterns, func(p codacy.Pattern) bool { + return p.ID == ruleIDVulnerability + }) + if !vulnerabilityScanningEnabled { + return []codacy.Result{}, nil + } + + config := flag.Options{ + GlobalOptions: flag.GlobalOptions{ + // CacheDir needs to be explicitly set and match the directory in the Dockerfile. + // The cache dir will contain the pre-downloaded vulnerability DBs. + CacheDir: cacheDir, + }, + DBOptions: flag.DBOptions{ + // Do not try to update vulnerability DBs. + SkipDBUpdate: true, + SkipJavaDBUpdate: true, + }, + ReportOptions: flag.ReportOptions{ + // Listing all packages will allow to obtain the line number of a vulnerability. + ListAllPkgs: true, + }, + ScanOptions: flag.ScanOptions{ + // Do not try to connect to the internet to download vulnerability DBs, for example. + OfflineScan: true, + Scanners: types.Scanners{types.VulnerabilityScanner}, + // Instead of scanning files individually, scan the whole source directory since it's faster. + // Then filter issues from files that were not supposed to be analysed. + Target: toolExecution.SourceDir, + }, + VulnerabilityOptions: flag.VulnerabilityOptions{ + // Only scan libraries not OS packages. + VulnType: []types.VulnType{types.VulnTypeLibrary}, + }, + } + runner, err := t.runnerFactory.NewRunner(ctx, config) if err != nil { return nil, err @@ -76,7 +122,6 @@ func (t codacyTrivy) run(ctx context.Context, config flag.Options) ([]codacy.Iss lineNumberByPackageId[pkgID(pkg.ID, pkg.Name, pkg.Version)] = lineNumber } - // Vulnerability scanning results for _, vuln := range result.Vulnerabilities { ID := pkgID(vuln.PkgID, vuln.PkgName, vuln.InstalledVersion) issues = append( @@ -90,69 +135,29 @@ func (t codacyTrivy) run(ctx context.Context, config flag.Options) ([]codacy.Iss ) } - // Secret scanning results - for _, secret := range result.Secrets { - issues = append( - issues, - codacy.Issue{ - File: result.Target, - Line: secret.StartLine, - Message: fmt.Sprintf("Possible hardcoded secret: %s", secret.Title), - PatternID: ruleIDSecret, - }, - ) - } } - return issues, nil + + return mapIssuesWithoutLineNumber(filterIssuesFromKnownFiles(issues, *toolExecution.Files)), nil } -func newConfiguration(patterns []codacy.Pattern, sourceDir string) (*flag.Options, error) { - scanners := types.Scanners{} - for _, pattern := range patterns { - switch pattern.ID { - case ruleIDSecret: - scanners = append(scanners, types.SecretScanner) - case ruleIDVulnerability: - scanners = append(scanners, types.VulnerabilityScanner) - } +func newConfiguration(patterns []codacy.Pattern) error { + if len(patterns) == 0 { + return &ToolError{msg: "Failed to configure Codacy Trivy: no patterns configured"} } - if len(scanners) == 0 { - return nil, &ToolError{msg: "Failed to configure Codacy Trivy: provided patterns don't match existing rules"} + noSupportedPatterns := lo.NoneBy(patterns, func(p codacy.Pattern) bool { + return p.ID == ruleIDSecret || p.ID == ruleIDVulnerability + }) + + if noSupportedPatterns { + return &ToolError{msg: "Failed to configure Codacy Trivy: provided patterns don't match existing rules"} } // The `quiet` field in global options is not used by the runner. // This is the only way to suppress Trivy logs. log.InitLogger(false, true) - return &flag.Options{ - GlobalOptions: flag.GlobalOptions{ - // CacheDir needs to be explicitly set and match the directory in the Dockerfile. - // The cache dir will contain the pre-downloaded vulnerability DBs. - CacheDir: cacheDir, - }, - DBOptions: flag.DBOptions{ - // Do not try to update vulnerability DBs. - SkipDBUpdate: true, - SkipJavaDBUpdate: true, - }, - ReportOptions: flag.ReportOptions{ - // Listing all packages will allow to obtain the line number of a vulnerability. - ListAllPkgs: true, - }, - ScanOptions: flag.ScanOptions{ - // Do not try to connect to the internet to download vulnerability DBs, for example. - OfflineScan: true, - Scanners: scanners, - // Instead of scanning files individually, scan the whole source directory since it's faster. - // Then filter issues from files that were not supposed to be analysed. - Target: sourceDir, - }, - VulnerabilityOptions: flag.VulnerabilityOptions{ - // Only scan libraries not OS packages. - VulnType: []types.VulnType{types.VulnTypeLibrary}, - }, - }, nil + return nil } // Results without a line number (0 is the empty value) can't be displayed by Codacy and are mapped to a `codacy.FileError`. @@ -192,3 +197,55 @@ func pkgID(ID, name, version string) string { } return fmt.Sprintf("%s@%s", name, version) } + +// Running Trivy for secret scanning is not as efficient as running for vulnerability scanning. +// It's much more efficient to run the two scan separately, even though that results in more wrapper code. +func (t codacyTrivy) runSecretScanning(patterns []codacy.Pattern, files *[]string, sourceDir string) ([]codacy.Result, error) { + secretDetectionEnabled := lo.SomeBy(patterns, func(p codacy.Pattern) bool { + return p.ID == ruleIDSecret + }) + if !secretDetectionEnabled { + return []codacy.Result{}, nil + } + + if files == nil || len(*files) == 0 { + // TODO Run for all files in the source dir? + return []codacy.Result{}, nil + } + + scanner := secret.NewScanner(&secret.Config{}) + + results := []codacy.Result{} + + for _, f := range *files { + + filePath := path.Join(sourceDir, f) + content, err := os.ReadFile(filePath) + + if err != nil { + results = append( + results, + codacy.FileError{ + File: f, + Message: "Failed to read source file", + }, + ) + } + content = bytes.ReplaceAll(content, []byte("\r"), []byte("")) + + secrets := scanner.Scan(secret.ScanArgs{FilePath: filePath, Content: content}) + + for _, result := range secrets.Findings { + results = append( + results, + codacy.Issue{ + File: f, + Message: fmt.Sprintf("Possible hardcoded secret: %s", result.Title), + PatternID: ruleIDSecret, + Line: result.StartLine, + }, + ) + } + } + return results, nil +} diff --git a/internal/tool/tool_test.go b/internal/tool/tool_test.go index 7e4fff6..77d2b16 100644 --- a/internal/tool/tool_test.go +++ b/internal/tool/tool_test.go @@ -65,7 +65,7 @@ func TestRun(t *testing.T) { }, ScanOptions: flag.ScanOptions{ OfflineScan: true, - Scanners: types.Scanners{types.SecretScanner, types.VulnerabilityScanner}, + Scanners: types.Scanners{types.VulnerabilityScanner}, Target: sourceDir, }, VulnerabilityOptions: flag.VulnerabilityOptions{ @@ -174,12 +174,6 @@ func TestRun(t *testing.T) { PatternID: ruleIDVulnerability, Message: "Insecure dependency package-1 (vuln id: vuln title) (update to vuln fixed)", }, - codacy.Issue{ - File: file2, - Line: 2, - PatternID: ruleIDSecret, - Message: "Possible hardcoded secret: secret title", - }, codacy.FileError{ File: file1, Message: "Line numbers not supported", @@ -188,6 +182,14 @@ func TestRun(t *testing.T) { File: file2, Message: "Line numbers not supported", }, + codacy.FileError{ + File: file1, + Message: "Failed to read source file", + }, + codacy.FileError{ + File: file2, + Message: "Failed to read source file", + }, } assert.ElementsMatch(t, expectedResults, results) } @@ -234,9 +236,10 @@ func TestRunNewRunnerError(t *testing.T) { toolExecution := codacy.ToolExecution{ Patterns: &[]codacy.Pattern{ { - ID: ruleIDSecret, + ID: ruleIDVulnerability, }, }, + Files: &[]string{}, } underTest := codacyTrivy{ @@ -284,7 +287,7 @@ func TestRunScanFilesystemError(t *testing.T) { }, ScanOptions: flag.ScanOptions{ OfflineScan: true, - Scanners: types.Scanners{types.SecretScanner, types.VulnerabilityScanner}, + Scanners: types.Scanners{types.VulnerabilityScanner}, Target: sourceDir, }, VulnerabilityOptions: flag.VulnerabilityOptions{