From 24e5baf21f2d6d764cd6a4e616fd5207b7c4fd53 Mon Sep 17 00:00:00 2001 From: Scott Gress Date: Tue, 12 May 2026 15:58:13 -0700 Subject: [PATCH] Only collect data about tracked CVEs (#45247) **Related issue:** Resolves #45163 # Details Limits CVE data collection to only those CVEs which we report on in the chart. This is a performance optimization necessitated by the large amount of data that bigger fleets may generate. The plan is to implement a data compression strategy so that we can go back to collecting full CVE data soon. # Checklist for submitter If some of the following don't apply, delete the relevant line. - [ ] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. n/a, unreleased - [X] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. ## Testing - [X] Added/updated automated tests - [X] QA'd all new/changed functionality manually - [X] Ran some collection jobs and verified that only tracked CVEs were represented in "open" rows. - [ ] Ran load test w/ new code For unreleased bug fixes in a release candidate, one of: - [ ] Confirmed that the fix is not expected to adversely impact load test results should improve results! - [X] Alerted the release DRI if additional load testing is needed ## Summary by CodeRabbit * **Enhancements** * CVE vulnerability tracking is now scoped to a curated set of critical vulnerabilities, improving the relevance of security impact data displayed across your systems. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/fleetdm/fleet/pull/45247) --- server/chart/api/chart.go | 14 ++++-- server/chart/datasets.go | 12 ++++- server/chart/internal/mysql/charts.go | 47 +++++++++++------- server/chart/internal/service/service_test.go | 49 +++++++++++++++++-- server/chart/internal/types/chart.go | 11 +++-- 5 files changed, 101 insertions(+), 32 deletions(-) diff --git a/server/chart/api/chart.go b/server/chart/api/chart.go index 3195cd0c72..4d58999ba0 100644 --- a/server/chart/api/chart.go +++ b/server/chart/api/chart.go @@ -74,11 +74,19 @@ type DatasetStore interface { // recent host activity. FindRecentlySeenHostIDs(ctx context.Context, since time.Time, disabledFleetIDs []uint) ([]uint, error) - // AffectedHostIDsByCVE returns, for every CVE currently affecting any host, - // the slice of host IDs impacted by it. Unresolved-only is implicit in the + // AffectedHostIDsByCVE returns host IDs grouped by CVE, scoped to the given + // cves set. nil or empty cves returns an empty map — callers must pass the + // CVE set they want to collect for. Unresolved-only is implicit in the // underlying joins: a host's software/OS row transitions when it upgrades // past the vulnerable version, so the join naturally stops matching. - AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint) (map[string][]uint, error) + AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint, cves []string) (map[string][]uint, error) + + // TrackedCriticalCVEs returns CVE IDs matching the iteration-1 curated + // filter: critical (CVSS >= 9.0) CVEs on a hard-coded set of software + // titles, unioned with all critical OS vulnerabilities. Used by the CVE + // collector to scope collection to only the CVEs the chart actually + // renders. See TODO in the mysql implementation. + TrackedCriticalCVEs(ctx context.Context) ([]string, error) // RecordBucketData writes one or more entity bitmaps for the given bucket // using the specified sample strategy. See SampleStrategy for semantics. diff --git a/server/chart/datasets.go b/server/chart/datasets.go index be000e402b..bea81dcf58 100644 --- a/server/chart/datasets.go +++ b/server/chart/datasets.go @@ -43,7 +43,14 @@ func (c *CVEDataset) SampleStrategy() api.SampleStrategy { return api.SampleStra func (c *CVEDataset) DefaultVisualization() string { return "line" } func (c *CVEDataset) Collect(ctx context.Context, store api.DatasetStore, now time.Time, disabledFleetIDs []uint) error { - hostIDsByCVE, err := store.AffectedHostIDsByCVE(ctx, disabledFleetIDs) + // Only track the CVEs that the chart API currently returns. + // TODO: implement bitmap compression so we can track all CVEs. + tracked, err := store.TrackedCriticalCVEs(ctx) + if err != nil { + return err + } + + hostIDsByCVE, err := store.AffectedHostIDsByCVE(ctx, disabledFleetIDs, tracked) if err != nil { return err } @@ -52,5 +59,8 @@ func (c *CVEDataset) Collect(ctx context.Context, store api.DatasetStore, now ti bitmaps[cve] = HostIDsToBlob(hostIDs) } bucketStart := now.UTC().Truncate(time.Hour) + // Always call RecordBucketData, even when bitmaps is empty: snapshot + // semantics use an empty input to close any open rows for entities no + // longer in the tracked set (recordSnapshot's "absent entities" branch). return store.RecordBucketData(ctx, c.Name(), bucketStart, time.Hour, c.SampleStrategy(), bitmaps) } diff --git a/server/chart/internal/mysql/charts.go b/server/chart/internal/mysql/charts.go index 9039c3e6f0..2c5c85a171 100644 --- a/server/chart/internal/mysql/charts.go +++ b/server/chart/internal/mysql/charts.go @@ -102,11 +102,9 @@ func (ds *Datastore) FindRecentlySeenHostIDs(ctx context.Context, since time.Tim return ids, nil } -// TODO(iteration-2): the matcher list and TrackedCriticalCVEs exist only -// until user-configurable CVE filtering ships on the dashboard. When that -// lands, delete trackedCVESoftwareMatchers, TrackedCriticalCVEs, its entries -// on the Datastore/DatasetStore interfaces, and the `if metric == "cve"` -// branch in the chart service. See change `cve-chart-demo-filter`. +// The matcher list and TrackedCriticalCVEs exist as performance optimizations. +// TODO: implement bitmap compression so we can collect more CVE data. +// TODO: implement more filtering options for users. // cveSoftwareMatcher filters `software` rows by a MySQL LIKE pattern and an // optional source allowlist. Empty Sources means any source. @@ -148,12 +146,18 @@ var trackedCVESoftwareMatchers = []cveSoftwareMatcher{ {"kernel-%", []string{"rpm_packages"}}, } -// AffectedHostIDsByCVE returns host IDs grouped by CVE. It streams two joins -// (software-level and OS-level vulnerabilities) and merges the results into a -// single map. Duplicates across sources are harmless — the downstream -// HostIDsToBlob setBit is idempotent. -func (ds *Datastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint) (map[string][]uint, error) { +// AffectedHostIDsByCVE returns host IDs grouped by CVE, scoped to the given +// cves set. It streams two joins (software-level and OS-level vulnerabilities) +// and merges the results into a single map. Duplicates across sources are +// harmless — the downstream HostIDsToBlob setBit is idempotent. +// +// nil or empty cves returns an empty map without running any query. +// TODO: support `nil` meaning "all CVEs" once bitmap compression is implemented. +func (ds *Datastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint, cves []string) (map[string][]uint, error) { result := make(map[string][]uint) + if len(cves) == 0 { + return result, nil + } // Both subqueries gain a hosts JOIN + WHERE only when there are fleets to // exclude. Skipping the JOIN entirely when the slice is empty keeps the @@ -167,19 +171,26 @@ func (ds *Datastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs FROM operating_system_vulnerabilities osv JOIN host_operating_system hos ON hos.os_id = osv.operating_system_id` - var swArgs, osArgs []any + swWhere := []string{"sc.cve IN (?)"} + osWhere := []string{"osv.cve IN (?)"} + swArgs := []any{cves} + osArgs := []any{cves} + if len(disabledFleetIDs) > 0 { swQuery += ` - JOIN hosts h ON h.id = hs.host_id - WHERE (h.team_id IS NULL OR h.team_id NOT IN (?))` - swArgs = []any{disabledFleetIDs} + JOIN hosts h ON h.id = hs.host_id` + swWhere = append(swWhere, "(h.team_id IS NULL OR h.team_id NOT IN (?))") + swArgs = append(swArgs, disabledFleetIDs) osQuery += ` - JOIN hosts h ON h.id = hos.host_id - WHERE (h.team_id IS NULL OR h.team_id NOT IN (?))` - osArgs = []any{disabledFleetIDs} + JOIN hosts h ON h.id = hos.host_id` + osWhere = append(osWhere, "(h.team_id IS NULL OR h.team_id NOT IN (?))") + osArgs = append(osArgs, disabledFleetIDs) } + swQuery += " WHERE " + strings.Join(swWhere, " AND ") + osQuery += " WHERE " + strings.Join(osWhere, " AND ") + if err := ds.streamCVEHostPairs(ctx, swQuery, swArgs, result); err != nil { return nil, ctxerr.Wrap(ctx, err, "stream software CVE host pairs") } @@ -199,7 +210,7 @@ func (ds *Datastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs // distinguish "filter resolved to empty" from "no filter requested" (nil). // See GetSCDData for how empty vs nil is interpreted at the query layer. // -// TODO(iteration-2): replace with user-configurable filtering. See the +// TODO: replace with user-configurable filtering. See the // matcher-list comment above. func (ds *Datastore) TrackedCriticalCVEs(ctx context.Context) ([]string, error) { const criticalCVSS = 9.0 diff --git a/server/chart/internal/service/service_test.go b/server/chart/internal/service/service_test.go index 7517859693..6cf69936ca 100644 --- a/server/chart/internal/service/service_test.go +++ b/server/chart/internal/service/service_test.go @@ -59,7 +59,7 @@ type mockDatastore struct { getSCDDataFunc func(ctx context.Context, dataset string, startDate, endDate time.Time, bucketSize time.Duration, strategy api.SampleStrategy, filterMask []byte, entityIDs []string) ([]api.DataPoint, error) getHostIDsForFilterFunc func(ctx context.Context, hostFilter *types.HostFilter) ([]uint, error) findRecentlySeenHostIDsFn func(ctx context.Context, since time.Time, disabledFleetIDs []uint) ([]uint, error) - affectedHostIDsByCVEFn func(ctx context.Context, disabledFleetIDs []uint) (map[string][]uint, error) + affectedHostIDsByCVEFn func(ctx context.Context, disabledFleetIDs []uint, cves []string) (map[string][]uint, error) trackedCriticalCVEsFn func(ctx context.Context) ([]string, error) recordBucketDataFn func(ctx context.Context, dataset string, bucketStart time.Time, bucketSize time.Duration, strategy api.SampleStrategy, entityBitmaps map[string][]byte) error recordBucketDataInvoked bool @@ -75,9 +75,9 @@ func (m *mockDatastore) FindRecentlySeenHostIDs(ctx context.Context, since time. return nil, nil } -func (m *mockDatastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint) (map[string][]uint, error) { +func (m *mockDatastore) AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint, cves []string) (map[string][]uint, error) { if m.affectedHostIDsByCVEFn != nil { - return m.affectedHostIDsByCVEFn(ctx, disabledFleetIDs) + return m.affectedHostIDsByCVEFn(ctx, disabledFleetIDs, cves) } return nil, nil } @@ -600,7 +600,13 @@ func TestCollectDatasetsCVE(t *testing.T) { now := time.Date(2026, 4, 8, 14, 37, 0, 0, time.UTC) wantBucketStart := time.Date(2026, 4, 8, 14, 0, 0, 0, time.UTC) - ds.affectedHostIDsByCVEFn = func(_ context.Context, _ []uint) (map[string][]uint, error) { + wantTracked := []string{"CVE-2024-0001", "CVE-2024-0002"} + ds.trackedCriticalCVEsFn = func(_ context.Context) ([]string, error) { + return wantTracked, nil + } + var gotCVEs []string + ds.affectedHostIDsByCVEFn = func(_ context.Context, _ []uint, cves []string) (map[string][]uint, error) { + gotCVEs = cves return map[string][]uint{ "CVE-2024-0001": {1, 2, 3}, "CVE-2024-0002": {2, 4}, @@ -620,6 +626,36 @@ func TestCollectDatasetsCVE(t *testing.T) { err := svc.CollectDatasets(t.Context(), now, nil) require.NoError(t, err) assert.True(t, ds.recordBucketDataInvoked) + assert.Equal(t, wantTracked, gotCVEs, "TrackedCriticalCVEs result must be forwarded as the cves filter") +} + +// TestCollectDatasetsCVEEmptyTracked verifies that when TrackedCriticalCVEs +// returns an empty set, the collector still calls RecordBucketData with empty +// bitmaps so recordSnapshot's "absent entities" branch can close any open +// rows from prior cron ticks. Without this, dropping a CVE from the tracked +// set would leave its open row hanging forever. +func TestCollectDatasetsCVEEmptyTracked(t *testing.T) { + ds := &mockDatastore{} + svc := NewService(&mockAuthorizer{}, ds, globalViewer(), nil) + svc.RegisterDataset(&chart.CVEDataset{}) + + ds.trackedCriticalCVEsFn = func(_ context.Context) ([]string, error) { + return []string{}, nil + } + ds.affectedHostIDsByCVEFn = func(_ context.Context, _ []uint, cves []string) (map[string][]uint, error) { + assert.Empty(t, cves, "empty tracked set must propagate as empty cves filter") + return map[string][]uint{}, nil + } + var gotBitmaps map[string][]byte + ds.recordBucketDataFn = func(_ context.Context, _ string, _ time.Time, _ time.Duration, _ api.SampleStrategy, entityBitmaps map[string][]byte) error { + gotBitmaps = entityBitmaps + return nil + } + + err := svc.CollectDatasets(t.Context(), time.Now(), nil) + require.NoError(t, err) + assert.True(t, ds.recordBucketDataInvoked, "RecordBucketData must run on empty tracked set to close stale rows") + assert.Empty(t, gotBitmaps) } // TestCollectDatasetsForwardsScope verifies the scope resolver wiring: @@ -670,8 +706,11 @@ func TestCollectDatasetsForwardsScope(t *testing.T) { svc := NewService(&mockAuthorizer{}, ds, globalViewer(), nil) svc.RegisterDataset(&chart.CVEDataset{}) + ds.trackedCriticalCVEsFn = func(_ context.Context) ([]string, error) { + return []string{"CVE-1"}, nil + } var gotDisabled []uint - ds.affectedHostIDsByCVEFn = func(_ context.Context, disabled []uint) (map[string][]uint, error) { + ds.affectedHostIDsByCVEFn = func(_ context.Context, disabled []uint, _ []string) (map[string][]uint, error) { gotDisabled = disabled return map[string][]uint{"CVE-1": {1}}, nil } diff --git a/server/chart/internal/types/chart.go b/server/chart/internal/types/chart.go index 13a73b7159..307ebbe86f 100644 --- a/server/chart/internal/types/chart.go +++ b/server/chart/internal/types/chart.go @@ -34,11 +34,12 @@ type Datastore interface { // recent host activity. FindRecentlySeenHostIDs(ctx context.Context, since time.Time, disabledFleetIDs []uint) ([]uint, error) - // AffectedHostIDsByCVE returns, for every CVE currently affecting any host, - // the slice of host IDs impacted by it. Unresolved-only is implicit in the - // underlying joins: a host's software/OS row transitions when it upgrades - // past the vulnerable version, so the join naturally stops matching. - AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint) (map[string][]uint, error) + // AffectedHostIDsByCVE returns host IDs grouped by CVE, scoped to the given + // cves set. nil or empty cves returns an empty map. Unresolved-only is + // implicit in the underlying joins: a host's software/OS row transitions + // when it upgrades past the vulnerable version, so the join naturally + // stops matching. + AffectedHostIDsByCVE(ctx context.Context, disabledFleetIDs []uint, cves []string) (map[string][]uint, error) // TrackedCriticalCVEs returns CVE IDs matching the iteration-1 curated // filter: critical (CVSS >= 9.0) CVEs on a hard-coded set of software