From 76383ec764feaaff8cd0224c007c72701b498f45 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Thu, 19 Feb 2026 22:08:02 -0600 Subject: [PATCH] fix vuln scanner db not refreshing --- pkg/appview/handlers/scan_result.go | 7 +++ pkg/appview/handlers/scan_result_test.go | 7 +++ .../templates/partials/vuln-badge.html | 2 + pkg/hold/admin/handlers_crew.go | 60 ++++++++++++------ .../admin/templates/partials/tab_crew.html | 9 ++- pkg/hold/pds/scan_broadcaster.go | 7 ++- scanner/internal/scan/grype.go | 62 +++++++++++++------ scanner/internal/scan/worker.go | 18 +++++- 8 files changed, 126 insertions(+), 46 deletions(-) diff --git a/pkg/appview/handlers/scan_result.go b/pkg/appview/handlers/scan_result.go index 32ddc5b..1ac52aa 100644 --- a/pkg/appview/handlers/scan_result.go +++ b/pkg/appview/handlers/scan_result.go @@ -34,6 +34,7 @@ type vulnBadgeData struct { ScannedAt string Found bool // true if scan record exists Error bool // true if hold unreachable or error + ScanFailed bool // true if scan record exists but scan failed (no blobs) Digest string // for the detail modal link HoldEndpoint string // for the detail modal link } @@ -127,6 +128,10 @@ func (h *ScanResultHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } + // A failed scan has nil blobs (no SBOM generated) and zero counts. + // Successful scans always have an SBOM blob even with 0 vulnerabilities. + scanFailed := scanRecord.SbomBlob == nil && scanRecord.Total == 0 + h.renderBadge(w, vulnBadgeData{ Critical: scanRecord.Critical, High: scanRecord.High, @@ -135,6 +140,7 @@ func (h *ScanResultHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { Total: scanRecord.Total, ScannedAt: scanRecord.ScannedAt, Found: true, + ScanFailed: scanFailed, Digest: digest, HoldEndpoint: holdDID, }) @@ -194,6 +200,7 @@ func fetchScanRecord(ctx context.Context, holdEndpoint, holdDID, hexDigest strin Total: scanRecord.Total, ScannedAt: scanRecord.ScannedAt, Found: true, + ScanFailed: scanRecord.SbomBlob == nil && scanRecord.Total == 0, Digest: fullDigest, HoldEndpoint: holdDID, } diff --git a/pkg/appview/handlers/scan_result_test.go b/pkg/appview/handlers/scan_result_test.go index 96a33b1..6fde643 100644 --- a/pkg/appview/handlers/scan_result_test.go +++ b/pkg/appview/handlers/scan_result_test.go @@ -38,6 +38,13 @@ func mockScanRecord(critical, high, medium, low, total int64) string { "total": total, "scannerVersion": "atcr-scanner-v1.0.0", "scannedAt": "2025-01-15T10:30:00Z", + // Successful scans always have an SBOM blob + "sbomBlob": map[string]any{ + "$type": "blob", + "ref": map[string]any{"$link": "bafkreigv3xw47pk7cbeahkmttetf4smxyluwlu3jmteo2nzke2oa7dbhhm"}, + "mimeType": "application/spdx+json", + "size": 1234, + }, } envelope := map[string]any{ "uri": "at://did:web:hold.example.com/io.atcr.hold.scan/abc123", diff --git a/pkg/appview/templates/partials/vuln-badge.html b/pkg/appview/templates/partials/vuln-badge.html index 9a77683..060ac71 100644 --- a/pkg/appview/templates/partials/vuln-badge.html +++ b/pkg/appview/templates/partials/vuln-badge.html @@ -1,6 +1,8 @@ {{ define "vuln-badge" }} {{ if .Error }} {{/* Silently hide on error / no scan record — scan badges are non-critical */}} +{{ else if .ScanFailed }} +{{/* Scan failed (no SBOM blob) — don't show misleading "Clean" badge */}} {{ else if eq .Total 0 }} {{ icon "shield-check" "size-3" }} Clean {{ else }} diff --git a/pkg/hold/admin/handlers_crew.go b/pkg/hold/admin/handlers_crew.go index 9598637..b6333bb 100644 --- a/pkg/hold/admin/handlers_crew.go +++ b/pkg/hold/admin/handlers_crew.go @@ -9,6 +9,7 @@ import ( "time" "atcr.io/pkg/atproto" + "atcr.io/pkg/hold/pds" "github.com/go-chi/chi/v5" ) @@ -27,17 +28,6 @@ type CrewMemberView struct { AddedAt time.Time } -// CrewSkeletonView is the minimal crew member data for skeleton rendering. -// Contains only data available from the MST walk (no network calls). -type CrewSkeletonView struct { - RKey string - DID string - Role string - Permissions []string - Tier string - AddedAt time.Time -} - // resolveHandle attempts to resolve a DID to a handle // Returns empty string if resolution fails func resolveHandle(ctx context.Context, did string) string { @@ -61,8 +51,8 @@ type TierOption struct { } // handleCrewTab returns the crew tab content (HTMX partial). -// Only does the MST walk — no handle resolution or usage queries. -// Each row lazy-loads its details via handleCrewMemberInfo. +// Includes usage data (fast bulk SQL query) for correct sort order. +// Handles are lazy-loaded per-row via handleCrewMemberInfo. func (ui *AdminUI) handleCrewTab(w http.ResponseWriter, r *http.Request) { crew, err := ui.pds.ListCrewMembers(r.Context()) if err != nil { @@ -70,35 +60,65 @@ func (ui *AdminUI) handleCrewTab(w http.ResponseWriter, r *http.Request) { return } + allQuotas, err := ui.pds.GetAllUserQuotas(r.Context()) + if err != nil { + slog.Warn("Failed to get user quotas for crew tab", "error", err) + allQuotas = make(map[string]*pds.QuotaStats) + } + defaultTier := "default" if ui.quotaMgr != nil && ui.quotaMgr.IsEnabled() { defaultTier = ui.quotaMgr.GetDefaultTier() } - var skeletons []CrewSkeletonView + var crewViews []CrewMemberView for _, member := range crew { tier := member.Record.Tier if tier == "" { tier = defaultTier } - skeletons = append(skeletons, CrewSkeletonView{ + + view := CrewMemberView{ RKey: member.Rkey, DID: member.Record.Member, Role: member.Record.Role, Permissions: member.Record.Permissions, Tier: tier, AddedAt: parseTime(member.Record.AddedAt), - }) + } + + usage := int64(0) + if q, ok := allQuotas[member.Record.Member]; ok { + usage = q.TotalSize + } + + if ui.quotaMgr != nil && ui.quotaMgr.IsEnabled() { + if limit := ui.quotaMgr.GetTierLimit(tier); limit != nil { + view.TierLimit = formatHumanBytes(*limit) + if *limit > 0 { + view.UsagePercent = int(float64(usage) / float64(*limit) * 100) + } + } else { + view.TierLimit = "Unlimited" + } + } else { + view.TierLimit = "Unlimited" + } + + view.CurrentUsage = usage + view.UsageHuman = formatHumanBytes(usage) + + crewViews = append(crewViews, view) } - sort.Slice(skeletons, func(i, j int) bool { - return skeletons[i].AddedAt.After(skeletons[j].AddedAt) + sort.Slice(crewViews, func(i, j int) bool { + return crewViews[i].CurrentUsage > crewViews[j].CurrentUsage }) data := struct { - Crew []CrewSkeletonView + Crew []CrewMemberView }{ - Crew: skeletons, + Crew: crewViews, } ui.renderTemplate(w, "partials/tab_crew.html", data) } diff --git a/pkg/hold/admin/templates/partials/tab_crew.html b/pkg/hold/admin/templates/partials/tab_crew.html index 67f2caf..a3a4a73 100644 --- a/pkg/hold/admin/templates/partials/tab_crew.html +++ b/pkg/hold/admin/templates/partials/tab_crew.html @@ -55,9 +55,14 @@ {{.Tier}} +
{{.TierLimit}} - - + +
+ {{.UsageHuman}} + + {{.UsagePercent}}% +
{{formatTime .AddedAt}} diff --git a/pkg/hold/pds/scan_broadcaster.go b/pkg/hold/pds/scan_broadcaster.go index 9d536ed..5850fa1 100644 --- a/pkg/hold/pds/scan_broadcaster.go +++ b/pkg/hold/pds/scan_broadcaster.go @@ -567,12 +567,13 @@ func (sb *ScanBroadcaster) handleError(sub *ScanSubscriber, msg ScannerMessage) "seq", msg.Seq, "error", err) } else { // Create a scan record with zero counts and nil blobs — marks it as - // "scanned" so the proactive scheduler won't retry until rescan interval + // "scanned" so the proactive scheduler won't retry until rescan interval. + // Nil blobs signal failure to the appview (successful scans always have blobs). scanRecord := atproto.NewScanRecord( manifestDigest, repository, userDID, - nil, nil, // no SBOM or vuln report + nil, nil, // no SBOM or vuln report — signals scan failure 0, 0, 0, 0, 0, - "failed: "+truncateError(msg.Error, 200), + "atcr-scanner-v1.0.0", ) if _, _, err := sb.pds.CreateScanRecord(ctx, scanRecord); err != nil { slog.Error("Failed to store failure scan record", diff --git a/scanner/internal/scan/grype.go b/scanner/internal/scan/grype.go index aa010af..df89a2e 100644 --- a/scanner/internal/scan/grype.go +++ b/scanner/internal/scan/grype.go @@ -32,10 +32,15 @@ import ( // Global vulnerability database (shared across workers) var ( - vulnDB vulnerability.Provider - vulnDBLock sync.RWMutex + vulnDB vulnerability.Provider + vulnDBLock sync.RWMutex + vulnDBLoaded time.Time // when the current vulnDB was loaded ) +// vulnDBRefreshAge is how long a cached DB is considered fresh. +// Set 1 day before the 5-day MaxAllowedBuiltAge so we refresh proactively. +const vulnDBRefreshAge = 4 * 24 * time.Hour + // 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") @@ -119,10 +124,12 @@ func scanVulnerabilities(ctx context.Context, s *sbom.SBOM, vulnDBPath string) ( return reportJSON, digest, summary, nil } -// loadVulnDatabase loads the Grype vulnerability database (with caching) +// 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, it downloads a fresh copy. func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Provider, error) { vulnDBLock.RLock() - if vulnDB != nil { + if vulnDB != nil && time.Since(vulnDBLoaded) < vulnDBRefreshAge { vulnDBLock.RUnlock() return vulnDB, nil } @@ -131,7 +138,8 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro vulnDBLock.Lock() defer vulnDBLock.Unlock() - if vulnDB != nil { + // Double-check after acquiring write lock + if vulnDB != nil && time.Since(vulnDBLoaded) < vulnDBRefreshAge { return vulnDB, nil } @@ -149,9 +157,21 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro MaxAllowedBuiltAge: 5 * 24 * time.Hour, // 5 days } + // Try loading existing DB first (no network) store, status, err := grype.LoadVulnerabilityDB(distConfig, installConfig, false) if err != nil { - return nil, fmt.Errorf("failed to load vulnerability database (status=%v): %w", status, err) + slog.Warn("Vulnerability database load failed, attempting update", "error", err) + + // Download fresh DB + if updateErr := updateVulnDatabase(vulnDBPath); updateErr != nil { + return nil, fmt.Errorf("failed to update vulnerability database: %w (original: %w)", updateErr, err) + } + + // Retry loading after update + store, status, err = grype.LoadVulnerabilityDB(distConfig, installConfig, false) + if err != nil { + return nil, fmt.Errorf("failed to load vulnerability database after update (status=%v): %w", status, err) + } } slog.Info("Vulnerability database loaded", @@ -159,17 +179,14 @@ func loadVulnDatabase(ctx context.Context, vulnDBPath string) (vulnerability.Pro "schemaVersion", status.SchemaVersion) vulnDB = store + vulnDBLoaded = time.Now() return vulnDB, nil } -// initializeVulnDatabase downloads the vulnerability database on startup +// initializeVulnDatabase ensures a fresh vulnerability database exists on startup. func initializeVulnDatabase(vulnDBPath, tmpDir string) error { slog.Info("Initializing vulnerability database", "path", vulnDBPath) - if err := os.MkdirAll(vulnDBPath, 0755); err != nil { - return fmt.Errorf("failed to create database directory: %w", err) - } - grpeTmpDir := filepath.Join(tmpDir, "grype-dl") if err := os.MkdirAll(grpeTmpDir, 0755); err != nil { return fmt.Errorf("failed to create temp directory: %w", err) @@ -185,6 +202,17 @@ func initializeVulnDatabase(vulnDBPath, tmpDir string) error { } }() + return updateVulnDatabase(vulnDBPath) +} + +// updateVulnDatabase downloads a fresh vulnerability database if needed. +// The curator internally checks whether an update is necessary (DB missing, +// stale, or update-check cooldown expired) so this is safe to call often. +func updateVulnDatabase(vulnDBPath string) error { + if err := os.MkdirAll(vulnDBPath, 0755); err != nil { + return fmt.Errorf("failed to create database directory: %w", err) + } + distConfig := distribution.DefaultConfig() installConfig := installation.Config{ DBRootDir: vulnDBPath, @@ -203,20 +231,14 @@ func initializeVulnDatabase(vulnDBPath, tmpDir string) error { return fmt.Errorf("failed to create database curator: %w", err) } - status := curator.Status() - if !status.Built.IsZero() && status.Error == nil { - slog.Info("Vulnerability database already exists", "built", status.Built) - return nil - } - - slog.Info("Downloading vulnerability database (this may take 5-10 minutes)...") + slog.Info("Checking vulnerability database for updates...") updated, err := curator.Update() if err != nil { - return fmt.Errorf("failed to download vulnerability database: %w", err) + return fmt.Errorf("failed to update vulnerability database: %w", err) } if updated { - slog.Info("Vulnerability database downloaded successfully") + slog.Info("Vulnerability database updated successfully") } else { slog.Info("Vulnerability database is up to date") } diff --git a/scanner/internal/scan/worker.go b/scanner/internal/scan/worker.go index 9c1db89..ce02ec5 100644 --- a/scanner/internal/scan/worker.go +++ b/scanner/internal/scan/worker.go @@ -7,6 +7,7 @@ import ( "fmt" "log/slog" "os" + "strings" "sync" "time" @@ -83,7 +84,11 @@ func (wp *WorkerPool) worker(ctx context.Context, id int) { result, err := wp.processJob(ctx, job) if err != nil { - slog.Error("Scan job failed", + logLevel := slog.LevelError + if strings.HasPrefix(err.Error(), "skipped:") { + logLevel = slog.LevelInfo + } + slog.Log(ctx, logLevel, "Scan job failed", "worker_id", id, "repository", job.Repository, "error", err) @@ -100,9 +105,20 @@ func (wp *WorkerPool) worker(ctx context.Context, id int) { } } +// unscannable config media types — these are OCI artifacts that aren't +// container images so Syft/Grype can't analyze their layers. +var unscannableConfigTypes = map[string]bool{ + "application/vnd.cncf.helm.config.v1+json": true, // Helm charts +} + func (wp *WorkerPool) processJob(ctx context.Context, job *scanner.ScanJob) (*scanner.ScanResult, error) { startTime := time.Now() + // Skip non-container OCI artifacts (Helm charts, WASM modules, etc.) + if unscannableConfigTypes[job.Config.MediaType] { + return nil, fmt.Errorf("skipped: unscannable artifact type %s", job.Config.MediaType) + } + // Ensure tmp dir exists if err := ensureDir(wp.cfg.Vuln.TmpDir); err != nil { return nil, fmt.Errorf("failed to create tmp dir: %w", err)