Add source-agnostic RHEL vulnerability integration tests - #43185
Conversation
Adds integration tests for RHEL vulnerability scanning that are decoupled from the data source (OVAL/OSV) via a Scanner abstraction. The same software fixtures and per-package CVE mappings can verify both OVAL today and OSV after migration without modifying test data. - Scanner type abstracts setup + analyze so tests are parameterized by scanner - OVALScanner and GovalDictionaryScanner wrap existing analyzers - Software fixtures derived from OVAL feeds (2722 packages for RHEL 8, 2594 for RHEL 9) - Per-package CVE assertions verify correct attribution (not just total count) - No changes to existing oval/ or goval_dictionary/ packages
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughIntroduces vulnerability scanning integration test infrastructure comprising a test helper package ( Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
server/vulnerabilities/vulntest/gen_fixture_test.go (2)
245-248: Externalbzip2dependency for compression.Shelling out to
bzip2requires the binary to be installed. This is reasonable since Go'scompress/bzip2only supports reading, not writing. Consider documenting this dependency in the test comment or adding a check for the binary's existence with a helpful error message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/vulnerabilities/vulntest/gen_fixture_test.go` around lines 245 - 248, The test currently shells out to the external bzip2 binary via exec.Command("bzip2", "-f", jsonPath) and will fail if bzip2 isn't installed; update the test to check for the binary before invoking it and give a clear error message (e.g., use exec.LookPath("bzip2") or similar) and/or add a test comment documenting that bzip2 is required; reference the existing usage of exec.Command, jsonPath and sys.outPath so the check runs before calling exec.Command and the error message mentions bzip2 and the affected test artifact paths.
99-107: Edge case: decrement logic may produce unexpected results for certain release strings.The decrement logic only handles digits '1'-'9' and skips leading zeros followed by non-dots. If a release string has all zeros or starts with '0' followed by a digit (e.g.,
"0.1.el9"), the logic may not decrement correctly and could return the original release unchanged.This is acceptable for fixture generation since OVAL data typically has normal release numbers, but worth documenting or adding a fallback.
server/vulnerabilities/vulntest/vulntest.go (1)
103-153: Consider extracting shared host creation logic to reduce duplication.
LoadSoftwareFromFixtureandLoadSoftwareshare identical host creation and CPE upsert logic. Consider extracting a shared helper for host creation that accepts a software slice.♻️ Suggested refactor to reduce duplication
// createHostWithSoftware creates a host and populates it with the given software. func createHostWithSoftware( ds *mysql.Datastore, platformStr string, ver fleet.OSVersion, software []fleet.Software, t require.TestingT, ) *fleet.Host { osqueryHostID, err := server.GenerateRandomText(10) require.NoError(t, err) ctx := context.Background() h, err := ds.NewHost(ctx, &fleet.Host{ Hostname: platformStr, NodeKey: ptr.String(platformStr), UUID: platformStr, DetailUpdatedAt: time.Now(), LabelUpdatedAt: time.Now(), PolicyUpdatedAt: time.Now(), SeenTime: time.Now(), OsqueryHostID: &osqueryHostID, Platform: ver.Platform, OSVersion: ver.Name, }) require.NoError(t, err) _, err = ds.UpdateHostSoftware(ctx, h.ID, software) require.NoError(t, err) err = ds.LoadHostSoftware(ctx, h, false) require.NoError(t, err) var cpes []fleet.SoftwareCPE for _, s := range h.Software { cpes = append(cpes, fleet.SoftwareCPE{SoftwareID: s.ID, CPE: fmt.Sprintf("%s-%s", s.Name, s.Version)}) } _, err = ds.UpsertSoftwareCPEs(ctx, cpes) require.NoError(t, err) return h }Then
LoadSoftwareFromFixtureandLoadSoftwarecan delegate to this helper.Also applies to: 231-289
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/vulnerabilities/vulntest/vulntest.go` around lines 103 - 153, LoadSoftwareFromFixture and LoadSoftware duplicate host-creation, software update, LoadHostSoftware and CPE upsert logic; extract that shared logic into a helper (e.g., createHostWithSoftware) that takes (ds *mysql.Datastore, platformStr string, ver fleet.OSVersion, software []fleet.Software, t require.TestingT) and inside it call server.GenerateRandomText, ds.NewHost, ds.UpdateHostSoftware, ds.LoadHostSoftware and ds.UpsertSoftwareCPEs to populate CPEs; then have LoadSoftwareFromFixture and LoadSoftware build their software slice (or convert fixture output) and delegate to createHostWithSoftware to remove duplication.
🤖 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/vulntest/gen_fixture_test.go`:
- Around line 38-50: Remove the unused file handle: delete the os.Open(defPath)
call and its associated variable f and defer f.Close() in gen_fixture_test.go,
since ExtractBzip2(defPath, tmpJSON, t) reads the file itself; ensure defPath,
tmpJSON, ovalDef and the subsequent ReadFile/Unmarshal remain unchanged (or if
you intended to assert the file exists, replace the os.Open with
os.Stat(defPath) instead).
---
Nitpick comments:
In `@server/vulnerabilities/vulntest/gen_fixture_test.go`:
- Around line 245-248: The test currently shells out to the external bzip2
binary via exec.Command("bzip2", "-f", jsonPath) and will fail if bzip2 isn't
installed; update the test to check for the binary before invoking it and give a
clear error message (e.g., use exec.LookPath("bzip2") or similar) and/or add a
test comment documenting that bzip2 is required; reference the existing usage of
exec.Command, jsonPath and sys.outPath so the check runs before calling
exec.Command and the error message mentions bzip2 and the affected test artifact
paths.
In `@server/vulnerabilities/vulntest/vulntest.go`:
- Around line 103-153: LoadSoftwareFromFixture and LoadSoftware duplicate
host-creation, software update, LoadHostSoftware and CPE upsert logic; extract
that shared logic into a helper (e.g., createHostWithSoftware) that takes (ds
*mysql.Datastore, platformStr string, ver fleet.OSVersion, software
[]fleet.Software, t require.TestingT) and inside it call
server.GenerateRandomText, ds.NewHost, ds.UpdateHostSoftware,
ds.LoadHostSoftware and ds.UpsertSoftwareCPEs to populate CPEs; then have
LoadSoftwareFromFixture and LoadSoftware build their software slice (or convert
fixture output) and delegate to createHostWithSoftware to remove duplication.
🪄 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: 6fbf6d86-aad0-4299-8b0f-13ee05743f2f
⛔ Files ignored due to path filters (4)
server/vulnerabilities/testdata/rhel/2026/rhel_08-oval_def.json.bz2is excluded by!**/*.bz2server/vulnerabilities/testdata/rhel/2026/rhel_09-oval_def.json.bz2is excluded by!**/*.bz2server/vulnerabilities/testdata/rhel/software/0810/rhel_08-vulns.json.bz2is excluded by!**/*.bz2server/vulnerabilities/testdata/rhel/software/0904/rhel_09-vulns.json.bz2is excluded by!**/*.bz2
📒 Files selected for processing (4)
server/vulnerabilities/vulntest/gen_fixture_test.goserver/vulnerabilities/vulntest/scanners.goserver/vulnerabilities/vulntest/vulnerability_scanning_test.goserver/vulnerabilities/vulntest/vulntest.go
| @@ -0,0 +1,253 @@ | |||
| package vulntest_test | |||
There was a problem hiding this comment.
this is the fixture generator. I expect we'll remove this once we feel confident that the OSV transition is stable. A deep review is probably not needed here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43185 +/- ##
==========================================
+ Coverage 66.86% 66.89% +0.02%
==========================================
Files 2578 2588 +10
Lines 206869 207518 +649
Branches 9283 9283
==========================================
+ Hits 138328 138824 +496
- Misses 55978 56071 +93
- Partials 12563 12623 +60
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.
Pull request overview
This PR introduces a new server/vulnerabilities/vulntest helper package to run source-agnostic RHEL vulnerability scanning integration tests (OVAL now, OSV later) using large, feed-derived fixtures.
Changes:
- Added a
Scannerabstraction plus shared fixture loading + assertion helpers for vulnerability integration tests. - Added table-driven RHEL integration tests that run against a scanner implementation (currently OVAL) and verify per-package CVE mappings.
- Added a gated fixture generator test and new large bzip2-compressed fixtures for RHEL 8.10 and 9.4.
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/vulnerabilities/vulntest/vulntest.go | New shared helpers: scanner interface, fixture formats, host/software loading, and assertion logic. |
| server/vulnerabilities/vulntest/scanners.go | Scanner implementations wrapping existing OVAL and goval-dictionary analyzers. |
| server/vulnerabilities/vulntest/vulnerability_scanning_test.go | New integration tests for RHEL package + kernel vulnerability scanning using the scanner abstraction. |
| server/vulnerabilities/vulntest/gen_fixture_test.go | Gated fixture generation test to derive vulnerable package fixtures from OVAL definitions. |
| server/vulnerabilities/testdata/rhel/software/0810/rhel_08-vulns.json.bz2 | New large RHEL 8.10 software→CVE fixture. |
| server/vulnerabilities/testdata/rhel/software/0904/rhel_09-vulns.json.bz2 | New large RHEL 9.4 software→CVE fixture. |
💡 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.
Resolves #43182
Summary
server/vulnerabilities/vulntest/package with aScannerabstraction that decouples integration tests from the vulnerability data source (OVAL/OSV)oval/orgoval_dictionary/packagesChanges
New package:
server/vulnerabilities/vulntest/vulntest.goScannertype,VulnFixture(software→CVE mapping),RunAndAssertwith per-package assertions, legacy helpers for existingoval/analyzer_test.goscanners.goOVALScanner()andGovalDictionaryScanner()factory functions that wrap the existing analyzers behind theScannerinterfacevulnerability_scanning_test.gogen_fixture_test.goGENERATE_FIXTURES=1) that extracts vulnerable packages from OVAL definitions and captures per-package CVE mappingsNew test fixtures
testdata/rhel/2026/rhel_08-oval_def.json.bz2testdata/rhel/2026/rhel_09-oval_def.json.bz2testdata/rhel/software/0810/rhel_08-vulns.json.bz2testdata/rhel/software/0904/rhel_09-vulns.json.bz2How OSV plugs in later
When RHEL OSV is implemented, the only changes needed are:
testdata/rhel/osv/OSVScanner()factory inscanners.govulntest.OSVScanner()in the test's scanners sliceSame fixtures, same assertions — zero test data changes.
Test plan
MYSQL_TEST=1 go test -run TestRHELVulnerabilityScanning ./server/vulnerabilities/vulntest/...— verifies per-package CVE mappings for RHEL 8.10 and 9.4MYSQL_TEST=1 go test -run TestRHELKernelVulnerabilities ./server/vulnerabilities/vulntest/...— verifies goval-dictionary kernel scanning (vulnerable + patched)MYSQL_TEST=1 go test -run TestOvalAnalyzer ./server/vulnerabilities/oval/...— existing tests pass unchangedmake lint-go— passesSummary by CodeRabbit