From eb92dc078efda0fe63ecd7e6ee24d727cb0363d9 Mon Sep 17 00:00:00 2001 From: Leo Di Donato <120051+leodido@users.noreply.github.com> Date: Thu, 16 Jul 2026 07:34:05 +0000 Subject: [PATCH] fix(sbom): reuse grype database provider Co-authored-by: Codex --- pkg/leeway/sbom-scan.go | 127 ++++++++++++++---- pkg/leeway/sbom_scan_test.go | 247 +++++++++++++++++++++++++++++++++++ 2 files changed, 351 insertions(+), 23 deletions(-) create mode 100644 pkg/leeway/sbom_scan_test.go diff --git a/pkg/leeway/sbom-scan.go b/pkg/leeway/sbom-scan.go index 0cf0cc8d..c25c408d 100644 --- a/pkg/leeway/sbom-scan.go +++ b/pkg/leeway/sbom-scan.go @@ -45,6 +45,26 @@ type PackageVulnerabilityStats struct { Ignored int `json:"ignored"` } +type sbomVulnerabilityScanner interface { + Scan(buildctx *buildContext, p *Package, sbomFile string, outputDir string) (*PackageVulnerabilityStats, error) + Close() error +} + +type grypeVulnerabilityScanner struct { + provider vulnerability.Provider + status *vulnerability.ProviderStatus +} + +func (s *grypeVulnerabilityScanner) Scan(buildctx *buildContext, p *Package, sbomFile string, outputDir string) (*PackageVulnerabilityStats, error) { + return scanSBOMForVulnerabilities(buildctx, p, sbomFile, outputDir, s.provider, s.status) +} + +func (s *grypeVulnerabilityScanner) Close() error { + return s.provider.Close() +} + +type vulnerabilityScannerFactory func(buildctx *buildContext, p *Package) (sbomVulnerabilityScanner, error) + // scanAllPackagesForVulnerabilities scans all packages for vulnerabilities. // This function is called after the build process completes to identify security issues // in all built packages, including those loaded from cache. It generates comprehensive @@ -54,6 +74,10 @@ type PackageVulnerabilityStats struct { // This prevents errors when a dependency build fails in a parallel goroutine but the main // build continues (due to the build lock mechanism allowing other goroutines to proceed). func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Package, pkgstatus map[*Package]PackageBuildStatus, customOutputDir ...string) error { + return scanAllPackagesForVulnerabilitiesWithScannerFactory(buildctx, packages, pkgstatus, newGrypeVulnerabilityScanner, customOutputDir...) +} + +func scanAllPackagesForVulnerabilitiesWithScannerFactory(buildctx *buildContext, packages []*Package, pkgstatus map[*Package]PackageBuildStatus, scannerFactory vulnerabilityScannerFactory, customOutputDir ...string) error { if len(packages) == 0 { return nil } @@ -76,6 +100,17 @@ func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Packa return xerrors.Errorf(errMsg) } + var scanner sbomVulnerabilityScanner + var scannerPackage *Package + defer func() { + if scanner == nil { + return + } + if closeErr := scanner.Close(); closeErr != nil { + buildctx.Reporter.PackageBuildLog(scannerPackage, true, []byte("failed to close vulnerability provider: "+closeErr.Error()+"\n")) + } + }() + // Process each package for _, p := range packages { // Skip packages that were not successfully built or downloaded @@ -153,7 +188,19 @@ func scanAllPackagesForVulnerabilities(buildctx *buildContext, packages []*Packa } // Scan for vulnerabilities - stats, err := scanSBOMForVulnerabilities(buildctx, p, sbomFilename, outputDir) + if scanner == nil { + // Scanner construction opens and may update Grype's database. Keep that + // lifecycle outside the package scan so update failures are retried once. + scanner, err = scannerFactory(buildctx, p) + if err != nil { + errMsg := fmt.Sprintf("failed to initialize vulnerability scanner: %s", err) + buildctx.Reporter.PackageBuildLog(p, true, []byte(errMsg+"\n")) + return xerrors.Errorf(errMsg) + } + scannerPackage = p + } + + stats, err := scanner.Scan(buildctx, p, sbomFilename, outputDir) if err != nil { buildctx.Reporter.PackageBuildLog(p, false, fmt.Appendf(nil, "Failed to scan package %s for vulnerabilities: %s\n", p.FullName(), err.Error())) failedPackages = append(failedPackages, p.FullName()) @@ -242,12 +289,8 @@ func ScanAllPackagesForVulnerabilities(localCache cache.LocalCache, packages []* return scanAllPackagesForVulnerabilities(buildctx, packages, pkgstatus, customOutputDir...) } -// scanSBOMForVulnerabilities scans an SBOM file for vulnerabilities and generates reports. -// This function can be called independently of the build process to analyze a specific SBOM file. -// It returns vulnerability statistics for the package and an error if the scan fails. -// The function handles loading the vulnerability database, parsing the SBOM, finding matches, -// and generating reports in multiple formats. -func scanSBOMForVulnerabilities(buildctx *buildContext, p *Package, sbomFile string, outputDir string) (stats *PackageVulnerabilityStats, err error) { +// scanSBOMForVulnerabilities scans an SBOM file using an initialized vulnerability provider. +func scanSBOMForVulnerabilities(buildctx *buildContext, p *Package, sbomFile string, outputDir string, vulnProvider vulnerability.Provider, vulnProviderStatus *vulnerability.ProviderStatus) (stats *PackageVulnerabilityStats, err error) { if !p.C.W.SBOM.Enabled { return nil, xerrors.Errorf("SBOM feature is disabled, cannot scan for vulnerabilities") } @@ -260,19 +303,6 @@ func scanSBOMForVulnerabilities(buildctx *buildContext, p *Package, sbomFile str return nil, xerrors.Errorf(errMsg) } - // Load vulnerability database - vulnProvider, vulnProviderStatus, err := loadVulnerabilityDB(buildctx, p) - if err != nil { - errMsg := fmt.Sprintf("failed to load vulnerability database: %s", err) - buildctx.Reporter.PackageBuildLog(p, true, []byte(errMsg+"\n")) - return nil, xerrors.Errorf(errMsg) - } - defer func() { - if closeErr := vulnProvider.Close(); closeErr != nil { - buildctx.Reporter.PackageBuildLog(p, true, []byte("failed to close vulnerability provider: "+closeErr.Error()+"\n")) - } - }() - buildctx.Reporter.PackageBuildLog(p, false, fmt.Appendf(nil, "Using vulnerability database (path: %s, built on: %s)\n", vulnProviderStatus.Path, vulnProviderStatus.Built.Format("2006-01-02"))) @@ -673,9 +703,62 @@ func WritePackageVulnerabilityMarkdown(outputDir string, stats []*PackageVulnera return nil } +const ( + vulnerabilityDBLoadMaxAttempts = 3 + vulnerabilityDBLoadInitialBackoff = time.Second +) + +type vulnerabilityDBLoadFunc func() (vulnerability.Provider, *vulnerability.ProviderStatus, error) + +type vulnerabilityDBLoader struct { + load vulnerabilityDBLoadFunc + sleep func(time.Duration) + maxAttempts int + initialBackoff time.Duration +} + +func newGrypeVulnerabilityScanner(buildctx *buildContext, p *Package) (sbomVulnerabilityScanner, error) { + loader := vulnerabilityDBLoader{ + load: loadVulnerabilityDB, + sleep: time.Sleep, + maxAttempts: vulnerabilityDBLoadMaxAttempts, + initialBackoff: vulnerabilityDBLoadInitialBackoff, + } + + provider, status, err := loader.Load(buildctx, p) + if err != nil { + return nil, err + } + + return &grypeVulnerabilityScanner{provider: provider, status: status}, nil +} + +func (l vulnerabilityDBLoader) Load(buildctx *buildContext, p *Package) (vulnerability.Provider, *vulnerability.ProviderStatus, error) { + backoff := l.initialBackoff + var lastErr error + + for attempt := 1; attempt <= l.maxAttempts; attempt++ { + buildctx.Reporter.PackageBuildLog(p, false, fmt.Appendf(nil, "Loading vulnerability database (attempt %d/%d) ...\n", attempt, l.maxAttempts)) + + provider, status, err := l.load() + if err == nil { + return provider, status, nil + } + lastErr = err + + if attempt < l.maxAttempts { + buildctx.Reporter.PackageBuildLog(p, true, fmt.Appendf(nil, "Failed to load vulnerability database: %s; retrying in %s\n", err, backoff)) + l.sleep(backoff) + backoff *= 2 + } + } + + return nil, nil, xerrors.Errorf("failed to load vulnerability database after %d attempts: %w", l.maxAttempts, lastErr) +} + // loadVulnerabilityDB initializes and loads the vulnerability database. // It configures the database provider and handles downloading/updating the database if needed. -func loadVulnerabilityDB(buildctx *buildContext, p *Package) (vulnerability.Provider, *vulnerability.ProviderStatus, error) { +func loadVulnerabilityDB() (vulnerability.Provider, *vulnerability.ProviderStatus, error) { distConfig := distribution.DefaultConfig() id := clio.Identification{ @@ -685,8 +768,6 @@ func loadVulnerabilityDB(buildctx *buildContext, p *Package) (vulnerability.Prov installConfig := installation.DefaultConfig(id) - buildctx.Reporter.PackageBuildLog(p, false, []byte("Loading vulnerability database (this may take a moment on first run) ...\n")) - provider, status, err := grype.LoadVulnerabilityDB(distConfig, installConfig, true) if err != nil { return nil, nil, xerrors.Errorf("failed to load vulnerability database: %w", err) diff --git a/pkg/leeway/sbom_scan_test.go b/pkg/leeway/sbom_scan_test.go new file mode 100644 index 00000000..5180c4f6 --- /dev/null +++ b/pkg/leeway/sbom_scan_test.go @@ -0,0 +1,247 @@ +package leeway + +import ( + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/anchore/grype/grype/vulnerability" + "github.com/google/go-cmp/cmp" + + leewaycache "github.com/gitpod-io/leeway/pkg/leeway/cache" +) + +func TestVulnerabilityDBLoaderLoad(t *testing.T) { + t.Parallel() + + type Expectation struct { + Calls int + Sleeps []time.Duration + Err string + } + + tests := []struct { + Name string + LoadErrors []string + Expected Expectation + }{ + { + Name: "loads_on_first_attempt", + LoadErrors: []string{""}, + Expected: Expectation{ + Calls: 1, + }, + }, + { + Name: "retries_with_bounded_backoff", + LoadErrors: []string{"temporary failure", "temporary failure", ""}, + Expected: Expectation{ + Calls: 3, + Sleeps: []time.Duration{time.Second, 2 * time.Second}, + }, + }, + { + Name: "returns_last_error_after_max_attempts", + LoadErrors: []string{"first failure", "second failure", "final failure"}, + Expected: Expectation{ + Calls: 3, + Sleeps: []time.Duration{time.Second, 2 * time.Second}, + Err: "failed to load vulnerability database after 3 attempts: final failure", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + var got Expectation + loader := vulnerabilityDBLoader{ + maxAttempts: 3, + initialBackoff: time.Second, + sleep: func(delay time.Duration) { + got.Sleeps = append(got.Sleeps, delay) + }, + load: func() (vulnerability.Provider, *vulnerability.ProviderStatus, error) { + loadErr := tc.LoadErrors[got.Calls] + got.Calls++ + if loadErr != "" { + return nil, nil, errors.New(loadErr) + } + return nil, &vulnerability.ProviderStatus{}, nil + }, + } + + buildctx := &buildContext{buildOptions: buildOptions{Reporter: &NoopReporter{}}} + _, _, err := loader.Load(buildctx, NewTestPackage("scan")) + if err != nil { + got.Err = err.Error() + } + + if diff := cmp.Diff(tc.Expected, got); diff != "" { + t.Errorf("vulnerabilityDBLoader.Load() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestScanAllPackagesForVulnerabilitiesReusesScanner(t *testing.T) { + t.Parallel() + + type Expectation struct { + FactoryCalls int + ScannedPackages []string + CloseCalls int + CloseErrorLogged bool + Err string + } + + tests := []struct { + Name string + PackageStatus PackageBuildStatus + FactoryErr string + ScanErrPackage string + CloseErr string + Expected Expectation + }{ + { + Name: "reuses_and_closes_one_scanner", + Expected: Expectation{ + FactoryCalls: 1, + ScannedPackages: []string{"testcomp:first", "testcomp:second"}, + CloseCalls: 1, + }, + }, + { + Name: "skips_scanner_without_scannable_packages", + PackageStatus: PackageNotBuiltYet, + Expected: Expectation{}, + }, + { + Name: "stops_when_scanner_initialization_fails", + FactoryErr: "database unavailable", + Expected: Expectation{ + FactoryCalls: 1, + Err: "failed to initialize vulnerability scanner: database unavailable", + }, + }, + { + Name: "keeps_scanning_after_package_failure", + ScanErrPackage: "testcomp:first", + Expected: Expectation{ + FactoryCalls: 1, + ScannedPackages: []string{"testcomp:first", "testcomp:second"}, + CloseCalls: 1, + Err: "vulnerability scan failed for packages: testcomp:first", + }, + }, + { + Name: "logs_scanner_close_failure", + CloseErr: "close failure", + Expected: Expectation{ + FactoryCalls: 1, + ScannedPackages: []string{"testcomp:first", "testcomp:second"}, + CloseCalls: 1, + CloseErrorLogged: true, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() + + var got Expectation + reporter := &sbomScanRecordingReporter{} + buildctx := &buildContext{buildOptions: buildOptions{ + Reporter: reporter, + LocalCache: sbomScanTestCache{}, + }} + + packages := []*Package{NewTestPackage("first"), NewTestPackage("second")} + pkgstatus := make(map[*Package]PackageBuildStatus, len(packages)) + packageStatus := tc.PackageStatus + if packageStatus == "" { + packageStatus = PackageBuilt + } + for _, pkg := range packages { + pkg.C.W.SBOM.Enabled = true + artifactPath := filepath.Join(t.TempDir(), pkg.FilesystemSafeName()+".tar.gz") + if err := os.WriteFile(artifactPath+".sbom.cdx.json", []byte("{}"), 0644); err != nil { + got.Err = "test setup: " + err.Error() + break + } + buildctx.LocalCache.(sbomScanTestCache)[pkg.FullName()] = artifactPath + pkgstatus[pkg] = packageStatus + } + + scanner := &fakeSBOMVulnerabilityScanner{ + scanErrPackage: tc.ScanErrPackage, + closeErr: tc.CloseErr, + } + factory := func(_ *buildContext, _ *Package) (sbomVulnerabilityScanner, error) { + got.FactoryCalls++ + if tc.FactoryErr != "" { + return nil, errors.New(tc.FactoryErr) + } + return scanner, nil + } + + if got.Err == "" { + err := scanAllPackagesForVulnerabilitiesWithScannerFactory(buildctx, packages, pkgstatus, factory, t.TempDir()) + if err != nil { + got.Err = err.Error() + } + } + got.ScannedPackages = scanner.scannedPackages + got.CloseCalls = scanner.closeCalls + got.CloseErrorLogged = strings.Contains(strings.Join(reporter.logs, ""), tc.CloseErr) && tc.CloseErr != "" + + if diff := cmp.Diff(tc.Expected, got); diff != "" { + t.Errorf("scanAllPackagesForVulnerabilitiesWithScannerFactory() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +type fakeSBOMVulnerabilityScanner struct { + scannedPackages []string + scanErrPackage string + closeErr string + closeCalls int +} + +func (s *fakeSBOMVulnerabilityScanner) Scan(_ *buildContext, p *Package, _ string, _ string) (*PackageVulnerabilityStats, error) { + s.scannedPackages = append(s.scannedPackages, p.FullName()) + if p.FullName() == s.scanErrPackage { + return nil, errors.New("scan failure") + } + return &PackageVulnerabilityStats{Name: p.FullName()}, nil +} + +func (s *fakeSBOMVulnerabilityScanner) Close() error { + s.closeCalls++ + if s.closeErr != "" { + return errors.New(s.closeErr) + } + return nil +} + +type sbomScanTestCache map[string]string + +func (c sbomScanTestCache) Location(pkg leewaycache.Package) (string, bool) { + path, ok := c[pkg.FullName()] + return path, ok +} + +type sbomScanRecordingReporter struct { + NoopReporter + logs []string +} + +func (r *sbomScanRecordingReporter) PackageBuildLog(_ *Package, _ bool, buf []byte) { + r.logs = append(r.logs, string(buf)) +}