diff --git a/changes/45602-vuln-corrupted-download b/changes/45602-vuln-corrupted-download new file mode 100644 index 0000000000..88b6687d46 --- /dev/null +++ b/changes/45602-vuln-corrupted-download @@ -0,0 +1 @@ +- Fixed corrupted vulnerabilities download removing existing detections diff --git a/server/vulnerabilities/goval_dictionary/analyzer.go b/server/vulnerabilities/goval_dictionary/analyzer.go index 1c26cd319b..5e7e60f8a1 100644 --- a/server/vulnerabilities/goval_dictionary/analyzer.go +++ b/server/vulnerabilities/goval_dictionary/analyzer.go @@ -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 } diff --git a/server/vulnerabilities/macoffice/analyzer.go b/server/vulnerabilities/macoffice/analyzer.go index 9089e2b26c..8bb9ee0206 100644 --- a/server/vulnerabilities/macoffice/analyzer.go +++ b/server/vulnerabilities/macoffice/analyzer.go @@ -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 { diff --git a/server/vulnerabilities/macoffice/analyzer_test.go b/server/vulnerabilities/macoffice/analyzer_test.go index 81b410189e..f4b9cef302 100644 --- a/server/vulnerabilities/macoffice/analyzer_test.go +++ b/server/vulnerabilities/macoffice/analyzer_test.go @@ -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) { diff --git a/server/vulnerabilities/msrc/analyzer.go b/server/vulnerabilities/msrc/analyzer.go index f502e36b86..a276ceda9a 100644 --- a/server/vulnerabilities/msrc/analyzer.go +++ b/server/vulnerabilities/msrc/analyzer.go @@ -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) diff --git a/server/vulnerabilities/msrc/analyzer_test.go b/server/vulnerabilities/msrc/analyzer_test.go index 75df9411f0..9b4ce807c1 100644 --- a/server/vulnerabilities/msrc/analyzer_test.go +++ b/server/vulnerabilities/msrc/analyzer_test.go @@ -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) { diff --git a/server/vulnerabilities/nvd/cve.go b/server/vulnerabilities/nvd/cve.go index 06ad75b12e..f2005718b2 100644 --- a/server/vulnerabilities/nvd/cve.go +++ b/server/vulnerabilities/nvd/cve.go @@ -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 } diff --git a/server/vulnerabilities/osv/analyzer.go b/server/vulnerabilities/osv/analyzer.go index 1d9f2a80ff..3b77598d61 100644 --- a/server/vulnerabilities/osv/analyzer.go +++ b/server/vulnerabilities/osv/analyzer.go @@ -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, diff --git a/server/vulnerabilities/osv/analyzer_test.go b/server/vulnerabilities/osv/analyzer_test.go index 36bf715e47..820b06e5b9 100644 --- a/server/vulnerabilities/osv/analyzer_test.go +++ b/server/vulnerabilities/osv/analyzer_test.go @@ -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() diff --git a/server/vulnerabilities/oval/analyzer.go b/server/vulnerabilities/oval/analyzer.go index 263acea21c..cc8a7eed75 100644 --- a/server/vulnerabilities/oval/analyzer.go +++ b/server/vulnerabilities/oval/analyzer.go @@ -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 } diff --git a/server/vulnerabilities/oval/analyzer_test.go b/server/vulnerabilities/oval/analyzer_test.go index e19f1cb14d..fd65f97761 100644 --- a/server/vulnerabilities/oval/analyzer_test.go +++ b/server/vulnerabilities/oval/analyzer_test.go @@ -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") + } + }) }) } diff --git a/server/vulnerabilities/oval/sync.go b/server/vulnerabilities/oval/sync.go index 87b1f67d0b..14c1cce058 100644 --- a/server/vulnerabilities/oval/sync.go +++ b/server/vulnerabilities/oval/sync.go @@ -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 } diff --git a/server/vulnerabilities/oval/sync_test.go b/server/vulnerabilities/oval/sync_test.go index 8af3412973..19492e6dd0 100644 --- a/server/vulnerabilities/oval/sync_test.go +++ b/server/vulnerabilities/oval/sync_test.go @@ -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") + }) } diff --git a/server/vulnerabilities/winoffice/analyzer.go b/server/vulnerabilities/winoffice/analyzer.go index ab9d93f63c..53203e9474 100644 --- a/server/vulnerabilities/winoffice/analyzer.go +++ b/server/vulnerabilities/winoffice/analyzer.go @@ -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{ diff --git a/server/vulnerabilities/winoffice/analyzer_test.go b/server/vulnerabilities/winoffice/analyzer_test.go index a11351c8d2..5ea932e16a 100644 --- a/server/vulnerabilities/winoffice/analyzer_test.go +++ b/server/vulnerabilities/winoffice/analyzer_test.go @@ -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") +}