Fixed unnecessary error logging when no CPE match is found - #38754
Conversation
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe PR suppresses unnecessary error logging when no CPE database match is found for software items during vulnerability checking. The CPEFromSoftware function is modified to detect missing entries and return an empty CPE with no error, preventing error-level logs for unmatched software like VSCode extensions and JetBrains plugins. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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.
Pull request overview
This PR fixes unnecessary error logging that occurred when no CPE (Common Platform Enumeration) match is found for certain software items like VSCode extensions and JetBrains plugins. Previously, these cases were logged as errors with "error translating to CPE, skipping" messages, even though not finding a CPE match is a normal and expected outcome for software that doesn't have corresponding entries in the NVD CPE dictionary.
Changes:
- Modified error handling in
CPEFromSoftwareto treatsql.ErrNoRowsas a normal "no match" case rather than an error - Added changelog entry documenting the fix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/vulnerabilities/nvd/cpe.go | Added check to return empty string instead of error when no CPE database row is found |
| changes/35447-fix-cpe-translation-error-logging | Added user-visible changelog entry describing the fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if errors.Is(err, sql.ErrNoRows) { | ||
| return "", nil |
There was a problem hiding this comment.
This error check uses errors.Is(err, sql.ErrNoRows) which is the more robust approach for error checking. However, there are two other instances in this same function (lines 552 and 614) that use direct comparison err == sql.ErrNoRows. For consistency and best practice, those should also use errors.Is(). While sql.ErrNoRows is a sentinel error that can be compared directly, using errors.Is() is more future-proof and follows Go best practices for error checking.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #38754 +/- ##
==========================================
- Coverage 66.07% 66.07% -0.01%
==========================================
Files 2414 2414
Lines 192710 192715 +5
Branches 8488 8488
==========================================
Hits 127329 127329
- Misses 53818 53819 +1
- Partials 11563 11567 +4
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:
|
| var result IndexedCPEItem | ||
| err = db.Get(&result, stm, args...) | ||
| if errors.Is(err, sql.ErrNoRows) { | ||
| return "", nil |
There was a problem hiding this comment.
if errors.Is(err, sql.ErrNoRows) {
level.Debug(logger).Log("msg", "no CPE found in database", "software", software.Name)
return "", nil
}
Any chance we want to have this log at debug? although I don't think it's necessary...
Related issue: Resolves #35447
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Testing
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.