scanner: measure grype DB freshness from its build time

Freshness was measured from load time, so a load that fell back to a
stale-but-valid on-disk DB earned a fresh cache lease and could ride past
Grype's MaxAllowedBuiltAge cliff. Measure from the DB's own build
timestamp instead, and throttle reload attempts with a 30m backoff so a
down upstream doesn't make every worker pay its own download timeout.

Two locking fixes come with it, both reachable only once the DB is stale
and so newly relevant now that staleness is tracked honestly:

  - FindMatches ran on a provider fetched outside the lock while a reload
    could Close() it under the write lock. The scan now holds the read
    lock across matching and reads the provider under it, so a reload
    waits for in-flight scans instead of closing a store mid-scan.
  - The retry backoff is also tested on the read-lock fast path. Once the
    DB is stale the freshness test never passes again, so every scan was
    taking the exclusive lock purely to reach the backoff return.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

scanner/internal/scan/grype.go

@@ -32,22 +32,30 @@ import (
 
 // Global vulnerability database (shared across workers)
 var (
-	vulnDB       vulnerability.Provider
-	vulnDBLock   sync.RWMutex
-	vulnDBLoaded time.Time      // when the current vulnDB was loaded
-	vulnDBScans  atomic.Int64   // scan counter for periodic reload
+	vulnDB        vulnerability.Provider
+	vulnDBLock    sync.RWMutex
+	vulnDBBuilt   time.Time    // build timestamp of the current vulnDB (not load time)
+	vulnDBAttempt time.Time    // last time we attempted a (re)load, success or fail
+	vulnDBScans   atomic.Int64 // scan counter for periodic reload
 )
 
-// vulnDBRefreshAge is how long a cached DB is considered fresh.
-// Set 1 day before the 5-day MaxAllowedBuiltAge so we refresh proactively.
+// vulnDBRefreshAge is how long a DB is considered fresh, measured from its build
+// timestamp (not from when we loaded it). Once the build age crosses this, every
+// scan re-attempts an update so we always refresh well before Grype's 14-day
+// MaxAllowedBuiltAge cliff, instead of riding a fixed cache lease past expiry.
 const vulnDBRefreshAge = 7 * 24 * time.Hour
 
+// vulnDBRetryBackoff throttles reload attempts once the DB is stale. When the
+// upstream is unreachable, the first worker probes and the rest keep scanning on
+// the still-usable (in-memory) provider until the backoff elapses, instead of
+// every worker serializing through its own download timeout.
+const vulnDBRetryBackoff = 30 * time.Minute
+
 // scanVulnerabilities scans an SBOM for vulnerabilities using Grype
 func scanVulnerabilities(ctx context.Context, s *sbom.SBOM, vulnDBPath string) ([]byte, string, scanner.VulnerabilitySummary, error) {
 	slog.Info("Scanning for vulnerabilities with Grype")
 
-	store, err := loadVulnDatabase(ctx, vulnDBPath)
-	if err != nil {
+	if _, err := loadVulnDatabase(ctx, vulnDBPath); err != nil {
 		return nil, "", scanner.VulnerabilitySummary{}, fmt.Errorf("failed to load vulnerability database: %w", err)
 	}
 
@@ -87,8 +95,18 @@ func scanVulnerabilities(ctx context.Context, s *sbom.SBOM, vulnDBPath string) (
 		Distro: grypeDistro,
 	}
 
+	// Hold the read lock across matching. A reload closes the previous provider
+	// (here and on the periodic flush) under the write lock, so a provider read
+	// before the lock could be closed out from under FindMatches mid-scan. Taking
+	// the lock here and reading vulnDB under it means a reload waits for in-flight
+	// scans to finish, and this scan always runs against a live store. Reloads are
+	// rare and Go's RWMutex blocks new readers once a writer is queued, so a
+	// pending reload cannot be starved by a steady stream of scans.
+	vulnDBLock.RLock()
+	defer vulnDBLock.RUnlock()
+
 	vulnerabilityMatcher := &grype.VulnerabilityMatcher{
-		VulnerabilityProvider: store,
+		VulnerabilityProvider: vulnDB,
 		Matchers:              matchers,
 		NormalizeByCVE:        true,
 	}
@@ -148,7 +166,12 @@ func grypeDBConfig(vulnDBPath string) (distribution.Config, installation.Config)
 // there is no chance of a double-curator update+load seeing different state.
 func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Provider, error) {
 	vulnDBLock.RLock()
-	if vulnDB != nil && time.Since(vulnDBLoaded) < vulnDBRefreshAge {
+	// Fresh, or stale but inside the retry backoff: either way this provider is
+	// what the scan will use, so take the shared lock only. Testing the backoff
+	// here as well as under the write lock matters — once the DB is stale the
+	// freshness test never passes again, and without this every scan would
+	// serialize through the exclusive lock just to reach the same conclusion.
+	if vulnDB != nil && (time.Since(vulnDBBuilt) < vulnDBRefreshAge || time.Since(vulnDBAttempt) < vulnDBRetryBackoff) {
 		vulnDBLock.RUnlock()
 		return vulnDB, nil
 	}
@@ -157,8 +180,10 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro
 	vulnDBLock.Lock()
 	defer vulnDBLock.Unlock()
 
-	// Double-check after acquiring write lock
-	if vulnDB != nil && time.Since(vulnDBLoaded) < vulnDBRefreshAge {
+	// Double-check after acquiring write lock. Freshness is measured from the
+	// DB's build timestamp, so a load that fell back to a stale-but-valid DB
+	// doesn't earn a fresh cache lease — it stays stale and keeps retrying.
+	if vulnDB != nil && time.Since(vulnDBBuilt) < vulnDBRefreshAge {
 		// Periodic reload: close and reopen DB every 50 scans to flush
 		// SQLite's page cache and mmap region.
 		n := vulnDBScans.Add(1)
@@ -172,6 +197,15 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro
 		}
 	}
 
+	// DB is stale (or absent). Throttle reload attempts: if we probed recently
+	// and still hold a usable provider, keep serving it rather than having every
+	// worker serialize through its own upstream timeout. The in-memory provider
+	// doesn't re-validate build age on queries, so a stale-but-loaded DB still
+	// scans fine until the upstream recovers.
+	if vulnDB != nil && time.Since(vulnDBAttempt) < vulnDBRetryBackoff {
+		return vulnDB, nil
+	}
+
 	slog.Info("Loading Grype vulnerability database", "path", vulnDBPath, "tmpdir", os.Getenv("TMPDIR"))
 
 	if err := os.MkdirAll(vulnDBPath, 0o755); err != nil {
@@ -184,8 +218,19 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro
 	// if needed, activates, and then opens the reader — all in one call. If
 	// 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)
 	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
+		// backoff elapses; only surface the error on a cold start with no DB.
+		if vulnDB != nil {
+			slog.Warn("Vulnerability database reload failed; serving previously loaded DB",
+				"error", err,
+				"built", vulnDBBuilt,
+				"age", time.Since(vulnDBBuilt).Round(time.Minute).String())
+			return vulnDB, nil
+		}
 		return nil, fmt.Errorf("failed to load vulnerability database: %w", err)
 	}
 
@@ -203,7 +248,7 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro
 		vulnDB.Close()
 	}
 	vulnDB = store
-	vulnDBLoaded = time.Now()
+	vulnDBBuilt = status.Built
 	return vulnDB, nil
 }