diff --git a/scanner/internal/scan/grype.go b/scanner/internal/scan/grype.go index 2eb4c26..8df8d14 100644 --- a/scanner/internal/scan/grype.go +++ b/scanner/internal/scan/grype.go @@ -159,6 +159,13 @@ func grypeDBConfig(vulnDBPath string) (distribution.Config, installation.Config) } } +// loadVulnDB is the Grype loader, indirected so tests can drive the freshness +// and fallback logic without a network fetch or a real database on disk. The +// whole point of this file is what happens when that call returns a stale DB or +// fails outright, and neither is reachable from a test that has to perform a +// real download. +var loadVulnDB = grype.LoadVulnerabilityDB + // loadVulnDatabase loads the Grype vulnerability database with caching and // automatic refresh. The cached DB is returned if loaded less than // vulnDBRefreshAge ago. On a stale or missing DB, Grype downloads a fresh copy @@ -219,7 +226,7 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro // the upstream is unreachable but the on-disk DB is still valid, it falls // back to serving the existing DB. vulnDBAttempt = time.Now() - store, status, err := grype.LoadVulnerabilityDB(distConfig, installConfig, true) + store, status, err := loadVulnDB(distConfig, installConfig, true) if err != nil { // Reload failed (e.g. upstream down and on-disk DB past the 14-day max // age). If we still hold a usable provider, keep serving it until the diff --git a/scanner/internal/scan/grype_test.go b/scanner/internal/scan/grype_test.go new file mode 100644 index 0000000..cf4bccf --- /dev/null +++ b/scanner/internal/scan/grype_test.go @@ -0,0 +1,243 @@ +package scan + +import ( + "context" + "errors" + "testing" + "time" + + v6dist "github.com/anchore/grype/grype/db/v6/distribution" + v6inst "github.com/anchore/grype/grype/db/v6/installation" + "github.com/anchore/grype/grype/vulnerability" +) + +// fakeProvider stands in for a loaded vulnerability database. Nothing here +// queries it; the tests only care which provider comes back and whether the +// previous one was closed. +type fakeProvider struct { + vulnerability.Provider + name string + closed bool +} + +func (f *fakeProvider) Close() error { + f.closed = true + return nil +} + +// resetVulnDBState clears the package-level database state. These globals are +// shared by every worker in the real scanner, so tests must not run in parallel +// and must not inherit each other's state. +func resetVulnDBState(t *testing.T) { + t.Helper() + original := loadVulnDB + t.Cleanup(func() { + vulnDBLock.Lock() + vulnDB, vulnDBBuilt, vulnDBAttempt = nil, time.Time{}, time.Time{} + vulnDBLock.Unlock() + vulnDBScans.Store(0) + loadVulnDB = original + }) + + vulnDBLock.Lock() + vulnDB, vulnDBBuilt, vulnDBAttempt = nil, time.Time{}, time.Time{} + vulnDBLock.Unlock() + vulnDBScans.Store(0) +} + +// stubLoader installs a fake loader and counts how many times it was called. +// Reaching it at all is significant: every call is a real download attempt in +// production, so the call count is the thing several of these tests assert on. +func stubLoader(t *testing.T, built time.Time, err error) (*fakeProvider, *int) { + t.Helper() + loaded := &fakeProvider{name: "loaded"} + calls := 0 + loadVulnDB = func(v6dist.Config, v6inst.Config, bool) (vulnerability.Provider, *vulnerability.ProviderStatus, error) { + calls++ + if err != nil { + return nil, nil, err + } + return loaded, &vulnerability.ProviderStatus{Built: built}, nil + } + return loaded, &calls +} + +// TestLoadVulnDatabase_FreshnessComesFromBuildTime is the defect fa1dfb0 fixed, +// stated as a test. Freshness used to be measured from load time, so a load that +// fell back to a stale-but-valid on-disk DB started a fresh 7-day lease and +// could ride past Grype's MaxAllowedBuiltAge. Past that cliff the scanner +// reports no vulnerabilities rather than an error, which is the worst possible +// way for a security tool to fail. +// +// Loading a DB built 10 days ago must therefore leave it stale, not fresh. +func TestLoadVulnDatabase_FreshnessComesFromBuildTime(t *testing.T) { + resetVulnDBState(t) + + staleBuild := time.Now().Add(-10 * 24 * time.Hour) + _, calls := stubLoader(t, staleBuild, nil) + + if _, err := loadVulnDatabase(context.Background(), t.TempDir()); err != nil { + t.Fatalf("loadVulnDatabase: %v", err) + } + if *calls != 1 { + t.Fatalf("loader called %d times, want 1", *calls) + } + + if !vulnDBBuilt.Equal(staleBuild) { + t.Errorf("vulnDBBuilt = %v, want the DB's own build time %v: recording load time here "+ + "is what let an expired database look fresh for another 7 days", vulnDBBuilt, staleBuild) + } + if time.Since(vulnDBBuilt) < vulnDBRefreshAge { + t.Error("a database built 10 days ago is being treated as fresh") + } +} + +// TestLoadVulnDatabase_FreshDBIsServedWithoutReloading pins the ordinary case: +// a recently built database is reused, and no download is attempted. +func TestLoadVulnDatabase_FreshDBIsServedWithoutReloading(t *testing.T) { + resetVulnDBState(t) + + cached := &fakeProvider{name: "cached"} + vulnDB = cached + vulnDBBuilt = time.Now().Add(-1 * time.Hour) + + _, calls := stubLoader(t, time.Now(), nil) + + got, err := loadVulnDatabase(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("loadVulnDatabase: %v", err) + } + if got != cached { + t.Error("a freshly built database was replaced instead of reused") + } + if *calls != 0 { + t.Errorf("loader called %d times for a fresh DB, want 0", *calls) + } + if cached.closed { + t.Error("the in-use provider was closed") + } +} + +// TestLoadVulnDatabase_StaleDBIsThrottled covers the backoff. Once the DB is +// stale the freshness test can never pass again, so without a throttle every +// worker on every scan would serialize through its own upstream download +// timeout. Inside the backoff window the stale-but-loaded provider keeps +// serving, untouched. +// +// What this pins, precisely: the behaviour, not either check that implements +// it. fa1dfb0 tests the backoff twice, once on the read-lock fast path and +// again under the write lock, and mutation confirms they are redundant for +// correctness — deleting either one alone leaves this test passing, and only +// deleting both fails it. That redundancy is deliberate. The fast-path copy +// exists so a stale DB does not push every scan through the exclusive lock, a +// contention property no unit test can assert without being flaky. +func TestLoadVulnDatabase_StaleDBIsThrottled(t *testing.T) { + resetVulnDBState(t) + + cached := &fakeProvider{name: "cached"} + vulnDB = cached + vulnDBBuilt = time.Now().Add(-10 * 24 * time.Hour) // stale + vulnDBAttempt = time.Now().Add(-5 * time.Minute) // probed recently + + _, calls := stubLoader(t, time.Now(), nil) + + got, err := loadVulnDatabase(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("loadVulnDatabase: %v", err) + } + if got != cached { + t.Error("the stale provider was replaced during the backoff window") + } + if *calls != 0 { + t.Errorf("loader called %d times inside the retry backoff, want 0", *calls) + } +} + +// TestLoadVulnDatabase_RetriesOnceTheBackoffElapses is the other half of the +// throttle: it has to expire. A backoff that never lets go would pin the +// scanner to a stale database indefinitely, which is the same silent failure +// the build-time fix exists to prevent. +func TestLoadVulnDatabase_RetriesOnceTheBackoffElapses(t *testing.T) { + resetVulnDBState(t) + + old := &fakeProvider{name: "old"} + vulnDB = old + vulnDBBuilt = time.Now().Add(-10 * 24 * time.Hour) + vulnDBAttempt = time.Now().Add(-vulnDBRetryBackoff - time.Minute) + + freshBuild := time.Now().Add(-1 * time.Hour) + loaded, calls := stubLoader(t, freshBuild, nil) + + got, err := loadVulnDatabase(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("loadVulnDatabase: %v", err) + } + if *calls != 1 { + t.Fatalf("loader called %d times after the backoff elapsed, want 1", *calls) + } + if got != loaded { + t.Error("the newly loaded provider was not adopted") + } + if !old.closed { + t.Error("the replaced provider was not closed: every reload leaks its predecessor") + } + if !vulnDBBuilt.Equal(freshBuild) { + t.Errorf("vulnDBBuilt = %v, want %v", vulnDBBuilt, freshBuild) + } +} + +// TestLoadVulnDatabase_FailedReloadKeepsServingTheOldDB covers an upstream +// outage. A loaded provider does not re-validate build age on queries, so it +// keeps working; refusing to scan because the update failed would be a worse +// outcome than scanning against a database a few days old. +// +// The attempt timestamp must still advance, or the backoff never engages and +// every subsequent scan pays the failed download again. +func TestLoadVulnDatabase_FailedReloadKeepsServingTheOldDB(t *testing.T) { + resetVulnDBState(t) + + old := &fakeProvider{name: "old"} + vulnDB = old + vulnDBBuilt = time.Now().Add(-10 * 24 * time.Hour) + vulnDBAttempt = time.Now().Add(-vulnDBRetryBackoff - time.Minute) + + _, calls := stubLoader(t, time.Time{}, errors.New("upstream unreachable")) + + got, err := loadVulnDatabase(context.Background(), t.TempDir()) + if err != nil { + t.Fatalf("a failed reload with a usable DB in hand must not fail the scan: %v", err) + } + if got != old { + t.Error("the previously loaded provider was not served after the reload failed") + } + if old.closed { + t.Error("the provider still in use was closed") + } + if *calls != 1 { + t.Errorf("loader called %d times, want 1", *calls) + } + if time.Since(vulnDBAttempt) > time.Minute { + t.Error("vulnDBAttempt was not advanced by the failed attempt, so the backoff " + + "never engages and every scan repeats the download") + } +} + +// TestLoadVulnDatabase_ColdStartFailureIsAnError is the one case that must fail +// loudly. With no provider in hand there is nothing to scan against, and +// returning success would report every image as clean. +func TestLoadVulnDatabase_ColdStartFailureIsAnError(t *testing.T) { + resetVulnDBState(t) + + _, calls := stubLoader(t, time.Time{}, errors.New("upstream unreachable")) + + got, err := loadVulnDatabase(context.Background(), t.TempDir()) + if err == nil { + t.Fatal("a cold start with no database returned success: every scan would find nothing") + } + if got != nil { + t.Error("a provider was returned alongside the error") + } + if *calls != 1 { + t.Errorf("loader called %d times, want 1", *calls) + } +}