Fix OS versions not populated in vulnerability details - #42759
Conversation
Fixes issue where `os_versions` array is empty in vulnerability details for OS-only vulnerabilities (e.g., macOS CVEs). This happened because `OSVersionsByCVE()` intersects `operating_system_vulnerabilities` with `aggregated_stats`, but the latter was only updated by the hourly cleanups cron, creating up to 1 hour lag after vulnerability scanning. By adding `UpdateOSVersions` to the vulnerability cron, the aggregated stats are refreshed immediately after vulnerability scanning completes. The operation remains in the cleanups cron as well for cases where vulnerability scanning is disabled.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThis change addresses an issue where OS versions were not populated in vulnerability details for OS-only vulnerabilities like macOS CVEs. The fix adds a new vulnerability-processing step to the vulnerability processing pipeline that calls Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/fleet/vuln_process.go (1)
198-205: Good placement forupdate_os_versions; add explicit runtime observability.This ordering is correct for reducing stale
os_versionsin CVE details right after vuln processing. SinceUpdateOSVersionsis a full aggregation, please add/confirm a dedicated metric (duration + failures) forvuln.update_os_versionsso DB pressure/regressions are visible early.Based on learnings: In
cmd/fleet/cron.go, cleanup sequencing after vulnerability writes is deliberate to avoid lock contention withsoftware_cveandoperating_system_vulnerabilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/fleet/vuln_process.go` around lines 198 - 205, Wrap the call to ds.UpdateOSVersions (inside the vuln.update_os_versions tracer span in vuln_process.go) with explicit runtime observability: record a duration metric (e.g., vuln.update_os_versions.duration_seconds as a histogram/summary) starting just after tracer.Start and observe it when the call returns, and increment a failure counter (e.g., vuln.update_os_versions.failures_total) if UpdateOSVersions returns an error; ensure you use the project’s existing metrics helper/registry and include the error branch to increment the failure metric before returning the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/fleet/vuln_process.go`:
- Around line 198-205: Wrap the call to ds.UpdateOSVersions (inside the
vuln.update_os_versions tracer span in vuln_process.go) with explicit runtime
observability: record a duration metric (e.g.,
vuln.update_os_versions.duration_seconds as a histogram/summary) starting just
after tracer.Start and observe it when the call returns, and increment a failure
counter (e.g., vuln.update_os_versions.failures_total) if UpdateOSVersions
returns an error; ensure you use the project’s existing metrics helper/registry
and include the error branch to increment the failure metric before returning
the error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 455f01c0-b22d-4624-8ecf-f76b34ce4fb8
📒 Files selected for processing (2)
changes/40581-os-versions-vuln-detailscmd/fleet/vuln_process.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42759 +/- ##
==========================================
+ Coverage 66.72% 66.79% +0.07%
==========================================
Files 2536 2536
Lines 203587 203855 +268
Branches 9077 9077
==========================================
+ Hits 135834 136160 +326
+ Misses 55461 55349 -112
- Partials 12292 12346 +54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
This PR addresses a lag where OS-only vulnerability details could show an empty os_versions array by ensuring OS version aggregated stats are refreshed as part of the vulnerabilities schedule.
Changes:
- Adds an
update_os_versionsjob to the vulnerability processing schedule (getVulnFuncs) to refreshaggregated_stats. - Adds a release-note entry describing the fix for OS-only vulnerability details.
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/fleet/vuln_process.go | Adds UpdateOSVersions as a scheduled vulnerability-processing step (used by both cron scheduling and the vuln_processing command). |
| changes/40581-os-versions-vuln-details | Release note documenting the OS versions population fix for OS-only vulnerabilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cronVulnerabilities calls ds.OSVersions() to get the list of OS versions to scan. This reads from aggregated_stats, so UpdateOSVersions must run first to ensure fresh data is available for the vulnerability scanner.
Summary
UpdateOSVersionsto the vulnerability cron soaggregated_statsis refreshed immediately after vulnerability scanning completesos_versionsarray is empty when viewing OS-only vulnerabilities (e.g., macOS CVEs)Resolves #40581
Root Cause
OSVersionsByCVE()intersectsoperating_system_vulnerabilities(populated by vuln cron) withaggregated_stats(populated byUpdateOSVersionsin cleanups cron). The cleanups cron runs hourly, creating up to 1 hour lag whereos_versionsdata is stale/empty.Summary by CodeRabbit