diff --git a/internal/security/scanner/service.go b/internal/security/scanner/service.go index dc5943d8c..9bbf9673f 100644 --- a/internal/security/scanner/service.go +++ b/internal/security/scanner/service.go @@ -32,6 +32,7 @@ type Storage interface { SaveScanJob(job *ScanJob) error GetScanJob(id string) (*ScanJob, error) ListScanJobs(serverName string) ([]*ScanJob, error) + ListScanJobMetas(serverName string) ([]*ScanJobMeta, error) GetLatestScanJob(serverName string) (*ScanJob, error) DeleteScanJob(id string) error DeleteServerScanJobs(serverName string) error @@ -1083,25 +1084,23 @@ func (s *Service) GetScanReportByJobID(ctx context.Context, jobID string) (*Aggr agg.Pass1Complete = job.ScanPass == ScanPassSecurityScan && job.Status == ScanJobStatusCompleted agg.Pass2Complete = job.ScanPass == ScanPassSupplyChainAudit && job.Status == ScanJobStatusCompleted - // If this is a Pass 1 job, try to find and merge companion Pass 2 results + // If this is a Pass 1 job, try to find and merge companion Pass 2 results. + // The companion is resolved via the lightweight scan-job index, so this does + // NOT deserialize the full per-server scan history (MCP-2205). if job.ScanPass == ScanPassSecurityScan || job.ScanPass == 0 { - allJobs, _ := s.storage.ListScanJobs(job.ServerName) - for _, j := range allJobs { - if j.ScanPass == ScanPassSupplyChainAudit && j.Status == ScanJobStatusCompleted && j.StartedAt.After(job.StartedAt) { - pass2Reports, err := s.storage.ListScanReportsByJob(j.ID) - if err == nil { - for _, r := range pass2Reports { - for i := range r.Findings { - r.Findings[i].ScanPass = ScanPassSupplyChainAudit - } + if companionID := s.findCompanionPass2JobID(job); companionID != "" { + pass2Reports, err := s.storage.ListScanReportsByJob(companionID) + if err == nil { + for _, r := range pass2Reports { + for i := range r.Findings { + r.Findings[i].ScanPass = ScanPassSupplyChainAudit } - allMerged := append(reports, pass2Reports...) - allMerged = deduplicatePass2Findings(allMerged) - agg = AggregateReportsWithJobStatus(job.ID, job.ServerName, allMerged, job) - agg.Pass1Complete = true - agg.Pass2Complete = true } - break + allMerged := append(reports, pass2Reports...) + allMerged = deduplicatePass2Findings(allMerged) + agg = AggregateReportsWithJobStatus(job.ID, job.ServerName, allMerged, job) + agg.Pass1Complete = true + agg.Pass2Complete = true } } @@ -1118,6 +1117,40 @@ func (s *Service) GetScanReportByJobID(ctx context.Context, jobID string) (*Aggr return agg, nil } +// findCompanionPass2JobID returns the ID of the Pass-2 (supply-chain audit) job +// that companions the given Pass-1 job: the earliest completed Pass-2 job that +// started after it. It reads the lightweight scan-job metadata index rather than +// the full job records, so its cost is independent of scan-output size and the +// report path no longer slows down as a server accrues scan history (MCP-2205). +// Returns "" when no companion exists. +func (s *Service) findCompanionPass2JobID(pass1 *ScanJob) string { + metas, err := s.storage.ListScanJobMetas(pass1.ServerName) + if err != nil { + s.logger.Warn("failed to list scan job metadata for companion lookup", + zap.String("server", pass1.ServerName), + zap.Error(err), + ) + return "" + } + + var best *ScanJobMeta + for _, m := range metas { + if m.ScanPass != ScanPassSupplyChainAudit || m.Status != ScanJobStatusCompleted { + continue + } + if !m.StartedAt.After(pass1.StartedAt) { + continue + } + if best == nil || m.StartedAt.Before(best.StartedAt) { + best = m + } + } + if best == nil { + return "" + } + return best.ID +} + // deduplicatePass2Findings removes Pass 2 findings that duplicate Pass 1 findings. // The dedup key is scanner + rule_id + title (not location, since paths may differ between passes). func deduplicatePass2Findings(reports []*ScanReport) []*ScanReport { @@ -1158,38 +1191,51 @@ func deduplicatePass2Findings(reports []*ScanReport) []*ScanReport { // findLatestPassJobs finds the latest Pass 1 and Pass 2 jobs for a server. // Returns (pass1Job, pass2Job, error). At least one must be non-nil on success. func (s *Service) findLatestPassJobs(serverName string) (*ScanJob, *ScanJob, error) { - jobs, err := s.storage.ListScanJobs(serverName) + // Read lightweight metadata rather than full job records so this scales with + // neither scan-output size nor history depth: we deserialize at most the two + // jobs we actually return (MCP-2205). + metas, err := s.storage.ListScanJobMetas(serverName) if err != nil { // Surface the underlying I/O error so the caller can distinguish // transient failures from "no records found". - return nil, nil, fmt.Errorf("list scan jobs for %s: %w", serverName, err) + return nil, nil, fmt.Errorf("list scan job metadata for %s: %w", serverName, err) } - if len(jobs) == 0 { + if len(metas) == 0 { return nil, nil, fmt.Errorf("%w: %s", errNoScans, serverName) } - // Sort by start time descending (newest first) - sort.Slice(jobs, func(i, j int) bool { - return jobs[i].StartedAt.After(jobs[j].StartedAt) - }) - - var pass1Job, pass2Job *ScanJob - for _, j := range jobs { - if j.ScanPass == ScanPassSupplyChainAudit && pass2Job == nil { - pass2Job = j - } else if (j.ScanPass == ScanPassSecurityScan || j.ScanPass == 0) && pass1Job == nil { + // Pick the newest Pass-1 and Pass-2 job IDs by start time. + var pass1Meta, pass2Meta *ScanJobMeta + for _, m := range metas { + switch m.ScanPass { + case ScanPassSupplyChainAudit: + if pass2Meta == nil || m.StartedAt.After(pass2Meta.StartedAt) { + pass2Meta = m + } + case ScanPassSecurityScan, 0: // ScanPass == 0 handles legacy jobs (before two-pass was added) - pass1Job = j - } - if pass1Job != nil && pass2Job != nil { - break + if pass1Meta == nil || m.StartedAt.After(pass1Meta.StartedAt) { + pass1Meta = m + } } } - if pass1Job == nil && pass2Job == nil { + if pass1Meta == nil && pass2Meta == nil { return nil, nil, fmt.Errorf("%w: %s", errNoScans, serverName) } + var pass1Job, pass2Job *ScanJob + if pass1Meta != nil { + if pass1Job, err = s.storage.GetScanJob(pass1Meta.ID); err != nil { + return nil, nil, fmt.Errorf("load latest pass-1 job %s: %w", pass1Meta.ID, err) + } + } + if pass2Meta != nil { + if pass2Job, err = s.storage.GetScanJob(pass2Meta.ID); err != nil { + return nil, nil, fmt.Errorf("load latest pass-2 job %s: %w", pass2Meta.ID, err) + } + } + return pass1Job, pass2Job, nil } diff --git a/internal/security/scanner/service_test.go b/internal/security/scanner/service_test.go index 87b4bb465..3d3afdb48 100644 --- a/internal/security/scanner/service_test.go +++ b/internal/security/scanner/service_test.go @@ -93,6 +93,26 @@ func (m *mockStorage) ListScanJobs(serverName string) ([]*ScanJob, error) { return result, nil } +func (m *mockStorage) ListScanJobMetas(serverName string) ([]*ScanJobMeta, error) { + m.mu.Lock() + defer m.mu.Unlock() + var result []*ScanJobMeta + for _, j := range m.jobs { + if serverName != "" && j.ServerName != serverName { + continue + } + result = append(result, &ScanJobMeta{ + ID: j.ID, + ServerName: j.ServerName, + Status: j.Status, + ScanPass: j.ScanPass, + StartedAt: j.StartedAt, + CompletedAt: j.CompletedAt, + }) + } + return result, nil +} + func (m *mockStorage) GetLatestScanJob(serverName string) (*ScanJob, error) { m.mu.Lock() defer m.mu.Unlock() @@ -1545,12 +1565,14 @@ func TestScanFindingScanPassTag(t *testing.T) { } } -// countingStorage wraps any Storage and counts ListScanJobs invocations. -// Used for verifying the GetScanSummary negative-cache behavior introduced in -// spec 047. +// countingStorage wraps any Storage and counts storage probes. listCalls counts +// the heavy full-history ListScanJobs path (MCP-2205 asserts this stays 0 on the +// report hot paths); metaCalls counts the lightweight ListScanJobMetas index +// path used by the GetScanSummary negative-cache behavior (spec 047). type countingStorage struct { Storage listCalls atomic.Int64 + metaCalls atomic.Int64 } func newCountingStorage(inner Storage) *countingStorage { @@ -1562,6 +1584,11 @@ func (c *countingStorage) ListScanJobs(serverName string) ([]*ScanJob, error) { return c.Storage.ListScanJobs(serverName) } +func (c *countingStorage) ListScanJobMetas(serverName string) ([]*ScanJobMeta, error) { + c.metaCalls.Add(1) + return c.Storage.ListScanJobMetas(serverName) +} + // erroringStorage returns a transient error from ListScanJobs while delegating // other calls to the inner Storage. Used to verify that non-errNoScans errors // do NOT populate the negative cache. @@ -1576,6 +1603,11 @@ func (e *erroringStorage) ListScanJobs(string) ([]*ScanJob, error) { return nil, e.err } +func (e *erroringStorage) ListScanJobMetas(string) ([]*ScanJobMeta, error) { + e.listCalls.Add(1) + return nil, e.err +} + // Spec 047 — Phase 3 (US1): cache the "no scans found" sentinel. func TestGetScanSummary_CachesNegativeResult(t *testing.T) { @@ -1590,8 +1622,8 @@ func TestGetScanSummary_CachesNegativeResult(t *testing.T) { } // Without the negative-cache fix, this would be N storage calls. - if got := store.listCalls.Load(); got != 1 { - t.Errorf("expected exactly 1 ListScanJobs call after %d GetScanSummary invocations, got %d", N, got) + if got := store.metaCalls.Load(); got != 1 { + t.Errorf("expected exactly 1 ListScanJobMetas call after %d GetScanSummary invocations, got %d", N, got) } } @@ -1606,7 +1638,7 @@ func TestGetScanSummary_DoesNotCacheOnTransientError(t *testing.T) { // Transient error must NOT populate the negative cache: every call retries. if got := store.listCalls.Load(); got != int64(N) { - t.Errorf("expected %d ListScanJobs calls (no caching of transient errors), got %d", N, got) + t.Errorf("expected %d storage probes (no caching of transient errors), got %d", N, got) } } @@ -1619,8 +1651,8 @@ func TestGetScanSummary_OverwritesNilSentinelOnRealScan(t *testing.T) { if got := svc.GetScanSummary(context.Background(), "later-scanned"); got != nil { t.Fatalf("expected nil summary on first call, got %+v", got) } - if got := store.listCalls.Load(); got != 1 { - t.Fatalf("expected 1 ListScanJobs call after first GetScanSummary, got %d", got) + if got := store.metaCalls.Load(); got != 1 { + t.Fatalf("expected 1 ListScanJobMetas call after first GetScanSummary, got %d", got) } // Simulate a real scan landing for that server: insert a completed Pass-1 job @@ -1646,3 +1678,95 @@ func TestGetScanSummary_OverwritesNilSentinelOnRealScan(t *testing.T) { t.Errorf("expected real summary {Status: clean}, got %+v", got) } } + +// TestGetScanReportByJobID_LatencyIndependentOfHistory verifies the MCP-2205 +// fix: aggregating a Pass-1 report must NOT deserialize the full per-server scan +// history (ListScanJobs), whose job payloads carry large stdout/stderr. The +// companion Pass-2 lookup uses the lightweight metadata index instead, so report +// latency does not grow with how many times a server has been scanned. +func TestGetScanReportByJobID_LatencyIndependentOfHistory(t *testing.T) { + mock := newMockStorage() + store := newCountingStorage(mock) + svc := NewService(store, NewRegistry(t.TempDir(), zap.NewNop()), nil, t.TempDir(), zap.NewNop()) + + now := time.Now() + + // Pass-1 job under inspection + its report. + pass1 := &ScanJob{ID: "p1", ServerName: "srv", Status: ScanJobStatusCompleted, ScanPass: ScanPassSecurityScan, StartedAt: now.Add(-10 * time.Minute)} + _ = mock.SaveScanJob(pass1) + _ = mock.SaveScanReport(&ScanReport{ID: "r1", JobID: "p1", ServerName: "srv", ScannerID: "s", + Findings: []ScanFinding{{RuleID: "T1", Title: "tool poisoning", Scanner: "s", ThreatLevel: ThreatLevelDangerous}}}) + + // Companion Pass-2 job that started after Pass-1 + its report. + pass2 := &ScanJob{ID: "p2", ServerName: "srv", Status: ScanJobStatusCompleted, ScanPass: ScanPassSupplyChainAudit, StartedAt: now.Add(-8 * time.Minute)} + _ = mock.SaveScanJob(pass2) + _ = mock.SaveScanReport(&ScanReport{ID: "r2", JobID: "p2", ServerName: "srv", ScannerID: "s", + Findings: []ScanFinding{{RuleID: "CVE-1", Title: "known cve", Scanner: "s", ThreatLevel: ThreatLevelWarning}}}) + + // Large historical backlog for the same server (the symptom: reports get + // slower the more a server is scanned). + for i := 0; i < 100; i++ { + _ = mock.SaveScanJob(&ScanJob{ + ID: fmt.Sprintf("noise-%d", i), + ServerName: "srv", + Status: ScanJobStatusCompleted, + ScanPass: ScanPassSecurityScan, + StartedAt: now.Add(time.Duration(-20-i) * time.Minute), + }) + } + + agg, err := svc.GetScanReportByJobID(context.Background(), "p1") + if err != nil { + t.Fatalf("GetScanReportByJobID: %v", err) + } + + // Correctness: companion Pass-2 still merged in. + if !agg.Pass1Complete || !agg.Pass2Complete { + t.Errorf("expected both passes complete, got pass1=%v pass2=%v", agg.Pass1Complete, agg.Pass2Complete) + } + if len(agg.Findings) != 2 { + t.Fatalf("expected 2 merged findings (Pass-1 + companion Pass-2), got %d", len(agg.Findings)) + } + + // Latency guard: must not scan the full per-server job history. + if got := store.listCalls.Load(); got != 0 { + t.Errorf("expected 0 ListScanJobs (full-history) calls in report path, got %d", got) + } +} + +// TestGetScanReport_LatestLatencyIndependentOfHistory verifies the MCP-2205 fix +// also covers the "latest report" path (GetScanReport by server name, used by the +// Web UI server-detail view). Resolving the latest Pass-1/Pass-2 jobs must use the +// lightweight metadata index plus targeted GetScanJob loads (at most two full job +// deserializations), not a full ListScanJobs over the server's scan history. +func TestGetScanReport_LatestLatencyIndependentOfHistory(t *testing.T) { + mock := newMockStorage() + store := newCountingStorage(mock) + svc := NewService(store, NewRegistry(t.TempDir(), zap.NewNop()), nil, t.TempDir(), zap.NewNop()) + + now := time.Now() + + // Latest Pass-1 + Pass-2 with reports. + _ = mock.SaveScanJob(&ScanJob{ID: "latest-p1", ServerName: "srv", Status: ScanJobStatusCompleted, ScanPass: ScanPassSecurityScan, StartedAt: now.Add(-2 * time.Minute)}) + _ = mock.SaveScanReport(&ScanReport{ID: "lr1", JobID: "latest-p1", ServerName: "srv", ScannerID: "s", + Findings: []ScanFinding{{RuleID: "T1", Title: "tp", Scanner: "s", ThreatLevel: ThreatLevelDangerous}}}) + _ = mock.SaveScanJob(&ScanJob{ID: "latest-p2", ServerName: "srv", Status: ScanJobStatusCompleted, ScanPass: ScanPassSupplyChainAudit, StartedAt: now.Add(-1 * time.Minute)}) + _ = mock.SaveScanReport(&ScanReport{ID: "lr2", JobID: "latest-p2", ServerName: "srv", ScannerID: "s", + Findings: []ScanFinding{{RuleID: "CVE-9", Title: "cve", Scanner: "s", ThreatLevel: ThreatLevelWarning}}}) + + // Large historical backlog. + for i := 0; i < 100; i++ { + _ = mock.SaveScanJob(&ScanJob{ID: fmt.Sprintf("old-%d", i), ServerName: "srv", Status: ScanJobStatusCompleted, ScanPass: ScanPassSecurityScan, StartedAt: now.Add(time.Duration(-10-i) * time.Minute)}) + } + + report, err := svc.GetScanReport(context.Background(), "srv") + if err != nil { + t.Fatalf("GetScanReport: %v", err) + } + if len(report.Findings) != 2 { + t.Fatalf("expected 2 merged findings, got %d", len(report.Findings)) + } + if got := store.listCalls.Load(); got != 0 { + t.Errorf("expected 0 ListScanJobs (full-history) calls in latest-report path, got %d", got) + } +} diff --git a/internal/security/scanner/types.go b/internal/security/scanner/types.go index 2d55be35b..59b98d2de 100644 --- a/internal/security/scanner/types.go +++ b/internal/security/scanner/types.go @@ -120,6 +120,20 @@ type ScanJob struct { ScanContext *ScanContext `json:"scan_context,omitempty"` } +// ScanJobMeta is a lightweight projection of a scan job, persisted in a +// dedicated index bucket so that companion-job lookups during report +// aggregation never deserialize the full job payload (whose ScannerStatuses can +// carry large stdout/stderr blobs). This keeps report latency independent of a +// server's scan history. See MCP-2205. +type ScanJobMeta struct { + ID string `json:"id"` + ServerName string `json:"server_name"` + Status string `json:"status"` + ScanPass int `json:"scan_pass"` + StartedAt time.Time `json:"started_at"` + CompletedAt time.Time `json:"completed_at,omitempty"` +} + // ScanJobSummary is a lightweight view of a scan job for history listing type ScanJobSummary struct { ID string `json:"id"` diff --git a/internal/storage/bbolt.go b/internal/storage/bbolt.go index 84399f9c2..55f2efb67 100644 --- a/internal/storage/bbolt.go +++ b/internal/storage/bbolt.go @@ -93,6 +93,7 @@ func (b *BoltDB) initBuckets() error { ActivityStatsBucket, ScannersBucket, ScanJobsBucket, + ScanJobIndexBucket, ScanReportsBucket, IntegrityBaselinesBucket, OnboardingBucket, @@ -104,6 +105,12 @@ func (b *BoltDB) initBuckets() error { } } + // Backfill the scan-job index for databases created before MCP-2205. + // Idempotent: only runs when the index is empty but jobs exist. + if err := backfillScanJobIndex(tx); err != nil { + return fmt.Errorf("failed to backfill scan job index: %w", err) + } + // Set schema version only for new databases. Existing databases keep their // stored version so migrations can observe and upgrade them. metaBucket := tx.Bucket([]byte(MetaBucket)) diff --git a/internal/storage/manager.go b/internal/storage/manager.go index 305c317e0..7408db4bc 100644 --- a/internal/storage/manager.go +++ b/internal/storage/manager.go @@ -532,6 +532,15 @@ func (m *Manager) ListScanJobs(serverName string) ([]*scanner.ScanJob, error) { return m.db.ListScanJobs(serverName) } +// ListScanJobMetas returns lightweight scan-job metadata, optionally filtered by +// server name (MCP-2205). +func (m *Manager) ListScanJobMetas(serverName string) ([]*scanner.ScanJobMeta, error) { + m.mu.RLock() + defer m.mu.RUnlock() + + return m.db.ListScanJobMetas(serverName) +} + // GetLatestScanJob returns the most recent scan job for a server func (m *Manager) GetLatestScanJob(serverName string) (*scanner.ScanJob, error) { m.mu.RLock() diff --git a/internal/storage/models.go b/internal/storage/models.go index b75c30247..f7b45edf1 100644 --- a/internal/storage/models.go +++ b/internal/storage/models.go @@ -32,6 +32,7 @@ const ( // Security scanner buckets (Spec 039) ScannersBucket = "security_scanners" ScanJobsBucket = "security_scan_jobs" + ScanJobIndexBucket = "security_scan_job_index" // lightweight ScanJobMeta index (MCP-2205) ScanReportsBucket = "security_reports" IntegrityBaselinesBucket = "integrity_baselines" diff --git a/internal/storage/scanner.go b/internal/storage/scanner.go index 1bd8b925c..ba6a8b774 100644 --- a/internal/storage/scanner.go +++ b/internal/storage/scanner.go @@ -1,12 +1,60 @@ package storage import ( + "encoding/json" "fmt" "github.com/smart-mcp-proxy/mcpproxy-go/internal/security/scanner" "go.etcd.io/bbolt" ) +// scanJobMetaFrom projects a full ScanJob into its lightweight index entry. +func scanJobMetaFrom(job *scanner.ScanJob) *scanner.ScanJobMeta { + return &scanner.ScanJobMeta{ + ID: job.ID, + ServerName: job.ServerName, + Status: job.Status, + ScanPass: job.ScanPass, + StartedAt: job.StartedAt, + CompletedAt: job.CompletedAt, + } +} + +// putScanJobIndex writes the lightweight index entry for a job within tx. +func putScanJobIndex(tx *bbolt.Tx, job *scanner.ScanJob) error { + data, err := json.Marshal(scanJobMetaFrom(job)) + if err != nil { + return err + } + return tx.Bucket([]byte(ScanJobIndexBucket)).Put([]byte(job.ID), data) +} + +// backfillScanJobIndex repopulates the scan-job index from the jobs bucket when +// the index is empty (i.e. a database created before the index existed). It is +// idempotent and a no-op once the index has at least one entry. Runs inside the +// open transaction. See MCP-2205. +func backfillScanJobIndex(tx *bbolt.Tx) error { + idx := tx.Bucket([]byte(ScanJobIndexBucket)) + if idx.Stats().KeyN > 0 { + return nil // already populated + } + jobs := tx.Bucket([]byte(ScanJobsBucket)) + if jobs == nil || jobs.Stats().KeyN == 0 { + return nil // nothing to backfill + } + return jobs.ForEach(func(k, v []byte) error { + job := &scanner.ScanJob{} + if err := job.UnmarshalBinary(v); err != nil { + return err + } + data, err := json.Marshal(scanJobMetaFrom(job)) + if err != nil { + return err + } + return idx.Put(k, data) + }) +} + // Scanner plugin CRUD operations // SaveScanner saves a scanner plugin record @@ -76,7 +124,57 @@ func (b *BoltDB) SaveScanJob(job *scanner.ScanJob) error { if err != nil { return err } - return bucket.Put([]byte(job.ID), data) + if err := bucket.Put([]byte(job.ID), data); err != nil { + return err + } + // Keep the lightweight index in sync (MCP-2205). + return putScanJobIndex(tx, job) + }) +} + +// ListScanJobMetas returns lightweight scan-job metadata, optionally filtered by +// server name. Unlike ListScanJobs it reads from a dedicated index bucket and +// never deserializes the large per-job stdout/stderr payloads, so its cost is +// independent of scan-output size. See MCP-2205. +func (b *BoltDB) ListScanJobMetas(serverName string) ([]*scanner.ScanJobMeta, error) { + var records []*scanner.ScanJobMeta + + err := b.db.View(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(ScanJobIndexBucket)) + return bucket.ForEach(func(_, v []byte) error { + meta := &scanner.ScanJobMeta{} + if err := json.Unmarshal(v, meta); err != nil { + return err + } + if serverName != "" && meta.ServerName != serverName { + return nil + } + records = append(records, meta) + return nil + }) + }) + + return records, err +} + +// dropScanJobIndexForTest deletes every entry in the scan-job index bucket. +// Used by tests to emulate a pre-index database for backfill verification. +func (b *BoltDB) dropScanJobIndexForTest() error { + return b.db.Update(func(tx *bbolt.Tx) error { + bucket := tx.Bucket([]byte(ScanJobIndexBucket)) + var keys [][]byte + if err := bucket.ForEach(func(k, _ []byte) error { + keys = append(keys, append([]byte(nil), k...)) + return nil + }); err != nil { + return err + } + for _, k := range keys { + if err := bucket.Delete(k); err != nil { + return err + } + } + return nil }) } @@ -154,8 +252,10 @@ func (b *BoltDB) GetLatestScanJob(serverName string) (*scanner.ScanJob, error) { // DeleteScanJob deletes a scan job by ID func (b *BoltDB) DeleteScanJob(id string) error { return b.db.Update(func(tx *bbolt.Tx) error { - bucket := tx.Bucket([]byte(ScanJobsBucket)) - return bucket.Delete([]byte(id)) + if err := tx.Bucket([]byte(ScanJobsBucket)).Delete([]byte(id)); err != nil { + return err + } + return tx.Bucket([]byte(ScanJobIndexBucket)).Delete([]byte(id)) }) } @@ -163,6 +263,7 @@ func (b *BoltDB) DeleteScanJob(id string) error { func (b *BoltDB) DeleteServerScanJobs(serverName string) error { return b.db.Update(func(tx *bbolt.Tx) error { bucket := tx.Bucket([]byte(ScanJobsBucket)) + idx := tx.Bucket([]byte(ScanJobIndexBucket)) var keysToDelete [][]byte err := bucket.ForEach(func(k, v []byte) error { record := &scanner.ScanJob{} @@ -170,7 +271,7 @@ func (b *BoltDB) DeleteServerScanJobs(serverName string) error { return err } if record.ServerName == serverName { - keysToDelete = append(keysToDelete, k) + keysToDelete = append(keysToDelete, append([]byte(nil), k...)) } return nil }) @@ -181,6 +282,9 @@ func (b *BoltDB) DeleteServerScanJobs(serverName string) error { if err := bucket.Delete(key); err != nil { return err } + if err := idx.Delete(key); err != nil { + return err + } } return nil }) diff --git a/internal/storage/scanner_test.go b/internal/storage/scanner_test.go index 6bc6731ff..168917e0a 100644 --- a/internal/storage/scanner_test.go +++ b/internal/storage/scanner_test.go @@ -499,3 +499,163 @@ func TestIntegrityBaselineGetNotFound(t *testing.T) { t.Error("expected error for nonexistent baseline, got nil") } } + +// largeStatuses builds scanner statuses with big stdout/stderr payloads so the +// backfill / metadata tests exercise the case the bug is about: full job records +// are expensive to deserialize, but metadata is not. +func largeStatuses() []scanner.ScannerJobStatus { + big := make([]byte, 64*1024) + for i := range big { + big[i] = 'x' + } + return []scanner.ScannerJobStatus{ + {ScannerID: "osv", Status: "completed", Stdout: string(big), Stderr: string(big), FindingsCount: 1}, + } +} + +func TestListScanJobMetas_ProjectsLightweightFields(t *testing.T) { + db := newTestDB(t) + + now := time.Now().Truncate(time.Second) + job := &scanner.ScanJob{ + ID: "scan-srv-1", + ServerName: "srv", + Status: scanner.ScanJobStatusCompleted, + ScanPass: scanner.ScanPassSecurityScan, + StartedAt: now, + CompletedAt: now.Add(time.Second), + ScannerStatuses: largeStatuses(), + } + if err := db.SaveScanJob(job); err != nil { + t.Fatalf("SaveScanJob: %v", err) + } + + metas, err := db.ListScanJobMetas("srv") + if err != nil { + t.Fatalf("ListScanJobMetas: %v", err) + } + if len(metas) != 1 { + t.Fatalf("expected 1 meta, got %d", len(metas)) + } + m := metas[0] + if m.ID != "scan-srv-1" || m.ServerName != "srv" || m.ScanPass != scanner.ScanPassSecurityScan || + m.Status != scanner.ScanJobStatusCompleted { + t.Errorf("unexpected meta: %+v", m) + } + if !m.StartedAt.Equal(now) { + t.Errorf("expected StartedAt %v, got %v", now, m.StartedAt) + } +} + +func TestListScanJobMetas_FilterByServer(t *testing.T) { + db := newTestDB(t) + now := time.Now() + for i, srv := range []string{"a", "a", "b"} { + job := &scanner.ScanJob{ + ID: "job-" + srv + "-" + time.Duration(i).String(), + ServerName: srv, + Status: scanner.ScanJobStatusCompleted, + StartedAt: now.Add(time.Duration(i) * time.Second), + } + if err := db.SaveScanJob(job); err != nil { + t.Fatalf("SaveScanJob: %v", err) + } + } + metas, err := db.ListScanJobMetas("a") + if err != nil { + t.Fatalf("ListScanJobMetas: %v", err) + } + if len(metas) != 2 { + t.Fatalf("expected 2 metas for server a, got %d", len(metas)) + } + all, err := db.ListScanJobMetas("") + if err != nil { + t.Fatalf("ListScanJobMetas(all): %v", err) + } + if len(all) != 3 { + t.Fatalf("expected 3 metas total, got %d", len(all)) + } +} + +func TestScanJobIndex_MaintainedOnDelete(t *testing.T) { + db := newTestDB(t) + now := time.Now() + job := &scanner.ScanJob{ID: "j1", ServerName: "srv", Status: scanner.ScanJobStatusCompleted, StartedAt: now} + if err := db.SaveScanJob(job); err != nil { + t.Fatalf("SaveScanJob: %v", err) + } + if err := db.DeleteScanJob("j1"); err != nil { + t.Fatalf("DeleteScanJob: %v", err) + } + metas, err := db.ListScanJobMetas("srv") + if err != nil { + t.Fatalf("ListScanJobMetas: %v", err) + } + if len(metas) != 0 { + t.Errorf("expected index entry removed on delete, got %d metas", len(metas)) + } +} + +func TestScanJobIndex_MaintainedOnDeleteServerJobs(t *testing.T) { + db := newTestDB(t) + now := time.Now() + for i := 0; i < 3; i++ { + job := &scanner.ScanJob{ID: "j" + time.Duration(i).String(), ServerName: "srv", Status: scanner.ScanJobStatusCompleted, StartedAt: now.Add(time.Duration(i) * time.Second)} + if err := db.SaveScanJob(job); err != nil { + t.Fatalf("SaveScanJob: %v", err) + } + } + if err := db.DeleteServerScanJobs("srv"); err != nil { + t.Fatalf("DeleteServerScanJobs: %v", err) + } + metas, err := db.ListScanJobMetas("srv") + if err != nil { + t.Fatalf("ListScanJobMetas: %v", err) + } + if len(metas) != 0 { + t.Errorf("expected all index entries removed, got %d metas", len(metas)) + } +} + +// TestScanJobIndex_BackfillFromExistingJobs simulates a DB upgraded from a +// version that predates the index: jobs exist in the jobs bucket but the index +// bucket is empty. Reopening the DB must backfill the index so metadata reads +// (and thus report aggregation) work without re-scanning. +func TestScanJobIndex_BackfillFromExistingJobs(t *testing.T) { + dir := t.TempDir() + logger := zap.NewNop().Sugar() + + db, err := NewBoltDB(dir, logger) + if err != nil { + t.Fatalf("NewBoltDB: %v", err) + } + now := time.Now().Truncate(time.Second) + job := &scanner.ScanJob{ID: "legacy-1", ServerName: "srv", Status: scanner.ScanJobStatusCompleted, ScanPass: scanner.ScanPassSecurityScan, StartedAt: now, ScannerStatuses: largeStatuses()} + if err := db.SaveScanJob(job); err != nil { + t.Fatalf("SaveScanJob: %v", err) + } + + // Wipe the index bucket to emulate a pre-index database. + if err := db.dropScanJobIndexForTest(); err != nil { + t.Fatalf("dropScanJobIndexForTest: %v", err) + } + if metas, _ := db.ListScanJobMetas("srv"); len(metas) != 0 { + t.Fatalf("precondition: expected empty index after wipe, got %d", len(metas)) + } + db.Close() + + // Reopen: backfill should repopulate the index from the jobs bucket. + db2, err := NewBoltDB(dir, logger) + if err != nil { + t.Fatalf("reopen NewBoltDB: %v", err) + } + t.Cleanup(func() { db2.Close() }) + + metas, err := db2.ListScanJobMetas("srv") + if err != nil { + t.Fatalf("ListScanJobMetas after reopen: %v", err) + } + if len(metas) != 1 || metas[0].ID != "legacy-1" { + t.Fatalf("expected backfilled meta legacy-1, got %+v", metas) + } +}