Fix fleetctl vulnerability-data-stream to download OSV data#44260
Merged
mostlikelee merged 4 commits intomainfrom Apr 30, 2026
Merged
Fix fleetctl vulnerability-data-stream to download OSV data#44260mostlikelee merged 4 commits intomainfrom
fleetctl vulnerability-data-stream to download OSV data#44260mostlikelee merged 4 commits intomainfrom
Conversation
Adds OSV (Ubuntu and RHEL) artifact downloads to the `fleetctl vulnerability-data-stream` command. Previously the command downloaded CPE, NVD, EPSS, CISA, OVAL, MSRC, and MacOffice feeds but silently skipped OSV, leaving operators who pre-seed the vuln directory without DB access (e.g. air-gapped) missing the artifacts that drive Ubuntu and RHEL vulnerability scanning. Adds an `osv.RefreshAll` entry point that downloads every Ubuntu and RHEL artifact in the latest osv-processor release (no host-inventory filtering), and wires it into `vulnerabilityDataStreamCommand`.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #44260 +/- ##
==========================================
+ Coverage 66.73% 66.83% +0.09%
==========================================
Files 2627 2631 +4
Lines 211192 211509 +317
Branches 9505 9505
==========================================
+ Hits 140938 141355 +417
+ Misses 57466 57305 -161
- Partials 12788 12849 +61
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:
|
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
WalkthroughThis PR adds OSV (Open Source Vulnerability) artifact downloading to 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.go (1)
21-31: Assert the OSV artifacts on disk, not just the new log line.This update only proves the CLI printed
Downloading OSV artifacts... Done. The test still passes ifosv.RefreshAllbecomes a no-op after logging success, which would miss the main regression this PR is fixing. Please add at least one filesystem assertion for bothosv-ubuntu-*.json.gzandosv-rhel-*.json.gz.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.go` around lines 21 - 31, The test currently only asserts the CLI log line but not that osv.RefreshAll actually wrote files; after invoking the code that triggers downloads in vulnerability_data_stream_test.go (the test's command run), add filesystem assertions that at least one file matching "osv-ubuntu-*.json.gz" and one matching "osv-rhel-*.json.gz" exist in the directory used for OSV artifacts (use the same test directory or dataDir variable the test uses), e.g. use filepath.Glob to find matches and require that len(matches) > 0 or os.Stat to verify a file exists; this guarantees osv.RefreshAll produced the expected artifacts rather than just logging success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/vulnerabilities/osv/sync.go`:
- Around line 163-178: The current calls to SyncOSV and syncRHELOSV only check
the returned error but ignore partial failures reported in result.Failed; update
the RefreshAll call site to treat any per-version failures as a hard error:
after calling SyncOSV or syncRHELOSV, if result.Failed is non-empty return the
accumulated downloaded list and an error describing which versions failed
(include result.Failed contents and contextual text like "failed to download OSV
for versions"). Keep references to SyncOSV, syncRHELOSV, and the
result.Downloaded/result.Failed fields so the change is applied to both Ubuntu
and RHEL branches.
---
Nitpick comments:
In `@cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.go`:
- Around line 21-31: The test currently only asserts the CLI log line but not
that osv.RefreshAll actually wrote files; after invoking the code that triggers
downloads in vulnerability_data_stream_test.go (the test's command run), add
filesystem assertions that at least one file matching "osv-ubuntu-*.json.gz" and
one matching "osv-rhel-*.json.gz" exist in the directory used for OSV artifacts
(use the same test directory or dataDir variable the test uses), e.g. use
filepath.Glob to find matches and require that len(matches) > 0 or os.Stat to
verify a file exists; this guarantees osv.RefreshAll produced the expected
artifacts rather than just logging success.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d4a61014-ac0b-4b88-8068-784e3dac2159
📒 Files selected for processing (5)
changes/44242-fleetctl-vuln-data-stream-osvcmd/fleetctl/fleetctl/vulnerability_data_stream.gocmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.goserver/vulnerabilities/osv/sync.goserver/vulnerabilities/osv/sync_test.go
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes fleetctl vulnerability-data-stream so it also pre-seeds OSV (Ubuntu + RHEL) vulnerability artifacts, aligning the CLI’s “air-gapped preseed” behavior with what the server-side vulnerability cron expects.
Changes:
- Add
osv.RefreshAllto download all OSV artifacts from the latest release without relying on host inventory/DB access. - Extend OSV sync helpers to derive versions + release date from release asset names.
- Update
fleetctlintegration test expected output and add unit tests covering the new OSV helper behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
server/vulnerabilities/osv/sync.go |
Adds RefreshAll and helpers to download all OSV artifacts from the latest release. |
server/vulnerabilities/osv/sync_test.go |
Adds tests for partial-failure behavior and new release parsing helpers. |
cmd/fleetctl/fleetctl/vulnerability_data_stream.go |
Invokes osv.RefreshAll as part of vulnerability-data-stream. |
cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.go |
Updates expected CLI output to include OSV download step. |
changes/44242-fleetctl-vuln-data-stream-osv |
Adds changelog entry for the user-visible fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
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.
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/vulnerabilities/osv/sync.go (2)
163-184: Prefer aggregating Ubuntu/RHEL failures instead of early-returning after Ubuntu.Line 170 currently exits before attempting RHEL sync when Ubuntu has partial failures. Continuing both syncs and returning a combined error would maximize artifacts written in a single run while still failing the command.
Proposed refactor
var downloaded []string + var failures []string if len(ubuntuVers) > 0 { result, err := SyncOSV(ctx, vulnPath, ubuntuVers, releaseDate, release) if err != nil { return downloaded, fmt.Errorf("syncing Ubuntu OSV artifacts: %w", err) } downloaded = append(downloaded, result.Downloaded...) if len(result.Failed) > 0 { - return downloaded, fmt.Errorf("failed to download OSV for Ubuntu versions: %v", result.Failed) + failures = append(failures, fmt.Sprintf("Ubuntu: %v", result.Failed)) } } if len(rhelVers) > 0 { result, err := syncRHELOSV(ctx, vulnPath, rhelVers, releaseDate, release) if err != nil { return downloaded, fmt.Errorf("syncing RHEL OSV artifacts: %w", err) } downloaded = append(downloaded, result.Downloaded...) if len(result.Failed) > 0 { - return downloaded, fmt.Errorf("failed to download OSV for RHEL versions: %v", result.Failed) + failures = append(failures, fmt.Sprintf("RHEL: %v", result.Failed)) } } + if len(failures) > 0 { + return downloaded, fmt.Errorf("failed to download OSV artifacts for %s", strings.Join(failures, "; ")) + } + return downloaded, nil }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/vulnerabilities/osv/sync.go` around lines 163 - 184, The current flow in SyncOSV/syncRHELOSV handling returns early when Ubuntu downloads have failures, preventing RHEL sync from running; change the logic in the block that calls SyncOSV and syncRHELOSV (symbols: SyncOSV, syncRHELOSV, downloaded, result.Downloaded, result.Failed) to always attempt both syncs, append each result.Downloaded to downloaded, collect any result.Failed entries from both calls into a single failures slice, and after both calls complete, if failures is non-empty return downloaded with a combined error listing all failed versions; preserve existing error wrapping for fatal call errors but do not early-return on partial failures.
192-206: Consider dedup + stable sort of extracted versions for deterministic behavior.
versionsFromReleaseiterates a map, so output order is non-deterministic and may include duplicates if release assets evolve. Deduping and sorting makes logs/errors/tests more stable.Proposed refactor
import ( "context" "fmt" "os" "path/filepath" + "sort" "strings" "time" @@ func versionsFromRelease(release *ReleaseInfo) (ubuntu []string, rhel []string) { + ubuntuSet := make(map[string]struct{}) + rhelSet := make(map[string]struct{}) + for assetName := range release.Assets { switch { case strings.HasPrefix(assetName, OSVFilePrefix): if v := versionFromAssetName(assetName, OSVFilePrefix); v != "" { - ubuntu = append(ubuntu, v) + ubuntuSet[v] = struct{}{} } case strings.HasPrefix(assetName, OSVRHELFilePrefix): if v := versionFromAssetName(assetName, OSVRHELFilePrefix); v != "" { - rhel = append(rhel, v) + rhelSet[v] = struct{}{} } } } + + for v := range ubuntuSet { + ubuntu = append(ubuntu, v) + } + for v := range rhelSet { + rhel = append(rhel, v) + } + sort.Strings(ubuntu) + sort.Strings(rhel) + return ubuntu, rhel }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/vulnerabilities/osv/sync.go` around lines 192 - 206, versionsFromRelease currently collects versions by iterating ReleaseInfo.Assets (a map) which yields non-deterministic ordering and may produce duplicates; change it to deduplicate and deterministically sort the ubuntu and rhel slices before returning: use a small set (map[string]struct{}) when calling versionFromAssetName for OSVFilePrefix and OSVRHELFilePrefix to collect unique versions, then convert each set to a slice and apply a stable sort (e.g., sort.Strings) before returning; keep the existing function names (versionsFromRelease, versionFromAssetName) and asset prefix checks (OSVFilePrefix, OSVRHELFilePrefix) to locate where to implement this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/vulnerabilities/osv/sync.go`:
- Around line 163-184: The current flow in SyncOSV/syncRHELOSV handling returns
early when Ubuntu downloads have failures, preventing RHEL sync from running;
change the logic in the block that calls SyncOSV and syncRHELOSV (symbols:
SyncOSV, syncRHELOSV, downloaded, result.Downloaded, result.Failed) to always
attempt both syncs, append each result.Downloaded to downloaded, collect any
result.Failed entries from both calls into a single failures slice, and after
both calls complete, if failures is non-empty return downloaded with a combined
error listing all failed versions; preserve existing error wrapping for fatal
call errors but do not early-return on partial failures.
- Around line 192-206: versionsFromRelease currently collects versions by
iterating ReleaseInfo.Assets (a map) which yields non-deterministic ordering and
may produce duplicates; change it to deduplicate and deterministically sort the
ubuntu and rhel slices before returning: use a small set (map[string]struct{})
when calling versionFromAssetName for OSVFilePrefix and OSVRHELFilePrefix to
collect unique versions, then convert each set to a slice and apply a stable
sort (e.g., sort.Strings) before returning; keep the existing function names
(versionsFromRelease, versionFromAssetName) and asset prefix checks
(OSVFilePrefix, OSVRHELFilePrefix) to locate where to implement this.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a50c347c-5c61-41cc-9e6c-37419c4f4ec5
📒 Files selected for processing (3)
cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.goserver/vulnerabilities/osv/sync.goserver/vulnerabilities/osv/sync_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/fleetctl/integrationtest/vuln/vulnerability_data_stream_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue: Resolves #44242
Summary
fleetctl vulnerability-data-streamis documented as the way to pre-seed a Fleet server's vulnerability directory (common in air-gapped or restricted-egress deployments). It already downloads CPE, NVD, EPSS, CISA, OVAL, MSRC, and MacOffice feeds, but it never downloaded OSV (Ubuntu / RHEL) artifacts even though the server's vulnerability cron uses them. Operators ended up with a directory that was missing the artifacts powering Ubuntu (and, withRHELOSVForVulnerabilities, RHEL) vulnerability scanning.Checklist for submitter
changes/.SELECT *is avoided, SQL injection is prevented, JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.fleethttp.NewClientand the existingSyncOSVflow which already handles per-asset failure isolation.Testing
server/vulnerabilities/osv/sync_test.gocover the new helpers across happy-path, empty, and malformed-input cases.fleetctl vulnerability-data-stream --dir /tmp/fleet-vulnagainst a clean dir and confirmosv-ubuntu-*.json.gzandosv-rhel-*.json.gzfiles appear alongside the existing CPE/NVD/OVAL/MSRC/MacOffice artifacts.For unreleased bug fixes in a release candidate, one of:
Summary by CodeRabbit
New Features
Tests