diff --git a/changelog/v0.21.27/scan-updates.yaml b/changelog/v0.21.27/scan-updates.yaml new file mode 100644 index 00000000..89658468 --- /dev/null +++ b/changelog/v0.21.27/scan-updates.yaml @@ -0,0 +1,5 @@ +changelog: + - type: FIX + description: Trivy scanning option to only update github issues for Max Patch in a majorminor combo + issueLink: https://github.com/solo-io/go-utils/issues/469 + \ No newline at end of file diff --git a/githubutils/repo.go b/githubutils/repo.go index d0e6d21c..e51e2b1a 100644 --- a/githubutils/repo.go +++ b/githubutils/repo.go @@ -145,17 +145,100 @@ func GetRawGitFile(ctx context.Context, client *github.Client, content *github.R return byt, err } +// A RepoFilter can check if a release shold be filtered out based on a condition. +// In general filters may be stateful and be a StatefulRepoFilter +type RepoFilter interface { + // Apply the filter and return true if it conforms to the flter and should + // not be removed. + Apply(release *github.RepositoryRelease) bool +} + +// A StatefulRepoFilter has a stateful prerun check and along with a way to determine +// if a release conforms to our desired filter. +type StatefulRepoFilter interface { + // Apply the filter and return true if it conforms to the flter and should + // not be removed. + Apply(release *github.RepositoryRelease) bool + // PreFilterCheck is a preprocessing state that updates the internal state + // of the filter. It should not have sideeffects. + PreFilterCheck(release *github.RepositoryRelease) +} + +// RetrievalOption applies the option the passed in set of retrieval options. +type RetrievalOption func(*RetrievalOptions) + +// RetrievalOptions are the list of optional flags on repo-retrieval. +type RetrievalOptions struct { + Sort bool + MaxReleases int + Filters []RepoFilter +} + +// NewCombinedFilter returns a nice combined filter from all the current +// repofilters that have been set. +func NewCombinedFilter(filters ...RepoFilter) StatefulRepoFilter { + cf := combinedFilter{} + + for _, rf := range filters { + if srf, ok := rf.(StatefulRepoFilter); ok { + cf.statefulFilters = append(cf.statefulFilters, srf) + } else { + cf.filters = append(cf.filters, rf) + } + + } + + return &cf +} + +type combinedFilter struct { + filters []RepoFilter + statefulFilters []StatefulRepoFilter +} + +// Apply all the subfilters and if any reject the release, return false. +func (c *combinedFilter) Apply(release *github.RepositoryRelease) bool { + for _, f := range c.filters { + if !f.Apply(release) { + return false + } + } + for _, f := range c.statefulFilters { + if !f.Apply(release) { + return false + } + } + return true +} + +// PreFilterCheck for all the combined stateful filters. +func (c *combinedFilter) PreFilterCheck(release *github.RepositoryRelease) { + for _, f := range c.statefulFilters { + f.PreFilterCheck(release) + } +} + +// RepositoryReleasePredicate is a non-stateful release contstraint +// that can be checked to see if a release should be filtered. +// The easiest form is to check whether a release has a given predicate. type RepositoryReleasePredicate interface { Apply(release *github.RepositoryRelease) bool } +// AllReleasesPredicate is a filter that filters... well nothing. type AllReleasesPredicate struct { } +// Apply always returns that the release should not be filtered. func (a *AllReleasesPredicate) Apply(_ *github.RepositoryRelease) bool { return true } +// PreFilterCheck for all releases is a no-op. +func (a *AllReleasesPredicate) PreFilterCheck(release *github.RepositoryRelease) { + return +} + func GetAllRepoReleases(ctx context.Context, client *github.Client, owner, repo string) ([]*github.RepositoryRelease, error) { return GetAllRepoReleasesWithMax(ctx, client, owner, repo, math.MaxInt32) } @@ -165,8 +248,36 @@ func GetAllRepoReleasesWithMax(ctx context.Context, client *github.Client, owner } func GetRepoReleasesWithPredicateAndMax(ctx context.Context, client *github.Client, owner, repo string, predicate RepositoryReleasePredicate, maxReleases int) ([]*github.RepositoryRelease, error) { + opt := func(ro *RetrievalOptions) { + ro.MaxReleases = maxReleases + ro.Filters = append(ro.Filters, predicate) + + } + return GetRepoReleases(ctx, client, owner, repo, opt) +} + +// GetRepoReleases retrieves releases from a repository given a set of retrieval options. +func GetRepoReleases(ctx context.Context, client *github.Client, owner, repo string, opts ...RetrievalOption) ([]*github.RepositoryRelease, error) { var allReleases []*github.RepositoryRelease - for i := MIN_GITHUB_PAGE_NUM; len(allReleases) < maxReleases; i += 1 { + ro := RetrievalOptions{} + + for _, opt := range opts { + opt(&ro) + } + + filter := NewCombinedFilter(ro.Filters...) + // uploadFilter := newCombinedFilter(ro.uploadFilters...) + + // This is silly but we dont want to change the exported value + i := MIN_GITHUB_PAGE_NUM - 1 + + for { + // check to see if we have gotten enough releases + if len(allReleases) >= ro.MaxReleases { + // One more filtering where we check for + allReleases = FilterRepositoryReleases(allReleases, filter) + } + releases, _, err := client.Repositories.ListReleases(ctx, owner, repo, &github.ListOptions{ Page: i, PerPage: MAX_GITHUB_RESULTS_PER_PAGE, @@ -175,9 +286,12 @@ func GetRepoReleasesWithPredicateAndMax(ctx context.Context, client *github.Clie return nil, err } - // Only append releases if they match the predicate + // Only append releases if they match the filter (predicate or other functions) // This is required since the Github API does not expose parameters to filter the RepositoryRelease list in the request - filteredReleases := FilterRepositoryReleases(releases, predicate) + filteredReleases := FilterRepositoryReleases(releases, filter) + if ro.Sort { + SortReleasesBySemver(filteredReleases) + } allReleases = append(allReleases, filteredReleases...) // If the number of releases on this page is less than the results per page, @@ -188,16 +302,25 @@ func GetRepoReleasesWithPredicateAndMax(ctx context.Context, client *github.Clie } // Ensure that if we have exceeded the number of maxReleases, we truncate the list - if len(allReleases) > maxReleases { - allReleases = allReleases[:maxReleases] + if len(allReleases) > ro.MaxReleases { + allReleases = allReleases[:ro.MaxReleases] } return allReleases, nil } -func FilterRepositoryReleases(releases []*github.RepositoryRelease, predicate RepositoryReleasePredicate) []*github.RepositoryRelease { +// FilterRepositoryReleases applys the filtering logic to rebuild the slice of +// releases that we care about. +func FilterRepositoryReleases(releases []*github.RepositoryRelease, filter RepoFilter) []*github.RepositoryRelease { var filteredReleases []*github.RepositoryRelease + // in case the filter is stateful + if statefulF, ok := filter.(StatefulRepoFilter); ok { + for _, release := range releases { + statefulF.PreFilterCheck(release) + } + } + for _, release := range releases { - if predicate.Apply(release) { + if filter.Apply(release) { filteredReleases = append(filteredReleases, release) } } diff --git a/securityscanutils/securityscan.go b/securityscanutils/securityscan.go index 95b2630c..8e035178 100644 --- a/securityscanutils/securityscan.go +++ b/securityscanutils/securityscan.go @@ -29,11 +29,16 @@ import ( "github.com/solo-io/go-utils/log" ) +// SecurityScanner contains a set of repos and a client to use with +// which to scan them. This assumes that they are on github. type SecurityScanner struct { Repos []*SecurityScanRepo githubClient *github.Client } +// SecurityScanRepo is the per repo construct used by securityscanner. +// This includes the passed in options as well as a way to store +// all issues that had the trivy label. type SecurityScanRepo struct { Repo string Owner string @@ -42,6 +47,8 @@ type SecurityScanRepo struct { allGithubIssues []*github.Issue } +// SecurityScanOpts is consumed as a struct that details how a given repo should +// be scanned and reported on. type SecurityScanOpts struct { // The following directory structure will be created in your output dir. /* @@ -86,17 +93,26 @@ type SecurityScanOpts struct { // Creates github issue if image vulnerabilities are found CreateGithubIssuePerVersion bool + + // ScanLatestInLTSOnly will only scan the latest version in the LTS + // This is nice as it gets what is actionable but does not serve to populate + // older release vulnerabilities. + ScanLatestsInLTSOnly bool + + // Creates github issues for the largest patch versions overriden by CreateGithubIssuePerVersion + CreateGithubIssuePerLTSVersion bool } -// Status code returned by Trivy if a vulnerability is found +// VulnerabilityFoundStatusCode is Trivy's returned code for a vulnerability. const VulnerabilityFoundStatusCode = 52 -// Labels that are applied to github issues that security scan generates +// TrivyLabels are the set of labels that are applied to github issues +// which the security scan generates var TrivyLabels = []string{"trivy", "vulnerability"} -// Main method to call on SecurityScanner which generates .md and .sarif files -// in OutputDir as defined above per repo. If UploadCodeScanToGithub is true, -// sarif files will be uploaded to the repository's code-scanning endpoint. +// GenerateSecurityScans is the overarching `main` method which generates +// .md and .sarif files OutputDir as well as optionall uploading scans / issues +// to github if the toggles are set. func (s *SecurityScanner) GenerateSecurityScans(ctx context.Context) error { var err error s.githubClient, err = githubutils.GetClient(ctx) @@ -116,36 +132,25 @@ func (s *SecurityScanner) GenerateSecurityScans(ctx context.Context) error { os.Remove(sarifTplFile) }() + // fails fast and does not continue to the next repo if there is an error for _, repo := range s.Repos { opts := repo.Opts // This predicate filters releases so we only perform scans on the images that are relevant to the repo - repoReleasePredicate := getReleasePredicateForSecurityScan(opts.VersionConstraint) - maxReleasesToScan := math.MaxInt32 - partialFilteredReleases, err := githubutils.GetRepoReleasesWithPredicateAndMax(ctx, s.githubClient, repo.Owner, repo.Repo, repoReleasePredicate, maxReleasesToScan) - if err != nil { - return eris.Wrapf(err, "unable to fetch all github releases for github.com/%s/%s", repo.Owner, repo.Repo) - } - githubutils.SortReleasesBySemver(partialFilteredReleases) - filteredReleases := []*github.RepositoryRelease{} - - // We could use maxint but we dont really care - // as we can just check if major minor changed - recentMajor := -1 - recentMinor := -1 - for _, release := range partialFilteredReleases { - version, err := versionutils.ParseVersion(release.GetTagName()) - if err != nil { - continue - } - if version.Major == recentMajor && version.Minor == recentMinor { - continue + repoFilter := getReleasePredicateForSecurityScan(opts.VersionConstraint) + repoLTSFilter := &SecurityScanRepositoryLTSFilter{} + filterOption := func(ro *githubutils.RetrievalOptions) { + ro.Sort = true + ro.MaxReleases = math.MaxInt32 + ro.Filters = append(ro.Filters, repoFilter) + if opts.ScanLatestsInLTSOnly { + ro.Filters = append(ro.Filters, repoLTSFilter) } + } - // This is the largest patch release - recentMajor = version.Major - recentMinor = version.Minor - filteredReleases = append(filteredReleases, release) + filteredReleases, err := githubutils.GetRepoReleases(ctx, s.githubClient, repo.Owner, repo.Repo, filterOption) + if err != nil { + return eris.Wrapf(err, "unable to fetch all github releases for github.com/%s/%s", repo.Owner, repo.Repo) } if repo.Opts.CreateGithubIssuePerVersion { @@ -157,14 +162,28 @@ func (s *SecurityScanner) GenerateSecurityScans(ctx context.Context) error { return eris.Wrapf(err, "error fetching all issues from github.com/%s/%s", repo.Owner, repo.Repo) } } + + if repo.Opts.CreateGithubIssuePerLTSVersion && len(repoLTSFilter.MaxPatchForMinor) == 0 { + for _, fr := range filteredReleases { + repoLTSFilter.PreFilterCheck(fr) + } + } + for _, release := range filteredReleases { // We can swallow the error here, any releases with improper tag names // will not be included in the filtered list ver, _ := semver.NewVersion(release.GetTagName()) - err = repo.RunMarkdownScan(ctx, s.githubClient, ver, markdownTplFile) + vulnerabilityMd, err := repo.GetMarkdownScanResults(ctx, s.githubClient, ver, markdownTplFile) if err != nil { return eris.Wrapf(err, "error generating markdown file from security scan for version %s", release.GetTagName()) } + + if repo.Opts.CreateGithubIssuePerVersion || repo.Opts.CreateGithubIssuePerLTSVersion && repoLTSFilter.Apply(release) { + err = repo.CreateUpdateVulnerabilityIssue(ctx, s.githubClient, ver.String(), vulnerabilityMd) + if err != nil { + return eris.Wrapf(err, "error updating github issue for version %s", release.GetTagName()) + } + } // Only generate sarif files if we are uploading code scan results // to github if repo.Opts.UploadCodeScanToGithub { @@ -179,16 +198,38 @@ func (s *SecurityScanner) GenerateSecurityScans(ctx context.Context) error { return nil } +// RunMarkdownScan on the given version of the repo and upload results to github. func (r *SecurityScanRepo) RunMarkdownScan(ctx context.Context, client *github.Client, versionToScan *semver.Version, markdownTplFile string) error { - images, err := r.GetImagesToScan(versionToScan) + vulnerabilityMd, err := r.GetMarkdownScanResults(ctx, client, versionToScan, markdownTplFile) if err != nil { return err } + // We did not find vulnerabilities in any of the images for this version + // do not create an empty github issue + if vulnerabilityMd == "" { + return nil + } + + // Create / Update Github issue for the repo if a vulnerability is found + // and CreateGithubIssuePerVersion is set to true + if r.Opts.CreateGithubIssuePerVersion || r.Opts.CreateGithubIssuePerLTSVersion { + return r.CreateUpdateVulnerabilityIssue(ctx, client, versionToScan.String(), vulnerabilityMd) + } + return nil +} + +// GetMarkdownScanResults generates the vulenrabiliy report in markdown format +func (r *SecurityScanRepo) GetMarkdownScanResults(ctx context.Context, client *github.Client, versionToScan *semver.Version, markdownTplFile string) (string, error) { + + images, err := r.GetImagesToScan(versionToScan) + if err != nil { + return "", err + } version := versionToScan.String() outputDir := path.Join(r.Opts.OutputDir, r.Repo, "markdown_results", version) err = os.MkdirAll(outputDir, os.ModePerm) if err != nil { - return err + return "", err } var vulnerabilityMd string for _, image := range images { @@ -203,29 +244,20 @@ func (r *SecurityScanRepo) RunMarkdownScan(ctx context.Context, client *github.C output := path.Join(outputDir, fileName) _, vulnFound, err := RunTrivyScan(imageWithRepo, version, markdownTplFile, output) if err != nil { - return eris.Wrapf(err, "error running image scan on image %s", imageWithRepo) + return "", eris.Wrapf(err, "error running image scan on image %s", imageWithRepo) } if vulnFound { trivyScanMd, err := ioutil.ReadFile(output) if err != nil { - return eris.Wrapf(err, "error reading trivy markdown scan file %s to generate github issue", output) + return "", eris.Wrapf(err, "error reading trivy markdown scan file %s to generate github issue", output) } vulnerabilityMd += fmt.Sprintf("# %s\n\n %s\n\n", imageWithRepo, trivyScanMd) } } - // Create / Update Github issue for the repo if a vulnerability is found - // and CreateGithubIssuePerVersion is set to true - if r.Opts.CreateGithubIssuePerVersion { - if vulnerabilityMd == "" { - // We did not find vulnerabilities in any of the images for this version - // do not create an empty github issue - return nil - } - return r.CreateUpdateVulnerabilityIssue(ctx, client, version, vulnerabilityMd) - } - return nil + + return vulnerabilityMd, nil } func (r *SecurityScanRepo) RunGithubSarifScan(versionToScan *semver.Version, sarifTplFile string) error { @@ -286,7 +318,7 @@ func (r *SecurityScanRepo) GetImagesToScan(versionToScan *semver.Version) ([]str return stringutils.Keys(imagesToScan), nil } -func getReleasePredicateForSecurityScan(versionConstraint *semver.Constraints) *SecurityScanRepositoryReleasePredicate { +func getReleasePredicateForSecurityScan(versionConstraint *semver.Constraints) githubutils.RepoFilter { return &SecurityScanRepositoryReleasePredicate{ versionConstraint: versionConstraint, } @@ -301,6 +333,7 @@ type SecurityScanRepositoryReleasePredicate struct { versionConstraint *semver.Constraints } +// Apply the predicate and return if the release conforms to the version contstraint. func (s *SecurityScanRepositoryReleasePredicate) Apply(release *github.RepositoryRelease) bool { if release.GetPrerelease() || release.GetDraft() { // Do not include pre-releases or drafts @@ -323,6 +356,40 @@ func (s *SecurityScanRepositoryReleasePredicate) Apply(release *github.Repositor return true } +// SecurityScanRepositoryLTSFilter is a statefulrepofilter that assumes +// releases are already sorted by semver. +type SecurityScanRepositoryLTSFilter struct { + recentMajor, recentMinor int + MaxPatchForMinor map[string]struct{} +} + +// Apply the filter and only return if the release is the latest in a branch +func (s *SecurityScanRepositoryLTSFilter) Apply(release *github.RepositoryRelease) bool { + version, err := versionutils.ParseVersion(release.GetTagName()) + if err != nil { + return false + } + _, isMaxPatchVersion := s.MaxPatchForMinor[version.String()] + return isMaxPatchVersion +} + +// PreFilterCheck constructs the map of most recent major minor and therefore +// defines an LTS release. +func (s *SecurityScanRepositoryLTSFilter) PreFilterCheck(release *github.RepositoryRelease) { + version, err := versionutils.ParseVersion(release.GetTagName()) + if err != nil { + return + } + if version.Major == s.recentMajor && version.Minor == s.recentMinor { + return + } + + // This is the largest patch release + s.recentMajor = version.Major + s.recentMinor = version.Minor + s.MaxPatchForMinor[version.String()] = struct{}{} +} + // Runs trivy scan command // Returns (trivy scan ran successfully, vulnerabilities found, error running trivy scan) func RunTrivyScan(image, version, templateFile, output string) (bool, bool, error) { @@ -467,26 +534,24 @@ func (r *SecurityScanRepo) CreateUpdateVulnerabilityIssue(ctx context.Context, c Body: github.String(vulnerabilityMarkdown), Labels: &TrivyLabels, } - createNewIssue := true for _, issue := range r.allGithubIssues { // If issue already exists, update existing issue with new security scan if issue.GetTitle() == issueTitle { - // Only create new issue if issue does not already exist - createNewIssue = false err := githubutils.UpdateIssue(ctx, client, r.Owner, r.Repo, issue.GetNumber(), issueRequest) if err != nil { return eris.Wrapf(err, "error updating issue with issue request %+v", issueRequest) } - break + return nil } } - if createNewIssue { - _, err := githubutils.CreateIssue(ctx, client, r.Owner, r.Repo, issueRequest) - if err != nil { - return eris.Wrapf(err, "error creating issue with issue request %+v", issueRequest) - } + + // No existing ticket found to update, create a new issue + _, err := githubutils.CreateIssue(ctx, client, r.Owner, r.Repo, issueRequest) + if err != nil { + return eris.Wrapf(err, "error creating issue with issue request %+v", issueRequest) } + return nil }