Added vulnerabilities cleanup cron (#41195)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -0,0 +1 @@
|
||||
* Fixed a bug where vulnerability counts increased over time due to orphaned entries remaining in the database after hosts were removed.
|
||||
@@ -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
|
||||
|
||||
+12
-1
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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())
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user