Fixed CPE matching failing for software names that sanitize to FTS5 reserved keywords (AND, OR, NOT) - #41226
Conversation
…eserved keywords (AND, OR, NOT)
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
Fixes an FTS5 query parsing failure in NVD CPE matching when a software name sanitizes to reserved FTS5 operators (AND/OR/NOT), preventing the vulnerability cron from skipping those software entries.
Changes:
- Update
sanitizeMatchto quote each token so FTS5 treats reserved keywords as literals. - Add a regression test covering software names that sanitize to reserved keywords.
- Add a changelog entry for the bug fix.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/vulnerabilities/nvd/sanitize.go | Quotes sanitized tokens to prevent FTS5 reserved keywords from being interpreted as operators in MATCH queries. |
| server/vulnerabilities/nvd/cpe_test.go | Adds coverage to ensure CPE matching does not error on names that sanitize to AND/OR/NOT. |
| changes/41225-cpe-matching-reserved-keywords | Records the user-visible bug fix in the changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR fixes a bug in CPE matching where software names containing or sanitizing to FTS5 reserved keywords (AND, OR, NOT) caused query syntax errors. The fix modifies 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 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.
🧹 Nitpick comments (1)
server/vulnerabilities/nvd/cpe_test.go (1)
59-76: Cover the successful-match path too.This only verifies that the query stops erroring. It still passes if
CPEFromSoftwarereturns an empty CPE, so the regression that prompted this PR is not fully protected. Please add at least one fixture/assertion that a keyword-like name resolves to the expected CPE, not justNoError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/vulnerabilities/nvd/cpe_test.go` around lines 59 - 76, The test currently only asserts CPEFromSoftware(t.Context(), slog.New(...), db, &fleet.Software{Name: name...}, ...) does not error for FTS5-keyword-like names; also assert the successful-match path by adding at least one fixture where a specific keyword-like software name (e.g., "_OR_" or "OR - Debug") maps to the expected non-empty CPE string returned by CPEFromSoftware; update the test to call CPEFromSoftware for that fixture, require.NoError on err, then require.NotEmpty (or require.Equal) on the returned CPE value to ensure a correct resolution (reference function CPEFromSoftware, the test variable ftsKeywordNames and the fleet.Software{Name, Version, Source} input, and reCache/db used in the call).
🤖 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/nvd/cpe_test.go`:
- Around line 59-76: The test currently only asserts
CPEFromSoftware(t.Context(), slog.New(...), db, &fleet.Software{Name: name...},
...) does not error for FTS5-keyword-like names; also assert the
successful-match path by adding at least one fixture where a specific
keyword-like software name (e.g., "_OR_" or "OR - Debug") maps to the expected
non-empty CPE string returned by CPEFromSoftware; update the test to call
CPEFromSoftware for that fixture, require.NoError on err, then require.NotEmpty
(or require.Equal) on the returned CPE value to ensure a correct resolution
(reference function CPEFromSoftware, the test variable ftsKeywordNames and the
fleet.Software{Name, Version, Source} input, and reCache/db used in the call).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a493abc9-72f0-40a7-9662-aa1ec285f9f3
📒 Files selected for processing (4)
changes/41225-cpe-matching-reserved-keywordscmd/osquery-perf/software-library/software.sqlserver/vulnerabilities/nvd/cpe_test.goserver/vulnerabilities/nvd/sanitize.go
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #41226 +/- ##
==========================================
- Coverage 66.35% 66.35% -0.01%
==========================================
Files 2475 2475
Lines 198385 198388 +3
Branches 8856 8856
==========================================
+ Hits 131639 131640 +1
+ Misses 54859 54857 -2
- Partials 11887 11891 +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:
|
Related issue: Resolves #41225
Most diffs are from regenerating software.sql, where we added this:
Checklist for submitter
If some of the following don't apply, delete the relevant line.
changes/,orbit/changes/oree/fleetd-chrome/changes.Testing
Summary by CodeRabbit
Bug Fixes
Tests