From be2cbfc162b271a47e2ecf22637e79535208fb5b Mon Sep 17 00:00:00 2001 From: Trevor Edris <121651828+t-edris@users.noreply.github.com> Date: Mon, 18 Dec 2023 16:44:07 -0500 Subject: [PATCH] [13501] Continue security scanning but highlight missing images (#524) * [13501] Continue scanning but highlight missing images * Add changelog * Test update for expected error * Remove extra addition to markdown * Re-order imports * Only add debug instructions once * Changelog to v0.24.9 * Fix issue link in changelog * Rename param to something more generic * Update comment * Add missing image to test suite * Fix Apply in predicate; add additional test cases * Add ability to output would-be github issues to local files * s/ioutil/os for ReadFile * Extract to fileutils * Revert exported function signature; unexport error * Recoverable and Unrecoverable errors * Move directory setup higher in call chain * Callout scanning pre-releases in Changelog * Refactor to IssueWriter interface * Reformat imports * Empty; trigger tests * Clarifying GetPrerelease in predicate * Update predicate_test * Add comments; update interface assertion var name * Unfocus test * Add unit tests for Unrecoverable vs Recoverable errors * Update comment; remove unused struct field * Remove param from function call * resolvesIssue: false in changelog * Update changelog description * Add DescribeTable entry back * More accurate log line --- .../13501-report-missing-images-in-scan.yaml | 15 +++ fileutils/messages.go | 6 +- fileutils/text.go | 11 +++ securityscanutils/commands/format_results.go | 5 +- securityscanutils/commands/scan_repo.go | 13 ++- securityscanutils/commands/utils.go | 4 +- .../{ => issuewriter}/github_writer.go | 17 ++-- securityscanutils/issuewriter/issue_writer.go | 15 +++ securityscanutils/issuewriter/local_writer.go | 47 +++++++++ securityscanutils/issuewriter/noop_writer.go | 21 ++++ securityscanutils/predicate.go | 3 + securityscanutils/predicate_test.go | 14 ++- securityscanutils/securityscan.go | 62 +++++++++--- securityscanutils/securityscan_test.go | 98 ++++++++++++++++++- securityscanutils/trivy_scanner.go | 21 ++-- securityscanutils/trivy_scanner_test.go | 2 +- 16 files changed, 309 insertions(+), 45 deletions(-) create mode 100644 changelog/v0.25.0/13501-report-missing-images-in-scan.yaml create mode 100644 fileutils/text.go rename securityscanutils/{ => issuewriter}/github_writer.go (93%) create mode 100644 securityscanutils/issuewriter/issue_writer.go create mode 100644 securityscanutils/issuewriter/local_writer.go create mode 100644 securityscanutils/issuewriter/noop_writer.go diff --git a/changelog/v0.25.0/13501-report-missing-images-in-scan.yaml b/changelog/v0.25.0/13501-report-missing-images-in-scan.yaml new file mode 100644 index 00000000..0286fc21 --- /dev/null +++ b/changelog/v0.25.0/13501-report-missing-images-in-scan.yaml @@ -0,0 +1,15 @@ +changelog: + - type: NEW_FEATURE + issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13499 + description: > + Scanner includes an optional Additional Context section on reported vulnerabilities. + resolvesIssue: false + - type: BREAKING_CHANGE + issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13499 + description: > + Create `IssueWriter` interface with implementations as `NoopWriter`, `GithubIssueWriter`, and `LocalIssueWriter`. + resolvesIssue: false + - type: FIX + issueLink: https://github.com/solo-io/gloo-mesh-enterprise/issues/13501 + description: > + Scanner continues to scan remaining images if one image cannot be found. diff --git a/fileutils/messages.go b/fileutils/messages.go index c34835b5..e0b0b6ad 100644 --- a/fileutils/messages.go +++ b/fileutils/messages.go @@ -1,7 +1,7 @@ package fileutils import ( - "io/ioutil" + "os" "github.com/ghodss/yaml" "github.com/gogo/protobuf/proto" @@ -18,11 +18,11 @@ func WriteToFile(filename string, pb proto.Message) error { if err != nil { return err } - return ioutil.WriteFile(filename, data, 0644) + return os.WriteFile(filename, data, 0644) } func ReadFileInto(filename string, v proto.Message) error { - data, err := ioutil.ReadFile(filename) + data, err := os.ReadFile(filename) if err != nil { return eris.Errorf("error reading file: %v", err) } diff --git a/fileutils/text.go b/fileutils/text.go new file mode 100644 index 00000000..e18b86e4 --- /dev/null +++ b/fileutils/text.go @@ -0,0 +1,11 @@ +package fileutils + +import "os" + +func ReadFileString(filename string) (string, error) { + contents, err := os.ReadFile(filename) + if err != nil { + return "", err + } + return string(contents), nil +} diff --git a/securityscanutils/commands/format_results.go b/securityscanutils/commands/format_results.go index 958620b5..60ca5218 100644 --- a/securityscanutils/commands/format_results.go +++ b/securityscanutils/commands/format_results.go @@ -8,6 +8,7 @@ import ( "io/ioutil" "log" "net/http" + "os" "strings" "github.com/Masterminds/semver/v3" @@ -114,7 +115,7 @@ func doFormatResults(ctx context.Context, opts *formatResultsOptions) error { } func getCachedReleases(fileName string) []*github.RepositoryRelease { - bArray, err := ioutil.ReadFile(fileName) + bArray, err := os.ReadFile(fileName) if err != nil { return nil } @@ -225,7 +226,7 @@ func readImageVersionConstraintsFile(opts *formatResultsOptions) (map[string][]s imagesPerVersion := make(map[string][]string) imageSet := make(map[string]interface{}) - dat, err := ioutil.ReadFile(opts.imageFile) + dat, err := os.ReadFile(opts.imageFile) if err != nil { return nil, err } diff --git a/securityscanutils/commands/scan_repo.go b/securityscanutils/commands/scan_repo.go index 818c1082..d606155d 100644 --- a/securityscanutils/commands/scan_repo.go +++ b/securityscanutils/commands/scan_repo.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "github.com/solo-io/go-utils/fileutils" + "github.com/Masterminds/semver/v3" "github.com/solo-io/go-utils/cliutils" "github.com/solo-io/go-utils/securityscanutils" @@ -41,20 +43,23 @@ type scanRepoOptions struct { // none (default): do nothing when a vulnerability is discovered // github-issue-latest (preferred): create a github issue only for the latest patch version of each minor version, when a vulnerability is discovered // github-issue-all: create a github issue for every version where a vulnerability is discovered + // output-locally: create a file in the generated output dir containing the final Markdown for each repo / version vulnerabilityAction string releaseVersionConstraint string imagesVersionConstraintFile string + additionalContextFile string } func (m *scanRepoOptions) addToFlags(flags *pflag.FlagSet) { flags.StringVarP(&m.githubRepository, "github-repo", "g", "", "github repository to scan") flags.StringVarP(&m.imageRepository, "image-repo", "r", securityscanutils.QuayRepository, "image repository to scan") - flags.StringVarP(&m.vulnerabilityAction, "vulnerability-action", "a", "none", "action to take when a vulnerability is discovered {none, github-issue-all, github-issue-latest}") + flags.StringVarP(&m.vulnerabilityAction, "vulnerability-action", "a", "none", "action to take when a vulnerability is discovered {none, github-issue-all, github-issue-latest, output-locally}") flags.StringVarP(&m.releaseVersionConstraint, "release-constraint", "c", "", "version constraint for releases to scan") flags.StringVarP(&m.imagesVersionConstraintFile, "image-constraint-file", "i", "", "name of file with mapping of version to images") + flags.StringVarP(&m.additionalContextFile, "additional-context-file", "d", "", "name of file with any additional context to add to the top of the generated vulnerability report") cliutils.MustMarkFlagRequired(flags, "github-repo") cliutils.MustMarkFlagRequired(flags, "release-constraint") @@ -70,6 +75,10 @@ func doScanRepo(ctx context.Context, opts *scanRepoOptions) error { if err != nil { return err } + additionalContext, err := fileutils.ReadFileString(opts.additionalContextFile) + if err != nil { + return err + } scanner := &securityscanutils.SecurityScanner{ Repos: []*securityscanutils.SecurityScanRepo{ @@ -81,8 +90,10 @@ func doScanRepo(ctx context.Context, opts *scanRepoOptions) error { ImagesPerVersion: imagesPerVersion, VersionConstraint: releaseVersionConstraint, ImageRepo: opts.imageRepository, + OutputResultLocally: opts.vulnerabilityAction == "output-locally", CreateGithubIssuePerVersion: opts.vulnerabilityAction == "github-issue-all", CreateGithubIssueForLatestPatchVersion: opts.vulnerabilityAction == "github-issue-latest", + AdditionalContext: additionalContext, }, }, }, diff --git a/securityscanutils/commands/utils.go b/securityscanutils/commands/utils.go index dc965499..3eda3aeb 100644 --- a/securityscanutils/commands/utils.go +++ b/securityscanutils/commands/utils.go @@ -1,7 +1,7 @@ package commands import ( - "io/ioutil" + "os" "strings" "github.com/rotisserie/eris" @@ -20,7 +20,7 @@ func GetImagesPerVersionFromFile(constraintsFile string) (map[string][]string, e imagesPerVersion := make(map[string][]string) imageSet := make(map[string]interface{}) - dat, err := ioutil.ReadFile(constraintsFile) + dat, err := os.ReadFile(constraintsFile) if err != nil { return nil, err } diff --git a/securityscanutils/github_writer.go b/securityscanutils/issuewriter/github_writer.go similarity index 93% rename from securityscanutils/github_writer.go rename to securityscanutils/issuewriter/github_writer.go index 525f62a6..639885a1 100644 --- a/securityscanutils/github_writer.go +++ b/securityscanutils/issuewriter/github_writer.go @@ -1,14 +1,13 @@ -package securityscanutils +package issuewriter import ( "context" "fmt" - "github.com/solo-io/go-utils/contextutils" - "github.com/Masterminds/semver/v3" "github.com/google/go-github/v32/github" "github.com/rotisserie/eris" + "github.com/solo-io/go-utils/contextutils" "github.com/solo-io/go-utils/githubutils" ) @@ -41,7 +40,9 @@ type GithubIssueWriter struct { allGithubIssues []*github.Issue } -func NewGithubIssueWriter(repo GithubRepo, client *github.Client, issuePredicate githubutils.RepositoryReleasePredicate) *GithubIssueWriter { +var _ IssueWriter = &GithubIssueWriter{} + +func NewGithubIssueWriter(repo GithubRepo, client *github.Client, issuePredicate githubutils.RepositoryReleasePredicate) IssueWriter { return &GithubIssueWriter{ repo: repo, client: client, @@ -71,10 +72,14 @@ func (g *GithubIssueWriter) getAllGithubIssues(ctx context.Context) ([]*github.I return g.allGithubIssues, nil } -// Creates/Updates a Github Issue per image +// Creates/Updates a Github Issue per release // The github issue will have the markdown table report of the image's vulnerabilities // example: https://github.com/solo-io/solo-projects/issues/2458 -func (g *GithubIssueWriter) CreateUpdateVulnerabilityIssue(ctx context.Context, release *github.RepositoryRelease, vulnerabilityMarkdown string) error { +func (g *GithubIssueWriter) Write( + ctx context.Context, + release *github.RepositoryRelease, + vulnerabilityMarkdown string, +) error { logger := contextutils.LoggerFrom(ctx) if vulnerabilityMarkdown == "" { diff --git a/securityscanutils/issuewriter/issue_writer.go b/securityscanutils/issuewriter/issue_writer.go new file mode 100644 index 00000000..7c10e03f --- /dev/null +++ b/securityscanutils/issuewriter/issue_writer.go @@ -0,0 +1,15 @@ +package issuewriter + +import ( + "context" + + "github.com/google/go-github/v32/github" +) + +// IssueWriter writes the generated contents of a scan to a location, either a file on the local filesystem +// or a GitHub issue. +type IssueWriter interface { + // Write writes `contents`, the results of a scan of the images in `release`, to a location + // designated by the implementation. + Write(ctx context.Context, release *github.RepositoryRelease, contents string) error +} diff --git a/securityscanutils/issuewriter/local_writer.go b/securityscanutils/issuewriter/local_writer.go new file mode 100644 index 00000000..32a29a73 --- /dev/null +++ b/securityscanutils/issuewriter/local_writer.go @@ -0,0 +1,47 @@ +package issuewriter + +import ( + "context" + "fmt" + "os" + "path" + + "github.com/Masterminds/semver/v3" + "github.com/google/go-github/v32/github" +) + +// LocalIssueWriter writes the scan results to a file on the local file system. +type LocalIssueWriter struct { + // The directory in which to create files + outputDir string +} + +var _ IssueWriter = &LocalIssueWriter{} + +func NewLocalIssueWriter(outputDir string) (IssueWriter, error) { + // Set up the directory structure for local output + err := os.MkdirAll(outputDir, os.ModePerm) + if err != nil { + return nil, err + } + return &LocalIssueWriter{ + outputDir: outputDir, + }, nil +} + +func (l *LocalIssueWriter) Write(_ context.Context, release *github.RepositoryRelease, contents string) error { + version, err := semver.NewVersion(release.GetTagName()) + if err != nil { + return err + } + filename := path.Join(l.outputDir, version.String()+".md") + f, err := os.OpenFile(filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0644) + if err != nil { + return err + } + _, err = fmt.Fprintf(f, contents) + if err != nil { + return err + } + return nil +} diff --git a/securityscanutils/issuewriter/noop_writer.go b/securityscanutils/issuewriter/noop_writer.go new file mode 100644 index 00000000..9c20fe6e --- /dev/null +++ b/securityscanutils/issuewriter/noop_writer.go @@ -0,0 +1,21 @@ +package issuewriter + +import ( + "context" + + "github.com/google/go-github/v32/github" +) + +// NoopWriter provides a no-op implementation of the IssueWriter interface, used when the +// specified scan action is `none`. +type NoopWriter struct{} + +var _ IssueWriter = &NoopWriter{} + +func NewNoopWriter() IssueWriter { + return &NoopWriter{} +} + +func (n *NoopWriter) Write(_ context.Context, _ *github.RepositoryRelease, _ string) error { + return nil +} diff --git a/securityscanutils/predicate.go b/securityscanutils/predicate.go index ecd9f60d..8750d845 100644 --- a/securityscanutils/predicate.go +++ b/securityscanutils/predicate.go @@ -24,6 +24,9 @@ func NewSecurityScanRepositoryReleasePredicate(constraint *semver.Constraints) * } func (s *securityScanRepositoryReleasePredicate) Apply(release *github.RepositoryRelease) bool { + // Note: GetPrerelease() is referring to a pre-release in GitHub. The term pre-release is + // slightly overloaded between GitHub and semver. We _do_ want to scan semver pre-releases + // as those correspond to GitHub releases whose tag matches the pattern of a semver pre-release. if release.GetPrerelease() || release.GetDraft() { // Do not include pre-releases or drafts return false diff --git a/securityscanutils/predicate_test.go b/securityscanutils/predicate_test.go index 5f555cde..038873ef 100644 --- a/securityscanutils/predicate_test.go +++ b/securityscanutils/predicate_test.go @@ -20,7 +20,7 @@ var _ = Describe("Predicate", func() { ) BeforeEach(func() { - twoPlusConstraint, err := semver.NewConstraint(fmt.Sprintf(">= %s", "v2.0.0")) + twoPlusConstraint, err := semver.NewConstraint(fmt.Sprintf(">= %s", "v2.0.0-0")) Expect(err).NotTo(HaveOccurred()) releasePredicate = securityscanutils.NewSecurityScanRepositoryReleasePredicate(twoPlusConstraint) @@ -34,9 +34,6 @@ var _ = Describe("Predicate", func() { Entry("release is draft", &github.RepositoryRelease{ Draft: github.Bool(true), }, false), - Entry("release is pre-release", &github.RepositoryRelease{ - Prerelease: github.Bool(true), - }, false), Entry("release tag does not respect semver", &github.RepositoryRelease{ TagName: github.String("non-semver-tag-name"), }, false), @@ -46,6 +43,15 @@ var _ = Describe("Predicate", func() { Entry("release tag does pass version constraint", &github.RepositoryRelease{ TagName: github.String("v2.0.1"), }, true), + Entry("release tag has beta", &github.RepositoryRelease{ + TagName: github.String("v2.0.1-beta1"), + }, true), + Entry("release tag has rc", &github.RepositoryRelease{ + TagName: github.String("v2.0.1-rc2"), + }, true), + Entry("release is a pre-release", &github.RepositoryRelease{ + Prerelease: github.Bool(true), + }, false), ) }) diff --git a/securityscanutils/securityscan.go b/securityscanutils/securityscan.go index 10150881..c8b14d29 100644 --- a/securityscanutils/securityscan.go +++ b/securityscanutils/securityscan.go @@ -6,7 +6,6 @@ import ( "context" "encoding/base64" "fmt" - "io/ioutil" "math" "os" "os/exec" @@ -14,6 +13,10 @@ import ( "strings" "time" + "github.com/solo-io/go-utils/securityscanutils/issuewriter" + + "github.com/pkg/errors" + "github.com/solo-io/go-utils/osutils/executils" "github.com/solo-io/go-utils/contextutils" @@ -46,8 +49,8 @@ type SecurityScanRepo struct { trivyScanner *TrivyScanner - // The writer responsible for generating Github Issues for certain releases - githubIssueWriter *GithubIssueWriter + // The writer responsible for generating Issues for certain releases + issueWriter issuewriter.IssueWriter } type SecurityScanOpts struct { @@ -68,8 +71,17 @@ type SecurityScanOpts struct { │ ├─ repo2/ │ │ ├─ 1.4.13/ │ │ ├─ 1.5.1/ + ├─ issue_results/ + │ ├─ repo1/ + │ │ ├─ 1.4.12.md + │ │ ├─ 1.5.0.md + │ ├─ repo2/ + │ │ ├─ 1.4.13.md + │ │ ├─ 1.5.1.md */ OutputDir string + // Output the would-be github issue Markdown to local files + OutputResultLocally bool // A mapping of version constraints to images scanned. // If 1.6 had images "gloo", "discovery" and 1.7 introduced a new image "rate-limit", // the map would look like: @@ -100,6 +112,10 @@ type SecurityScanOpts struct { // 2. The version is the latest patch version (Major.Minor.Patch) // If set to true, will override the behavior of CreateGithubIssuePerVersion CreateGithubIssueForLatestPatchVersion bool + + // Additional context to add to the top of the generated vulnerability report. + // Example: This could be used to provide debug instructions to developers. + AdditionalContext string } // Main method to call on SecurityScanner which generates .md and .sarif files @@ -185,12 +201,13 @@ func (s *SecurityScanner) initializeRepoConfiguration(ctx context.Context, repo logger.Debugf("Number of github releases to scan: %d", len(releasesToScan)) // Initialize a local store of GitHub issues if we will be creating new issues - githubRepo := GithubRepo{ + githubRepo := issuewriter.GithubRepo{ RepoName: repo.Repo, Owner: repo.Owner, } // Default to not creating any issues var issuePredicate githubutils.RepositoryReleasePredicate = &githubutils.NoReleasesPredicate{} + useGithubWriter := repoOptions.CreateGithubIssuePerVersion || repoOptions.CreateGithubIssueForLatestPatchVersion if repoOptions.CreateGithubIssuePerVersion { // Create Github issue for all releases, if configured issuePredicate = &githubutils.AllReleasesPredicate{} @@ -200,8 +217,19 @@ func (s *SecurityScanner) initializeRepoConfiguration(ctx context.Context, repo // Create Github issues for all releases in the set issuePredicate = NewLatestPatchRepositoryReleasePredicate(releasesToScan) } - repo.githubIssueWriter = NewGithubIssueWriter(githubRepo, s.githubClient, issuePredicate) - logger.Debugf("GithubIssueWriter configured with Predicate: %+v", issuePredicate) + if useGithubWriter { + repo.issueWriter = issuewriter.NewGithubIssueWriter(githubRepo, s.githubClient, issuePredicate) + logger.Debugf("GithubIssueWriter configured with Predicate: %+v", issuePredicate) + } else if repo.Opts.OutputResultLocally { + repo.issueWriter, err = issuewriter.NewLocalIssueWriter(path.Join(repo.Opts.OutputDir, githubRepo.RepoName, "issue_results")) + if err != nil { + return err + } + logger.Debugf("LocalIssueWriter configured with Predicate: %+v", issuePredicate) + } else { + repo.issueWriter = issuewriter.NewNoopWriter() + logger.Debugf("NoopIssueWriter configured with Predicate: %+v", issuePredicate) + } repo.trivyScanner = NewTrivyScanner(executils.CombinedOutputWithStatus) @@ -218,8 +246,8 @@ func (r *SecurityScanRepo) RunMarkdownScan(ctx context.Context, release *github. return err } version := versionToScan.String() - outputDir := path.Join(r.Opts.OutputDir, r.Repo, "markdown_results", version) - err = os.MkdirAll(outputDir, os.ModePerm) + trivyScanOutputDir := path.Join(r.Opts.OutputDir, r.Repo, "markdown_results", version) + err = os.MkdirAll(trivyScanOutputDir, os.ModePerm) if err != nil { return err } @@ -233,14 +261,17 @@ func (r *SecurityScanRepo) RunMarkdownScan(ctx context.Context, release *github. imageWithRepo = fmt.Sprintf("%s/%s:%s", r.Opts.ImageRepo, image, version) } fileName := fmt.Sprintf("%s_cve_report.docgen", image) - output := path.Join(outputDir, fileName) + output := path.Join(trivyScanOutputDir, fileName) _, vulnFound, err := r.trivyScanner.ScanImage(ctx, imageWithRepo, markdownTplFile, output) if err != nil { - return eris.Wrapf(err, "error running image scan on image %s", imageWithRepo) + if errors.Is(err, UnrecoverableErr) { + return eris.Wrapf(err, "error running image scan on image %s", imageWithRepo) + } + vulnerabilityMd += fmt.Sprintf("# %s\n\n %s", imageWithRepo, err) } if vulnFound { - trivyScanMd, err := ioutil.ReadFile(output) + trivyScanMd, err := os.ReadFile(output) if err != nil { return eris.Wrapf(err, "error reading trivy markdown scan file %s to generate github issue", output) } @@ -248,8 +279,11 @@ func (r *SecurityScanRepo) RunMarkdownScan(ctx context.Context, release *github. } } - // Create / Update Github issue for the repo if a vulnerability is found - return r.githubIssueWriter.CreateUpdateVulnerabilityIssue(ctx, release, vulnerabilityMd) + if vulnerabilityMd != "" && r.Opts.AdditionalContext != "" { + vulnerabilityMd = fmt.Sprintf("%s\n%s", r.Opts.AdditionalContext, vulnerabilityMd) + } + // Create / Update issue for the repo if a vulnerability is found + return r.issueWriter.Write(ctx, release, vulnerabilityMd) } func (r *SecurityScanRepo) runGithubSarifScan(ctx context.Context, release *github.RepositoryRelease, sarifTplFile string) error { @@ -331,7 +365,7 @@ func (r *SecurityScanRepo) UploadSecurityScanToGithub(fileName, versionTag strin if err != nil { return err } - sarifFileBytes, err := ioutil.ReadFile(fileName) + sarifFileBytes, err := os.ReadFile(fileName) if err != nil { return eris.Wrapf(err, "error reading sarif file %s", fileName) } diff --git a/securityscanutils/securityscan_test.go b/securityscanutils/securityscan_test.go index 8ade460b..d0dfa3dd 100644 --- a/securityscanutils/securityscan_test.go +++ b/securityscanutils/securityscan_test.go @@ -8,6 +8,8 @@ import ( "path" "sort" + "github.com/solo-io/go-utils/fileutils" + "github.com/Masterminds/semver/v3" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -46,10 +48,12 @@ var _ = Describe("Security Scan Suite", func() { Repo: glooRepoName, Owner: "solo-io", Opts: &SecurityScanOpts{ - OutputDir: outputDir, + OutputDir: outputDir, + OutputResultLocally: true, ImagesPerVersion: map[string][]string{ "v1.6.0": {"gloo"}, - "v1.7.0": {"gloo", "discovery"}, + // Scan should continue in the case an image cannot be found + "v1.7.0": {"thisimagecannotbefound", "gloo", "discovery"}, }, VersionConstraint: verConstraint, ImageRepo: "quay.io/solo-io", @@ -63,11 +67,16 @@ var _ = Describe("Security Scan Suite", func() { Expect(err).NotTo(HaveOccurred()) ExpectDirToHaveFiles(outputDir, "gloo") + // Have a markdown file for each version we scanned + glooDir := path.Join(outputDir, "gloo") + ExpectDirToHaveFiles(glooDir, "issue_results", "markdown_results") + githubIssueDir := path.Join(glooDir, "issue_results") + ExpectDirToHaveFiles(githubIssueDir, "1.6.0.md", "1.7.0.md") // Have a directory for each repo we scanned markdownDir := path.Join(outputDir, "gloo", "markdown_results") // Have a directory for each version we scanned ExpectDirToHaveFiles(markdownDir, "1.6.0", "1.7.0") - // Expect there to be a generated generated file for each image per version + // Expect there to be a generated docgen file for each image per version ExpectDirToHaveFiles(path.Join(markdownDir, "1.6.0"), "gloo_cve_report.docgen") ExpectDirToHaveFiles(path.Join(markdownDir, "1.7.0"), "discovery_cve_report.docgen", "gloo_cve_report.docgen") }) @@ -122,6 +131,89 @@ var _ = Describe("Security Scan Suite", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("version 1.7.0 matched no constraints and has no images to scan")) }) + + When("scan has unrecoverable error", func() { + It("short-circuits", func() { + verConstraint, err := semver.NewConstraint("=v1.6.0 || =v1.7.0") + Expect(err).NotTo(HaveOccurred()) + fmt.Println("Output dir:", outputDir) + secScanner := &SecurityScanner{ + Repos: []*SecurityScanRepo{{ + Repo: glooRepoName, + Owner: "solo-io", + Opts: &SecurityScanOpts{ + OutputDir: outputDir, + OutputResultLocally: true, + ImagesPerVersion: map[string][]string{ + "v1.7.0": {"gloo; $(poorly formatted image name to force UnrecoverableError)"}, + }, + VersionConstraint: verConstraint, + ImageRepo: "quay.io/solo-io", + UploadCodeScanToGithub: false, + }, + }}, + } + + // Run security scan + err = secScanner.GenerateSecurityScans(context.TODO()) + Expect(err).To(MatchError(UnrecoverableErr)) + + ExpectDirToHaveFiles(outputDir, "gloo") + // No images scanned; no files should exist + glooDir := path.Join(outputDir, "gloo") + ExpectDirToHaveFiles(glooDir, "issue_results", "markdown_results") + localIssueDir := path.Join(glooDir, "issue_results") + ExpectDirToHaveFiles(localIssueDir) + // Have a directory for each repo we scanned + markdownDir := path.Join(outputDir, "gloo", "markdown_results") + // Have a directory for each version we scanned + ExpectDirToHaveFiles(markdownDir, "1.7.0") + ExpectDirToHaveFiles(path.Join(markdownDir, "1.7.0")) + }) + }) + + When("scan has recoverable error", func() { + It("contains error in generated file", func() { + verConstraint, err := semver.NewConstraint("=v1.7.0") + Expect(err).NotTo(HaveOccurred()) + fmt.Println("Output dir:", outputDir) + secScanner := &SecurityScanner{ + Repos: []*SecurityScanRepo{{ + Repo: glooRepoName, + Owner: "solo-io", + Opts: &SecurityScanOpts{ + OutputDir: outputDir, + OutputResultLocally: true, + ImagesPerVersion: map[string][]string{ + "v1.7.0": {"thisimagedoesnotexist"}, + }, + VersionConstraint: verConstraint, + ImageRepo: "quay.io/solo-io", + UploadCodeScanToGithub: false, + }, + }}, + } + + // Run security scan + err = secScanner.GenerateSecurityScans(context.TODO()) + Expect(err).NotTo(HaveOccurred()) + + ExpectDirToHaveFiles(outputDir, "gloo") + // No images scanned; no files should exist + glooDir := path.Join(outputDir, "gloo") + ExpectDirToHaveFiles(glooDir, "issue_results", "markdown_results") + localIssueDir := path.Join(glooDir, "issue_results") + ExpectDirToHaveFiles(localIssueDir, "1.7.0.md") + contents, err := fileutils.ReadFileString(path.Join(localIssueDir, "1.7.0.md")) + Expect(err).NotTo(HaveOccurred()) + Expect(contents).To(ContainSubstring(ImageNotFoundError.Error())) + // Have a directory for each repo we scanned + markdownDir := path.Join(outputDir, "gloo", "markdown_results") + // Have a directory for each version we scanned + ExpectDirToHaveFiles(markdownDir, "1.7.0") + ExpectDirToHaveFiles(path.Join(markdownDir, "1.7.0")) + }) + }) }) }) diff --git a/securityscanutils/trivy_scanner.go b/securityscanutils/trivy_scanner.go index b589c905..fdf728ae 100644 --- a/securityscanutils/trivy_scanner.go +++ b/securityscanutils/trivy_scanner.go @@ -8,6 +8,8 @@ import ( "strings" "time" + "github.com/pkg/errors" + "github.com/solo-io/go-utils/contextutils" "github.com/rotisserie/eris" @@ -16,6 +18,10 @@ import ( // Status code returned by Trivy if a vulnerability is found const VulnerabilityFoundStatusCode = 52 +var RecoverableErr = errors.New("Recoverable") +var UnrecoverableErr = errors.New("Unrecoverable") +var ImageNotFoundError = eris.Wrap(RecoverableErr, "❗IMAGE MISSING UNEXPECTEDLY❗") + type CmdExecutor func(cmd *exec.Cmd) ([]byte, int, error) type TrivyScanner struct { @@ -81,27 +87,24 @@ func (t *TrivyScanner) executeScanWithRetries(ctx context.Context, scanArgs []st // If there is no error, the scan completed and no vulnerability was found, don't retry if err == nil { logger.Debugf("Trivy returned %d after %s on %s", statusCode, time.Since(attemptStart).String(), imageUri) - return true, false, err + return true, false, nil } // If there is no image, don't retry if IsImageNotFoundErr(string(out)) { logger.Warnf("Trivy scan with args [%v] produced image not found error", scanArgs) - // swallow error if image is not found error, so that we can continue scanning releases - // even if some releases failed and we didn't publish images for those releases - // this error used to happen if a release was a pre-release and therefore images - // weren't pushed to the container registry. - // we have since filtered out non-release images from being scanned so this warning - // shouldn't occur, but leaving here in case there was another edge case we missed - return false, false, nil + // Indicate the scan has not yet completed and no vulnerability was found but there was an ImageNotFoundError. + // The upstream handler should check specifically for this error to ensure that the remaining images for + // the specified version are scanned. + return false, false, ImageNotFoundError } //This backoff strategy is intended to handle network issues(i.e. an http 5xx error) t.scanBackoffStrategy(attempt) } // We only reach here if we exhausted our retries - return false, false, eris.Errorf("Trivy scan with args [%v] did not complete after %d attempts", scanArgs, t.scanMaxRetries) + return false, false, eris.Wrapf(UnrecoverableErr, "Trivy scan with args [%v] did not complete after %d attempts", scanArgs, t.scanMaxRetries) } func IsImageNotFoundErr(logs string) bool { diff --git a/securityscanutils/trivy_scanner_test.go b/securityscanutils/trivy_scanner_test.go index 2d3eec6c..5af9c5d8 100644 --- a/securityscanutils/trivy_scanner_test.go +++ b/securityscanutils/trivy_scanner_test.go @@ -56,7 +56,7 @@ var _ = Describe("Trivy Scanner", func() { }) completed, vulnFound, err := t.ScanImage(context.TODO(), inputImage, inputMarkdownTemplateFile, outputFile) - Expect(err).NotTo(HaveOccurred()) + Expect(err).To(MatchError(ImageNotFoundError)) Expect(completed).To(Equal(false)) Expect(vulnFound).To(Equal(false))