From 5c893a7b8ee34bc7c01a2cec22f028feb4706c79 Mon Sep 17 00:00:00 2001 From: Victor Lyuboslavsky <2685025+getvictor@users.noreply.github.com> Date: Tue, 10 Mar 2026 14:03:39 -0500 Subject: [PATCH] Added vulnerabilities cleanup cron (#41195) **Related issue:** Resolves #28091 # Checklist for submitter - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. ## Testing - [x] Added/updated automated tests - [x] Where appropriate, [automated tests simulate multiple hosts and test for host isolation](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/reference/patterns-backend.md#unit-testing) (updates to one hosts's records do not affect another) - [x] QA'd all new/changed functionality manually ## Summary by CodeRabbit ## Bug Fixes * Fixed an issue where vulnerability counts would inflate over time due to orphaned vulnerability entries remaining after hosts are removed. Vulnerability cleanup now automatically runs during routine scanning operations. --- changes/28091-vulnerabilities-cleanup | 1 + cmd/fleet/cron.go | 11 +++ cmd/fleet/serve_test.go | 13 +++- .../mysql/operating_system_vulnerabilities.go | 11 +++ .../operating_system_vulnerabilities_test.go | 66 ++++++++++++++++++ server/datastore/mysql/software.go | 11 +++ server/datastore/mysql/software_test.go | 67 +++++++++++++++++++ server/fleet/datastore.go | 6 ++ server/mock/datastore_mock.go | 24 +++++++ 9 files changed, 209 insertions(+), 1 deletion(-) create mode 100644 changes/28091-vulnerabilities-cleanup diff --git a/changes/28091-vulnerabilities-cleanup b/changes/28091-vulnerabilities-cleanup new file mode 100644 index 0000000000..166d3b9308 --- /dev/null +++ b/changes/28091-vulnerabilities-cleanup @@ -0,0 +1 @@ +* Fixed a bug where vulnerability counts increased over time due to orphaned entries remaining in the database after hosts were removed. diff --git a/cmd/fleet/cron.go b/cmd/fleet/cron.go index d68995a28d..5d84a648b3 100644 --- a/cmd/fleet/cron.go +++ b/cmd/fleet/cron.go @@ -199,6 +199,17 @@ func scanVulnerabilities( checkWinVulnerabilities(ctx, ds, logger, vulnPath, config, vulnAutomationEnabled != "") + // Clean up orphaned vulnerabilities (software/OS no longer associated with any host). + // This runs here (not in cleanups_then_aggregation) to stay in series with the scanners + // that write to the same tables, avoiding cross-schedule lock contention. The LEFT JOIN + // queries are index-backed on both sides, so execution time is fast for low orphan counts. + if err := ds.DeleteOrphanedSoftwareVulnerabilities(ctx); err != nil { + errHandler(ctx, logger, "deleting orphaned software vulnerabilities", err) + } + if err := ds.DeleteOrphanedOSVulnerabilities(ctx); err != nil { + errHandler(ctx, logger, "deleting orphaned OS vulnerabilities", err) + } + // If no automations enabled, then there is nothing else to do... if vulnAutomationEnabled == "" { return nil diff --git a/cmd/fleet/serve_test.go b/cmd/fleet/serve_test.go index ec486aa154..179b20737c 100644 --- a/cmd/fleet/serve_test.go +++ b/cmd/fleet/serve_test.go @@ -559,7 +559,12 @@ func TestScanVulnerabilities(t *testing.T) { ds.DeleteOutOfDateOSVulnerabilitiesFunc = func(ctx context.Context, src fleet.VulnerabilitySource, t time.Time) error { return nil } - + ds.DeleteOrphanedSoftwareVulnerabilitiesFunc = func(ctx context.Context) error { + return nil + } + ds.DeleteOrphanedOSVulnerabilitiesFunc = func(ctx context.Context) error { + return nil + } ds.ListCVEsFunc = func(ctx context.Context, maxAge time.Duration) ([]fleet.CVEMeta, error) { published := time.Date(2022, time.October, 26, 14, 15, 0, 0, time.UTC) @@ -741,6 +746,12 @@ func TestScanVulnerabilitiesFreeTier(t *testing.T) { ds.DeleteOutOfDateOSVulnerabilitiesFunc = func(ctx context.Context, src fleet.VulnerabilitySource, t time.Time) error { return nil } + ds.DeleteOrphanedSoftwareVulnerabilitiesFunc = func(ctx context.Context) error { + return nil + } + ds.DeleteOrphanedOSVulnerabilitiesFunc = func(ctx context.Context) error { + return nil + } ds.ListCVEsFunc = func(ctx context.Context, maxAge time.Duration) ([]fleet.CVEMeta, error) { t.Error("ListCVEs should not be called on free tier") return nil, nil diff --git a/server/datastore/mysql/operating_system_vulnerabilities.go b/server/datastore/mysql/operating_system_vulnerabilities.go index 0c07d7ecaf..e54960edd3 100644 --- a/server/datastore/mysql/operating_system_vulnerabilities.go +++ b/server/datastore/mysql/operating_system_vulnerabilities.go @@ -387,6 +387,17 @@ func (ds *Datastore) DeleteOutOfDateOSVulnerabilities(ctx context.Context, src f return nil } +func (ds *Datastore) DeleteOrphanedOSVulnerabilities(ctx context.Context) error { + if _, err := ds.writer(ctx).ExecContext(ctx, ` + DELETE osv FROM operating_system_vulnerabilities osv + LEFT JOIN host_operating_system hos ON hos.os_id = osv.operating_system_id + WHERE hos.host_id IS NULL + `); err != nil { + return ctxerr.Wrap(ctx, err, "deleting orphaned OS vulnerabilities") + } + return nil +} + func (ds *Datastore) ListKernelsByOS(ctx context.Context, osVersionID uint, teamID *uint) ([]*fleet.Kernel, error) { var kernels []*fleet.Kernel diff --git a/server/datastore/mysql/operating_system_vulnerabilities_test.go b/server/datastore/mysql/operating_system_vulnerabilities_test.go index 425c12e566..36b7a3f30d 100644 --- a/server/datastore/mysql/operating_system_vulnerabilities_test.go +++ b/server/datastore/mysql/operating_system_vulnerabilities_test.go @@ -29,6 +29,7 @@ func TestOperatingSystemVulnerabilities(t *testing.T) { {"DeleteOSVulnerabilitiesEmpty", testDeleteOSVulnerabilitiesEmpty}, {"DeleteOSVulnerabilities", testDeleteOSVulnerabilities}, {"DeleteOutOfDateOSVulnerabilities", testDeleteOutOfDateOSVulnerabilities}, + {"DeleteOrphanedOSVulnerabilities", testDeleteOrphanedOSVulnerabilities}, {"TestListKernelsByOS", testListKernelsByOS}, {"TestKernelVulnsHostCount", testKernelVulnsHostCount}, {"RefreshOSVersionVulnerabilities", testRefreshOSVersionVulnerabilities}, @@ -364,6 +365,71 @@ func testDeleteOutOfDateOSVulnerabilities(t *testing.T, ds *Datastore) { require.ElementsMatch(t, []fleet.OSVulnerability{newVuln}, actual) } +func testDeleteOrphanedOSVulnerabilities(t *testing.T, ds *Datastore) { + ctx := context.Background() + + hostWithOS := test.NewHost(t, ds, "host_with_os", "", "hwoskey", "hwosuuid", time.Now()) + hostToRemove := test.NewHost(t, ds, "host_to_remove", "", "htroskey", "htrosuuid", time.Now()) + + // Create two operating systems via raw SQL. + resWithHost, err := ds.writer(ctx).ExecContext(ctx, + "INSERT INTO operating_systems (name, version, arch, kernel_version, platform) VALUES (?, ?, ?, ?, ?)", + "Ubuntu", "22.04", "x86_64", "5.15.0", "ubuntu", + ) + require.NoError(t, err) + osWithHostID, err := resWithHost.LastInsertId() + require.NoError(t, err) + + resOrphan, err := ds.writer(ctx).ExecContext(ctx, + "INSERT INTO operating_systems (name, version, arch, kernel_version, platform) VALUES (?, ?, ?, ?, ?)", + "Ubuntu", "20.04", "x86_64", "5.4.0", "ubuntu", + ) + require.NoError(t, err) + osOrphanID, err := resOrphan.LastInsertId() + require.NoError(t, err) + + // Associate hosts with their operating systems. + _, err = ds.writer(ctx).ExecContext(ctx, + "INSERT INTO host_operating_system (host_id, os_id) VALUES (?, ?), (?, ?) ON DUPLICATE KEY UPDATE os_id = VALUES(os_id)", + hostWithOS.ID, osWithHostID, hostToRemove.ID, osOrphanID, + ) + require.NoError(t, err) + + // Insert vulnerabilities for both operating systems. + _, err = ds.InsertOSVulnerability(ctx, fleet.OSVulnerability{OSID: uint(osWithHostID), CVE: "CVE-2024-100"}, fleet.UbuntuOVALSource) + require.NoError(t, err) + _, err = ds.InsertOSVulnerability(ctx, fleet.OSVulnerability{OSID: uint(osOrphanID), CVE: "CVE-2024-200"}, fleet.UbuntuOVALSource) + require.NoError(t, err) + + // Remove the host, orphaning the OS. + err = ds.DeleteHost(ctx, hostToRemove.ID) + require.NoError(t, err) + + // Verify both vulns exist before cleanup. + vulnsWithHost, err := ds.ListOSVulnerabilitiesByOS(ctx, uint(osWithHostID)) + require.NoError(t, err) + require.Len(t, vulnsWithHost, 1) + + vulnsOrphan, err := ds.ListOSVulnerabilitiesByOS(ctx, uint(osOrphanID)) + require.NoError(t, err) + require.Len(t, vulnsOrphan, 1) + + // Run orphan cleanup. + err = ds.DeleteOrphanedOSVulnerabilities(ctx) + require.NoError(t, err) + + // Vulnerability for OS with a host should remain. + vulnsWithHost, err = ds.ListOSVulnerabilitiesByOS(ctx, uint(osWithHostID)) + require.NoError(t, err) + require.Len(t, vulnsWithHost, 1) + require.Equal(t, "CVE-2024-100", vulnsWithHost[0].CVE) + + // Vulnerability for orphaned OS should be deleted. + vulnsOrphan, err = ds.ListOSVulnerabilitiesByOS(ctx, uint(osOrphanID)) + require.NoError(t, err) + require.Empty(t, vulnsOrphan) +} + func testListKernelsByOS(t *testing.T, ds *Datastore) { ctx := context.Background() diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 5a55d36dca..8532685321 100644 --- a/server/datastore/mysql/software.go +++ b/server/datastore/mysql/software.go @@ -2468,6 +2468,17 @@ func (ds *Datastore) DeleteOutOfDateVulnerabilities(ctx context.Context, source return nil } +func (ds *Datastore) DeleteOrphanedSoftwareVulnerabilities(ctx context.Context) error { + if _, err := ds.writer(ctx).ExecContext(ctx, ` + DELETE sc FROM software_cve sc + LEFT JOIN host_software hs ON hs.software_id = sc.software_id + WHERE hs.host_id IS NULL + `); err != nil { + return ctxerr.Wrap(ctx, err, "deleting orphaned software vulnerabilities") + } + return nil +} + func (ds *Datastore) SoftwareByID(ctx context.Context, id uint, teamID *uint, includeCVEScores bool, tmFilter *fleet.TeamFilter) (*fleet.Software, error) { q := dialect.From(goqu.I("software").As("s")). Select( diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index 0ccf33d171..841c858281 100644 --- a/server/datastore/mysql/software_test.go +++ b/server/datastore/mysql/software_test.go @@ -71,6 +71,7 @@ func TestSoftware(t *testing.T) { {"AllSoftwareIteratorForCustomLinuxImages", testSoftwareIteratorForLinuxKernelCustomImages}, {"UpsertSoftwareCPEs", testUpsertSoftwareCPEs}, {"DeleteOutOfDateVulnerabilities", testDeleteOutOfDateVulnerabilities}, + {"DeleteOrphanedSoftwareVulnerabilities", testDeleteOrphanedSoftwareVulnerabilities}, {"DeleteSoftwareCPEs", testDeleteSoftwareCPEs}, {"SoftwareByIDNoDuplicatedVulns", testSoftwareByIDNoDuplicatedVulns}, {"SoftwareByIDIncludesCVEPublishedDate", testSoftwareByIDIncludesCVEPublishedDate}, @@ -3330,6 +3331,72 @@ func testDeleteOutOfDateVulnerabilities(t *testing.T, ds *Datastore) { require.Equal(t, "CVE-2023-001", storedSoftware.Vulnerabilities[0].CVE) } +func testDeleteOrphanedSoftwareVulnerabilities(t *testing.T, ds *Datastore) { + ctx := context.Background() + + hostWithSoftware := test.NewHost(t, ds, "host_with_sw", "", "hwskey", "hwsuuid", time.Now()) + hostToRemove := test.NewHost(t, ds, "host_to_remove", "", "htrkey", "htruuid", time.Now()) + + sharedSoftware := []fleet.Software{ + {Name: "shared_app", Version: "1.0", Source: "apps"}, + } + orphanSoftware := []fleet.Software{ + {Name: "orphan_app", Version: "2.0", Source: "apps"}, + } + + _, err := ds.UpdateHostSoftware(ctx, hostWithSoftware.ID, sharedSoftware) + require.NoError(t, err) + _, err = ds.UpdateHostSoftware(ctx, hostToRemove.ID, orphanSoftware) + require.NoError(t, err) + + require.NoError(t, ds.LoadHostSoftware(ctx, hostWithSoftware, false)) + require.NoError(t, ds.LoadHostSoftware(ctx, hostToRemove, false)) + + sharedSoftwareID := hostWithSoftware.Software[0].ID + orphanSoftwareID := hostToRemove.Software[0].ID + + // Insert vulnerabilities for both software items. + inserted, err := ds.InsertSoftwareVulnerability(ctx, fleet.SoftwareVulnerability{ + SoftwareID: sharedSoftwareID, CVE: "CVE-2024-001", + }, fleet.UbuntuOVALSource) + require.NoError(t, err) + require.True(t, inserted) + + inserted, err = ds.InsertSoftwareVulnerability(ctx, fleet.SoftwareVulnerability{ + SoftwareID: orphanSoftwareID, CVE: "CVE-2024-002", + }, fleet.UbuntuOVALSource) + require.NoError(t, err) + require.True(t, inserted) + + // Remove the host, making orphanSoftware's host_software entry disappear. + err = ds.DeleteHost(ctx, hostToRemove.ID) + require.NoError(t, err) + + // Verify both vulns still exist before cleanup. + storedShared, err := ds.SoftwareByID(ctx, sharedSoftwareID, nil, false, nil) + require.NoError(t, err) + require.Len(t, storedShared.Vulnerabilities, 1) + + storedOrphan, err := ds.SoftwareByID(ctx, orphanSoftwareID, nil, false, nil) + require.NoError(t, err) + require.Len(t, storedOrphan.Vulnerabilities, 1) + + // Run orphan cleanup. + err = ds.DeleteOrphanedSoftwareVulnerabilities(ctx) + require.NoError(t, err) + + // The vulnerability for shared software (still has a host) should remain. + storedShared, err = ds.SoftwareByID(ctx, sharedSoftwareID, nil, false, nil) + require.NoError(t, err) + require.Len(t, storedShared.Vulnerabilities, 1) + require.Equal(t, "CVE-2024-001", storedShared.Vulnerabilities[0].CVE) + + // The vulnerability for orphan software (no hosts) should be deleted. + storedOrphan, err = ds.SoftwareByID(ctx, orphanSoftwareID, nil, false, nil) + require.NoError(t, err) + require.Empty(t, storedOrphan.Vulnerabilities) +} + func testDeleteSoftwareCPEs(t *testing.T, ds *Datastore) { ctx := context.Background() host := test.NewHost(t, ds, "host1", "", "host1key", "host1uuid", time.Now()) diff --git a/server/fleet/datastore.go b/server/fleet/datastore.go index 3b69f86dfc..00e75e420c 100644 --- a/server/fleet/datastore.go +++ b/server/fleet/datastore.go @@ -879,6 +879,9 @@ type Datastore interface { // DeleteOutOfDateVulnerabilities deletes 'software_cve' entries from the provided source where // the updated_at timestamp is older than the provided timestamp DeleteOutOfDateVulnerabilities(ctx context.Context, source VulnerabilitySource, olderThan time.Time) error + // DeleteOrphanedSoftwareVulnerabilities deletes 'software_cve' entries where the software_id + // no longer has any associated hosts in 'host_software'. + DeleteOrphanedSoftwareVulnerabilities(ctx context.Context) error /////////////////////////////////////////////////////////////////////////////// // Calendar events @@ -1223,6 +1226,9 @@ type Datastore interface { // DeleteOutOfDateOSVulnerabilities deletes 'operating_system_vulnerabilities' entries from the provided source where // the updated_at timestamp is older than the supplied timestamp DeleteOutOfDateOSVulnerabilities(ctx context.Context, source VulnerabilitySource, olderThan time.Time) error + // DeleteOrphanedOSVulnerabilities deletes 'operating_system_vulnerabilities' entries where the operating_system_id + // no longer has any associated hosts in 'host_operating_system'. + DeleteOrphanedOSVulnerabilities(ctx context.Context) error ListKernelsByOS(ctx context.Context, osID uint, teamID *uint) ([]*Kernel, error) diff --git a/server/mock/datastore_mock.go b/server/mock/datastore_mock.go index 411ef48ec3..465063888c 100644 --- a/server/mock/datastore_mock.go +++ b/server/mock/datastore_mock.go @@ -669,6 +669,8 @@ type DeleteSoftwareVulnerabilitiesFunc func(ctx context.Context, vulnerabilities type DeleteOutOfDateVulnerabilitiesFunc func(ctx context.Context, source fleet.VulnerabilitySource, olderThan time.Time) error +type DeleteOrphanedSoftwareVulnerabilitiesFunc func(ctx context.Context) error + type CreateOrUpdateCalendarEventFunc func(ctx context.Context, uuid string, email string, startTime time.Time, endTime time.Time, data []byte, timeZone *string, hostID uint, webhookStatus fleet.CalendarWebhookStatus) (*fleet.CalendarEvent, error) type GetCalendarEventFunc func(ctx context.Context, email string) (*fleet.CalendarEvent, error) @@ -883,6 +885,8 @@ type InsertOSVulnerabilityFunc func(ctx context.Context, vuln fleet.OSVulnerabil type DeleteOutOfDateOSVulnerabilitiesFunc func(ctx context.Context, source fleet.VulnerabilitySource, olderThan time.Time) error +type DeleteOrphanedOSVulnerabilitiesFunc func(ctx context.Context) error + type ListKernelsByOSFunc func(ctx context.Context, osID uint, teamID *uint) ([]*fleet.Kernel, error) type InsertKernelSoftwareMappingFunc func(ctx context.Context) error @@ -2755,6 +2759,9 @@ type DataStore struct { DeleteOutOfDateVulnerabilitiesFunc DeleteOutOfDateVulnerabilitiesFunc DeleteOutOfDateVulnerabilitiesFuncInvoked bool + DeleteOrphanedSoftwareVulnerabilitiesFunc DeleteOrphanedSoftwareVulnerabilitiesFunc + DeleteOrphanedSoftwareVulnerabilitiesFuncInvoked bool + CreateOrUpdateCalendarEventFunc CreateOrUpdateCalendarEventFunc CreateOrUpdateCalendarEventFuncInvoked bool @@ -3076,6 +3083,9 @@ type DataStore struct { DeleteOutOfDateOSVulnerabilitiesFunc DeleteOutOfDateOSVulnerabilitiesFunc DeleteOutOfDateOSVulnerabilitiesFuncInvoked bool + DeleteOrphanedOSVulnerabilitiesFunc DeleteOrphanedOSVulnerabilitiesFunc + DeleteOrphanedOSVulnerabilitiesFuncInvoked bool + ListKernelsByOSFunc ListKernelsByOSFunc ListKernelsByOSFuncInvoked bool @@ -6693,6 +6703,13 @@ func (s *DataStore) DeleteOutOfDateVulnerabilities(ctx context.Context, source f return s.DeleteOutOfDateVulnerabilitiesFunc(ctx, source, olderThan) } +func (s *DataStore) DeleteOrphanedSoftwareVulnerabilities(ctx context.Context) error { + s.mu.Lock() + s.DeleteOrphanedSoftwareVulnerabilitiesFuncInvoked = true + s.mu.Unlock() + return s.DeleteOrphanedSoftwareVulnerabilitiesFunc(ctx) +} + func (s *DataStore) CreateOrUpdateCalendarEvent(ctx context.Context, uuid string, email string, startTime time.Time, endTime time.Time, data []byte, timeZone *string, hostID uint, webhookStatus fleet.CalendarWebhookStatus) (*fleet.CalendarEvent, error) { s.mu.Lock() s.CreateOrUpdateCalendarEventFuncInvoked = true @@ -7442,6 +7459,13 @@ func (s *DataStore) DeleteOutOfDateOSVulnerabilities(ctx context.Context, source return s.DeleteOutOfDateOSVulnerabilitiesFunc(ctx, source, olderThan) } +func (s *DataStore) DeleteOrphanedOSVulnerabilities(ctx context.Context) error { + s.mu.Lock() + s.DeleteOrphanedOSVulnerabilitiesFuncInvoked = true + s.mu.Unlock() + return s.DeleteOrphanedOSVulnerabilitiesFunc(ctx) +} + func (s *DataStore) ListKernelsByOS(ctx context.Context, osID uint, teamID *uint) ([]*fleet.Kernel, error) { s.mu.Lock() s.ListKernelsByOSFuncInvoked = true