Don't process empty vuln data and remove existing records (#46075)

**Related issue:** Resolves #45602
This commit is contained in:
Dante Catalfamo
2026-05-27 17:08:47 -04:00
committed by GitHub
parent 36eace6fdf
commit dc56776a30
15 changed files with 339 additions and 15 deletions
+1
View File
@@ -0,0 +1 @@
- Fixed corrupted vulnerabilities download removing existing detections
@@ -123,6 +123,10 @@ func Analyze(
}
// LoadDb returns the latest goval_dictionary database for the given platform.
// Returns an error if the database contains no definitions, since an empty database
// would cause every existing vulnerability for the platform to be deleted (every host
// would appear to be patched). An empty DB usually means the artifact download was
// corrupted or partially failed.
func LoadDb(platform oval.Platform, vulnPath string) (*Database, error) {
if !platform.IsGovalDictionarySupported() {
return nil, fmt.Errorf("platform %q not supported", platform)
@@ -139,6 +143,16 @@ func LoadDb(platform oval.Platform, vulnPath string) (*Database, error) {
return nil, err
}
var defCount int
if err := sqlite.QueryRow("SELECT COUNT(*) FROM definitions").Scan(&defCount); err != nil {
sqlite.Close()
return nil, fmt.Errorf("checking definitions count in %s: %w", latest, err)
}
if defCount == 0 {
sqlite.Close()
return nil, fmt.Errorf("goval_dictionary database %q contains no definitions (possible corrupted feed)", latest)
}
db := NewDB(sqlite, platform)
return db, nil
}
@@ -3,6 +3,7 @@ package macoffice
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
@@ -143,6 +144,20 @@ func Analyze(
return nil, nil
}
// Refuse to proceed if the loaded release notes contain no valid security updates —
// without them every existing MacOffice vulnerability would be marked as remediated.
// This usually indicates the bulletin file is corrupted or partially downloaded.
hasValid := false
for i := range relNotes {
if relNotes[i].Valid() {
hasValid = true
break
}
}
if !hasValid {
return nil, errors.New("MacOffice release notes contain no valid security updates (possible corrupted feed)")
}
queryParams := fleet.SoftwareIterQueryOptions{IncludedSources: []string{"apps"}}
iter, err := ds.AllSoftwareIterator(ctx, queryParams)
if err != nil {
@@ -30,6 +30,22 @@ func TestAnalyzer(t *testing.T) {
require.Empty(t, vulns)
require.NoError(t, err)
})
// Regression test for https://github.com/fleetdm/fleet/issues/45602:
// when the release notes file exists but contains no valid security updates
// (corrupted feed), Analyze must refuse to proceed rather than wiping every
// existing MacOffice vulnerability.
t.Run("when release notes contain no valid security updates", func(t *testing.T) {
vulnPath := t.TempDir()
// A release note without SecurityUpdates is considered invalid.
err := ReleaseNotes{{Version: "1.0"}}.Serialize(time.Now(), vulnPath)
require.NoError(t, err)
_, err = Analyze(ctx, nil, vulnPath, false)
require.Error(t, err)
require.Contains(t, err.Error(), "no valid security updates")
})
})
t.Run("updateVulnsInDB", func(t *testing.T) {
+8
View File
@@ -2,6 +2,7 @@ package msrc
import (
"context"
"errors"
"fmt"
"log/slog"
"strconv"
@@ -33,6 +34,13 @@ func Analyze(
return nil, err
}
// Refuse to proceed if the loaded bulletin contains no vulnerability data — an empty
// bulletin would cause every existing MSRC OS vulnerability for this OS to be marked as
// remediated. This usually indicates the bulletin file was corrupted during download.
if len(bulletin.Vulnerabilities) == 0 {
return nil, errors.New("MSRC bulletin contains no vulnerabilities (possible corrupted feed)")
}
// Find matching products inside the bulletin
matchingPIDs := make(map[string]bool)
pID, err := bulletin.Products.GetMatchForOS(ctx, os)
@@ -56,6 +56,28 @@ func TestIsVulnPatched(t *testing.T) {
require.Equal(t, prod.Name(), actual.ProductName)
})
})
// Regression test for https://github.com/fleetdm/fleet/issues/45602:
// A bulletin with no vulnerabilities (e.g., a corrupted artifact) must cause Analyze
// to return an error rather than silently marking every existing MSRC OS vulnerability
// for the host as remediated.
t.Run("Analyze rejects bulletin with no vulnerabilities", func(t *testing.T) {
d := time.Now()
dir := t.TempDir()
b := parsed.NewSecurityBulletin(prod.Name())
b.Products["1235"] = prod
// Vulnerabilities map intentionally left empty.
fileName := io.MSRCFileName(b.ProductName, d)
payload, err := json.Marshal(b)
require.NoError(t, err)
require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), payload, 0o644))
_, err = Analyze(t.Context(), nil, op, dir, false, slog.New(slog.DiscardHandler))
require.Error(t, err)
require.Contains(t, err.Error(), "no vulnerabilities")
})
}
func TestIsOSVulnerable(t *testing.T) {
+12 -2
View File
@@ -322,22 +322,32 @@ func TranslateCPEToCVE(
osInsertErr = true
}
// Detect corrupted/empty CVE feeds. If we had CPE/OS inputs to match against but produced
// zero results across every feed file, the feed is almost certainly empty or corrupted
// (e.g., a failed/corrupted artifact from GitHub) — skip the deletes so we don't wipe
// legitimate existing software_cve rows that will be re-matched on the next good sync.
feedProducedNoData := len(allSoftwareVulns) == 0 && len(allOSVulns) == 0
// Delete any stale vulnerabilities. A vulnerability is stale iff the last time it was
// updated was more than `2 * periodicity` ago. This assumes that the whole vulnerability
// process completes in less than `periodicity` units of time.
//
// This is used to get rid of false positives once they are fixed and no longer detected as vulnerabilities.
// Skip cleanup when the corresponding insert failed to avoid deleting data with nothing to replace it.
if softwareInsertErr == nil {
if softwareInsertErr == nil && !feedProducedNoData {
if err = ds.DeleteOutOfDateVulnerabilities(ctx, fleet.NVDSource, startTime); err != nil {
logger.ErrorContext(ctx, "error deleting out of date vulnerabilities", "err", err)
}
}
if !osInsertErr {
if !osInsertErr && !feedProducedNoData {
if err = ds.DeleteOutOfDateOSVulnerabilities(ctx, fleet.NVDSource, startTime); err != nil {
logger.ErrorContext(ctx, "error deleting out of date OS vulnerabilities", "err", err)
}
}
if feedProducedNoData {
logger.ErrorContext(ctx, "NVD scan produced no matches with non-empty input; skipping deletes to preserve existing software_cve rows (feed may be corrupted)",
"software_cpes", len(parsed), "os_cpes", len(cpes), "feed_files", len(files))
}
return newVulns, nil
}
+14
View File
@@ -278,6 +278,13 @@ func loadOSVArtifact(ctx context.Context, ver fleet.OSVersion, vulnPath string,
return nil, fmt.Errorf("decoding OSV artifact: %w", err)
}
// Refuse to use an artifact that has no vulnerability data — an empty artifact would
// cause every existing OSV vulnerability for matching software to be marked as remediated.
// This usually indicates the artifact download from GitHub was corrupted.
if len(artifact.Vulnerabilities) == 0 {
return nil, fmt.Errorf("OSV artifact %q contains no vulnerabilities (possible corrupted feed)", artifactFile)
}
logger.DebugContext(ctx, "loaded osv artifact",
"file", filepath.Base(artifactFile),
"ubuntu_version", artifact.UbuntuVersion,
@@ -648,6 +655,13 @@ func loadRHELOSVArtifact(ctx context.Context, ver fleet.OSVersion, vulnPath stri
return nil, fmt.Errorf("decoding RHEL OSV artifact: %w", err)
}
// Refuse to use an artifact that has no vulnerability data — an empty artifact would
// cause every existing RHEL OSV vulnerability for matching software to be marked as
// remediated. This usually indicates the artifact download from GitHub was corrupted.
if len(artifact.Vulnerabilities) == 0 {
return nil, fmt.Errorf("RHEL OSV artifact %q contains no vulnerabilities (possible corrupted feed)", artifactFile)
}
logger.DebugContext(ctx, "loaded rhel osv artifact",
"file", filepath.Base(artifactFile),
"rhel_version", artifact.RHELVersion,
+48 -1
View File
@@ -600,6 +600,53 @@ func TestFindLatestOSVArtifactForVersion(t *testing.T) {
}
}
// TestLoadOSVArtifactRejectsEmpty verifies that loadOSVArtifact refuses an artifact with
// no vulnerability data. An empty artifact would cause every existing OSV vulnerability for
// matching software to be marked as remediated.
// See https://github.com/fleetdm/fleet/issues/45602.
func TestLoadOSVArtifactRejectsEmpty(t *testing.T) {
tmpDir := t.TempDir()
path := filepath.Join(tmpDir, "osv-ubuntu-2204-2026-03-30.json.gz")
f, err := os.Create(path)
require.NoError(t, err)
gz := gzip.NewWriter(f)
_, err = gz.Write([]byte(`{"schema_version":"1.0.0","ubuntu_version":"2204","generated":"2026-03-30T00:00:00Z","total_cves":0,"total_packages":0,"vulnerabilities":{}}`))
require.NoError(t, err)
gz.Close()
f.Close()
ctx := context.Background()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
ver := fleet.OSVersion{Name: "Ubuntu 22.04.8 LTS", Version: "22.04.8 LTS"}
_, err = loadOSVArtifact(ctx, ver, tmpDir, logger, time.Time{})
require.Error(t, err)
require.Contains(t, err.Error(), "no vulnerabilities")
}
// TestLoadRHELOSVArtifactRejectsEmpty mirrors the Ubuntu OSV check for the RHEL OSV artifact.
func TestLoadRHELOSVArtifactRejectsEmpty(t *testing.T) {
tmpDir := t.TempDir()
path := filepath.Join(tmpDir, "osv-rhel-9-2026-04-08.json.gz")
f, err := os.Create(path)
require.NoError(t, err)
gz := gzip.NewWriter(f)
_, err = gz.Write([]byte(`{"schema_version":"1.0.0","rhel_version":"9","generated":"2026-04-08T00:00:00Z","total_cves":0,"total_packages":0,"vulnerabilities":{}}`))
require.NoError(t, err)
gz.Close()
f.Close()
ctx := context.Background()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
ver := fleet.OSVersion{Name: "Red Hat Enterprise Linux 9.0.0", Version: "9.0.0"}
_, err = loadRHELOSVArtifact(ctx, ver, tmpDir, logger, time.Time{})
require.Error(t, err)
require.Contains(t, err.Error(), "no vulnerabilities")
}
func TestLoadOSVArtifactZeroTimeUsesLatest(t *testing.T) {
tmpDir := t.TempDir()
@@ -620,7 +667,7 @@ func TestLoadOSVArtifactZeroTimeUsesLatest(t *testing.T) {
require.NoError(t, err)
gz := gzip.NewWriter(f)
_, err = gz.Write([]byte(`{"schema_version":"1.0.0","ubuntu_version":"2204","generated":"2026-03-30T00:00:00Z","total_cves":0,"total_packages":0,"vulnerabilities":{}}`))
_, err = gz.Write([]byte(`{"schema_version":"1.0.0","ubuntu_version":"2204","generated":"2026-03-30T00:00:00Z","total_cves":1,"total_packages":1,"vulnerabilities":{"openssl":[{"cve":"CVE-2024-0001","published":"2024-01-01T00:00:00Z","modified":"2024-01-01T00:00:00Z","details":"x","introduced":"0","fixed":"1.0"}]}}`))
require.NoError(t, err)
gz.Close()
f.Close()
+10
View File
@@ -149,6 +149,10 @@ func Analyze(
}
// loadDef returns the latest oval Definition for the given platform.
// Returns an error if the loaded definition file contains no rules, since an empty
// definition would cause every existing vulnerability for the platform to be deleted
// (it would look like every host was suddenly patched). An empty file usually means
// the artifact download from GitHub was corrupted or partially failed.
func loadDef(platform Platform, vulnPath string) (oval_parsed.Result, error) {
if !platform.IsSupported() {
return nil, fmt.Errorf("platform %q not supported", platform)
@@ -169,6 +173,9 @@ func loadDef(platform Platform, vulnPath string) (oval_parsed.Result, error) {
if err := json.Unmarshal(payload, &result); err != nil {
return nil, err
}
if len(result.Definitions) == 0 {
return nil, fmt.Errorf("OVAL definition file %q contains no rules (possible corrupted feed)", latest)
}
return result, nil
}
@@ -177,6 +184,9 @@ func loadDef(platform Platform, vulnPath string) (oval_parsed.Result, error) {
if err := json.Unmarshal(payload, &result); err != nil {
return nil, err
}
if len(result.Definitions) == 0 {
return nil, fmt.Errorf("OVAL definition file %q contains no rules (possible corrupted feed)", latest)
}
return result, nil
}
@@ -382,5 +382,24 @@ func TestOvalAnalyzer(t *testing.T) {
_, err := loadDef(platform, "")
require.Error(t, err, "invalid vulnerabity path")
})
// Regression test for https://github.com/fleetdm/fleet/issues/45602:
// loadDef must refuse a definition file with no rules, otherwise an empty/corrupted
// artifact would cause every existing vulnerability for the platform to be deleted.
t.Run("rejects empty definition file", func(t *testing.T) {
vulnPath := t.TempDir()
for _, platform := range []Platform{
NewPlatform("ubuntu", "Ubuntu 22.04.0"),
NewPlatform("rhel", "Red Hat Enterprise Linux 9.0.0"),
} {
fileName := platform.ToFilename(time.Now(), "json")
require.NoError(t, os.WriteFile(filepath.Join(vulnPath, fileName), []byte(`{"Definitions":null}`), 0o644))
_, err := loadDef(platform, vulnPath)
require.Error(t, err, "loadDef should reject %s definition with no rules", platform)
require.Contains(t, err.Error(), "no rules")
}
})
})
}
+71 -10
View File
@@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io"
"io/fs"
"net/http"
"net/url"
"os"
@@ -52,7 +53,7 @@ func downloadDecompressed(client *http.Client) func(string, string) error {
}
}
func whatToDownload(osVers *fleet.OSVersions, existing map[string]bool, date time.Time) []Platform {
func whatToDownload(osVers *fleet.OSVersions, existing map[string]struct{}, date time.Time) []Platform {
var r []Platform
for _, os := range osVers.OSVersions {
platform := NewPlatform(os.Platform, os.Name)
@@ -65,15 +66,19 @@ func whatToDownload(osVers *fleet.OSVersions, existing map[string]bool, date tim
}
// removeOldDefs walks 'path' removing any old oval definitions, returns a set containing
// definitions that are up to date according to 'date'
func removeOldDefs(date time.Time, path string) (map[string]bool, error) {
// definitions that are up to date according to 'date'.
//
// Prefer listUpToDateDefs + removeOutdatedDefs for the Refresh flow so that outdated files
// stay on disk when a sync fails (used as a fallback). This combined remove+list is kept
// for backwards compatibility with existing tests.
func removeOldDefs(date time.Time, path string) (map[string]struct{}, error) {
dateSuffix := fmt.Sprintf("-%d_%02d_%02d.json", date.Year(), date.Month(), date.Day())
upToDate := make(map[string]bool)
upToDate := make(map[string]struct{})
err := filepath.WalkDir(path, func(path string, d os.DirEntry, err error) error {
if strings.HasPrefix(filepath.Base(path), OvalFilePrefix) {
if strings.HasSuffix(path, dateSuffix) {
upToDate[filepath.Base(path)] = true
upToDate[filepath.Base(path)] = struct{}{}
} else {
err := os.Remove(path)
if err != nil {
@@ -90,6 +95,55 @@ func removeOldDefs(date time.Time, path string) (map[string]bool, error) {
return upToDate, nil
}
// listUpToDateDefs walks 'path' returning the set of OVAL definition filenames that match
// 'date'. Unlike removeOldDefs, it does NOT delete outdated files. Use this when the
// outdated files may still be needed as a fallback (e.g., when a fresh sync might fail).
func listUpToDateDefs(date time.Time, path string) (map[string]struct{}, error) {
dateSuffix := fmt.Sprintf("-%d_%02d_%02d.json", date.Year(), date.Month(), date.Day())
upToDate := make(map[string]struct{})
err := filepath.WalkDir(path, func(p string, d os.DirEntry, err error) error {
if err != nil {
return err
}
if strings.HasPrefix(filepath.Base(p), OvalFilePrefix) && strings.HasSuffix(p, dateSuffix) {
upToDate[filepath.Base(p)] = struct{}{}
}
return nil
})
if err != nil {
return nil, err
}
return upToDate, nil
}
// removeOutdatedDefs walks 'path' removing any OVAL definition files that do not match 'date'.
// Should be called only after a successful Sync so that yesterday's files remain available
// when today's download fails.
func removeOutdatedDefs(date time.Time, path string) error {
dateSuffix := fmt.Sprintf("-%d_%02d_%02d.json", date.Year(), date.Month(), date.Day())
root, err := os.OpenRoot(path)
if err != nil {
return err
}
defer func() { _ = root.Close() }()
return fs.WalkDir(root.FS(), ".", func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !strings.HasPrefix(filepath.Base(p), OvalFilePrefix) {
return nil
}
if strings.HasSuffix(p, dateSuffix) {
return nil
}
return root.Remove(p)
})
}
// Sync syncs the oval definitions for one or more platforms.
// If 'platforms' is nil, then all supported platforms will be synched.
func Sync(dstDir string, platforms []Platform) error {
@@ -127,8 +181,10 @@ func Sync(dstDir string, platforms []Platform) error {
return nil
}
// Refresh checks all local OVAL artifacts contained in 'vulnPath' deleting the old and downloading
// any missing definitions based on today's date and all the hosts' platforms/os versions contained in 'osVersions'.
// Refresh checks all local OVAL artifacts contained in 'vulnPath' and downloads any missing
// definitions based on today's date and the hosts' platforms/os versions contained in 'osVersions'.
// Outdated (non-today) definition files are only removed AFTER a successful sync, so that a
// failed sync leaves yesterday's files in place as a fallback.
// Returns a slice of Platforms of the newly downloaded OVAL files.
func Refresh(
ctx context.Context,
@@ -137,18 +193,23 @@ func Refresh(
) ([]Platform, error) {
now := time.Now()
existing, err := removeOldDefs(now, vulnPath)
existing, err := listUpToDateDefs(now, vulnPath)
if err != nil {
return nil, err
}
toDownload := whatToDownload(versions, existing, now)
if len(toDownload) > 0 {
err = Sync(vulnPath, toDownload)
if err != nil {
if err := Sync(vulnPath, toDownload); err != nil {
// Sync failed — leave outdated files on disk so the analyzer can fall back to them.
return nil, err
}
}
// Sync succeeded (or nothing needed to be downloaded). Now safe to remove outdated files.
if err := removeOutdatedDefs(now, vulnPath); err != nil {
return toDownload, fmt.Errorf("removing outdated OVAL definitions: %w", err)
}
return toDownload, nil
}
+50 -2
View File
@@ -83,8 +83,8 @@ func TestSync(t *testing.T) {
},
}
existing := map[string]bool{
NewPlatform("ubuntu", "Ubuntu 20.4.0").ToFilename(today, "json"): true,
existing := map[string]struct{}{
NewPlatform("ubuntu", "Ubuntu 20.4.0").ToFilename(today, "json"): {},
}
r := whatToDownload(&osVersions, existing, today)
@@ -92,4 +92,52 @@ func TestSync(t *testing.T) {
require.Contains(t, r, NewPlatform("ubuntu", "Ubuntu 18.4.0"))
require.NotContains(t, r, NewPlatform("rhle", "CentOS Linux 8.3.2011"))
})
t.Run("#listUpToDateDefs leaves outdated files in place", func(t *testing.T) {
ovalPlatform := NewPlatform("ubuntu", "Ubuntu 20.4.0")
today := time.Now()
yesterday := today.Add(-24 * time.Hour)
path := t.TempDir()
todayFile := filepath.Join(path, ovalPlatform.ToFilename(today, "json"))
yesterdayFile := filepath.Join(path, ovalPlatform.ToFilename(yesterday, "json"))
require.NoError(t, os.WriteFile(todayFile, []byte("{}"), 0o644))
require.NoError(t, os.WriteFile(yesterdayFile, []byte("{}"), 0o644))
upToDate, err := listUpToDateDefs(today, path)
require.NoError(t, err)
require.Contains(t, upToDate, filepath.Base(todayFile))
// Both files must still exist — listing should be read-only.
_, err = os.Stat(todayFile)
require.NoError(t, err)
_, err = os.Stat(yesterdayFile)
require.NoError(t, err)
})
// Regression test for https://github.com/fleetdm/fleet/issues/45602:
// When Sync fails, yesterday's definition files must remain on disk so the analyzer
// can fall back to them instead of deleting every existing vulnerability for the platform.
t.Run("#removeOutdatedDefs only runs after successful sync", func(t *testing.T) {
ovalPlatform := NewPlatform("ubuntu", "Ubuntu 20.4.0")
today := time.Now()
yesterday := today.Add(-24 * time.Hour)
path := t.TempDir()
yesterdayFile := filepath.Join(path, ovalPlatform.ToFilename(yesterday, "json"))
require.NoError(t, os.WriteFile(yesterdayFile, []byte("{}"), 0o644))
// listUpToDateDefs alone must not remove yesterday's file.
_, err := listUpToDateDefs(today, path)
require.NoError(t, err)
_, err = os.Stat(yesterdayFile)
require.NoError(t, err, "yesterday's file must remain after listUpToDateDefs")
// removeOutdatedDefs removes yesterday's file only when invoked explicitly
// (i.e. after a successful Sync).
require.NoError(t, removeOutdatedDefs(today, path))
_, err = os.Stat(yesterdayFile)
require.True(t, os.IsNotExist(err), "yesterday's file should be removed after removeOutdatedDefs")
})
}
@@ -3,6 +3,7 @@ package winoffice
import (
"context"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
@@ -223,6 +224,20 @@ func Analyze(
return nil, nil
}
// Refuse to proceed if the loaded bulletin contains no version data or no security updates.
// An empty bulletin would cause every existing WinOffice vulnerability for matching software
// to be marked as remediated. This usually indicates the bulletin file is corrupted.
hasSecurityUpdates := false
for _, vb := range bulletin.Versions {
if vb != nil && len(vb.SecurityUpdates) > 0 {
hasSecurityUpdates = true
break
}
}
if !hasSecurityUpdates {
return nil, errors.New("WinOffice bulletin contains no security updates (possible corrupted feed)")
}
// Query for Windows Office software from "programs" source.
// Use NameMatch/NameExclude to filter at the database level.
queryParams := fleet.SoftwareIterQueryOptions{
@@ -1,7 +1,9 @@
package winoffice
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -236,3 +238,25 @@ func TestCheckVersionResolvedVersionPointer(t *testing.T) {
assert.Equal(t, "16.0.19725.20200", *vulns[0].ResolvedInVersion)
assert.Equal(t, uint(0), vulns[0].SoftwareID)
}
// TestAnalyzeRejectsEmptyBulletin is a regression test for
// https://github.com/fleetdm/fleet/issues/45602: a bulletin with no security updates
// (e.g., a corrupted artifact) must NOT cause every existing WinOffice vulnerability
// to be marked as remediated. Analyze should return an error in that case so the cron
// skips the analysis (and therefore the deletes that would happen during it).
func TestAnalyzeRejectsEmptyBulletin(t *testing.T) {
vulnPath := t.TempDir()
// Serialize a bulletin where every version branch has no security updates.
bulletin := &BulletinFile{
Version: 1,
BuildPrefixes: map[string]string{"19725": "2602"},
Versions: map[string]*VersionBulletin{
"2602": {SecurityUpdates: nil},
},
}
require.NoError(t, bulletin.Serialize(time.Now(), vulnPath))
_, err := Analyze(context.Background(), nil, vulnPath, false)
require.Error(t, err)
require.Contains(t, err.Error(), "no security updates")
}