Improved cpe deterministic matching - #42325
Conversation
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.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Improves deterministic CPE candidate ranking for software entries that often lack vendor data (e.g., package-manager sources) by incorporating the software source’s implied ecosystem (target_sw) into match scoring to reduce nondeterministic vendor selection.
Changes:
- Add
target_swto indexed CPE query results and introduce ecosystem-based scoring when selecting the “best” CPE candidate. - Extend CPE matching tests and test CPE dictionary fixtures to cover python/npm package scenarios.
- Add a user-visible changes entry describing the improved deterministic matching.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/vulnerabilities/nvd/cpe.go | Adds target_sw to selection query and implements target_sw/ecosystem scoring in best-match selection. |
| server/vulnerabilities/nvd/indexed_cpe_item.go | Extends IndexedCPEItem with TargetSW to support the new scoring signal. |
| server/vulnerabilities/nvd/cpe_test.go | Adds test coverage for target_sw/ecosystem scoring and deterministic fallback behavior. |
| server/vulnerabilities/nvd/testing_utils.go | Adds additional CPE test fixtures (requests/duplicity/express) to exercise new ranking logic. |
| changes/41644-improve-cpe-matching | Documents the user-visible behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #42325 +/- ##
=======================================
Coverage 66.51% 66.51%
=======================================
Files 2526 2526
Lines 202654 202689 +35
Branches 9021 9021
=======================================
+ Hits 134787 134816 +29
- Misses 55695 55700 +5
- Partials 12172 12173 +1
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:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe change introduces the 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.
Actionable comments posted: 2
🤖 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/nvd/cpe.go`:
- Around line 229-257: The distro/ecosystem matches (the expectedTargetSW == "*"
branch checking software.Source for "deb_packages" and "rpm_packages" and
matching item.Vendor like "debian", "redhat", "fedora") should score higher than
the product "_project" fallback; change the return values in that branch from 1
to a higher score (e.g., 2) so these vendor-less distro/ecosystem signals beat
the vendorLower == productLower+"_project" case (which remains 1), ensuring
distro matches are preferred over the <product>_project fallback and preventing
tie-breaks by alphabetical vendor ordering.
- Around line 123-126: The SELECT list in cpeSelectColumns is missing the
sw_edition column, causing IndexedCPEItem.FmtStr() to read an empty i.SWEdition
and thus serialize CPEs without edition qualifiers; update the cpeSelectColumns
constant to include c.sw_edition (e.g., add c.sw_edition to the SELECT
projection) so the query returns SWEdition and the code that builds/returns
IndexedCPEItem (and its FmtStr()) can populate i.SWEdition correctly.
🪄 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: 65a25fff-3be3-4507-9ca2-e2a60a9dc9a9
📒 Files selected for processing (5)
changes/41644-improve-cpe-matchingserver/vulnerabilities/nvd/cpe.goserver/vulnerabilities/nvd/cpe_test.goserver/vulnerabilities/nvd/indexed_cpe_item.goserver/vulnerabilities/nvd/testing_utils.go
getvictor
left a comment
There was a problem hiding this comment.
Looks good. I think this is a good improvement that should result in fewer false positives/negatives.
Related issue: Resolves #41644
There are two cases that exist in the cpe database where this generic logic could not be applied.
django from python_packages:
gofiber:django
djangoproject:django
npm from npm_packages:
microsoft:npm
npmjs:npm
These will require individual cve overrides that is outside the scope of this task.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Summary by CodeRabbit