feat(sqlite): support SQLite alongside PostgreSQL in the vulnerability queries - #36
Open
scanoss-qg wants to merge 4 commits into
Open
feat(sqlite): support SQLite alongside PostgreSQL in the vulnerability queries#36scanoss-qg wants to merge 4 commits into
scanoss-qg wants to merge 4 commits into
Conversation
…y queries The service could not run against SQLite, and the two vulnerability queries could not run against the real schema at all. Both are rewritten as portable SQL, with the version matching moved into Go, and the tests now run on the production schema so this class of breakage cannot pass CI again. Queries ------- GetVulnsByPurlName joined cpes.id and nvd_match_criteria_ids.cpe_ids, neither of which the schema defines. The real path is purl -> short_cpe_purl.cpe_id -> nvd_match_criteria_ids.short_cpe_id -> match_criteria_id -> cves.match_criteria_ids, and both variants now share one query. CAST keeps the membership test working whether match_criteria_ids is TEXT or an array, and COALESCE guards the nullable columns. GetVulnsByPurlVersion used array_agg, the && array overlap operator and natural_sort_order, a custom PostgreSQL function. Version bounds are now compared in version_range.go with Masterminds/semver, the approach FilterCpesByRequirement already takes. Besides being portable this is more correct: versions order semantically, so 9.0.0 no longer sorts after 10.0.0. An exact match on an inclusive bound still short-circuits, preserving the old behaviour for versions that are not valid semver, and an unparseable bound is skipped rather than treated as a mismatch - missing a real vulnerability is worse than reporting a borderline one. versionBounds.covers deliberately does not log: it runs per row per request, and reaching the global zlog.S made a pure comparison panic wherever the logger was not initialised, which its own unit test hit immediately. Dates ----- utils.OnlyDate now implements sql.Scanner and driver.Valuer, accepting a string from a TEXT column and a time.Time from a PostgreSQL date. Without it, every query selecting cves.published failed on SQLite. Tests ----- Fixtures used to ship their own CREATE TABLE statements describing tables that do not exist - DATETIME dates, an integer[] cpe_ids array, surrogate id columns with AUTOINCREMENT. Queries were verified against invented tables, so the suite passed while the main query referenced columns the schema never had. - the schema now comes from testSchemaDDL in pkg/models/test_schema.go; fixtures carry data only, with all 960 inserts converted to the real column sets - added vulns_scenario.sql, a deterministic component whose match criteria cover each version-bound combination. The old fixtures could not exercise this path at all: every ndv_match_criteria_ids row has short_cpe_id = null, because the previous query joined through the non-existent cpe_ids column - dropped cpe_cve.sql, a table no query uses and the schema does not define - unified the test driver on modernc.org/sqlite, the one production uses. go-sqlite3 needed CGO and converted dates by decltype, which is precisely what hid the OnlyDate defect; it is no longer a direct dependency - tests now assert on results, not just on the absence of an error, at model and use case level Write paths ----------- saveLicense inserted is_sanitized, not a column in licenses, and saveVersion passed four arguments to a two-placeholder insert. Neither has a caller and the schema generates no ids, so rows go in without one; COALESCE(id, 0) keeps reads working without changing the public struct types. Notes ----- epss_data is included in the schema constant. While it was absent, enrichWithEPSS failed on every request and each vulnerability came back with epss.probability and epss.percentile reading 0, indistinguishable from a genuine zero. The schema constant is a transcription and can drift from the real database; the header of test_schema.go records the sqlite3 .schema command to diff against a live one. Two things found but deliberately left alone, both predating this work: - vulnerabilityWorker logs a query error and emits the component with no vulnerabilities, so the service answers Success with zero CVEs on failure - EXPLAIN QUERY PLAN shows cves scanned in full on every lookup, since matching a criterion inside a delimited list needs a leading-wildcard LIKE. Worth measuring against the production dataset; the structural fix is a cve to match_criteria_id bridge table Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR centralizes SQLite test schema setup, migrates tests to ChangesSQLite infrastructure and fixtures
Vulnerability matching and validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 (4)
pkg/utils/only_date_db_test.go (1)
29-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
OnlyDate.Value.The new database conversion contract also has write behavior. Test that a zero value returns
niland that a non-zero value returns"2020-01-15".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/utils/only_date_db_test.go` around lines 29 - 61, Add a dedicated test for OnlyDate.Value covering both write cases: assert a zero OnlyDate returns nil, and assert a non-zero value returns the string "2020-01-15". Follow the existing table-driven style used by TestOnlyDateScan and validate returned values and errors.pkg/models/vulns_purl.go (2)
84-98: 🚀 Performance & Scalability | 🔵 TrivialPlan an index-friendly criteria lookup for production data volumes.
The criteria-to-CVE relationship is stored as a delimited field, so this query joins with a leading-wildcard
LIKEand scanscvesfor every matching criterion row. On a full NVD dataset the cost grows with the CVE count multiplied by the criteria rows for the component.Two options reduce the cost:
- Add a join table, for example
cve_match_criteria(match_criteria_id, cve), with an index onmatch_criteria_id, and populate it during ingestion.- On PostgreSQL, keep the array column and add a GIN index, then match with the array containment operator behind a driver-specific query.
Also add an index on
short_cpe_purl.purlandnvd_match_criteria_ids.short_cpe_idif none exists, because both drive the first two joins.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/models/vulns_purl.go` around lines 84 - 98, Replace the leading-wildcard criteria join in vulnsForPurlQuery with an index-friendly cve_match_criteria lookup (or the PostgreSQL array-containment equivalent), and populate or maintain that representation during ingestion. Add indexes on short_cpe_purl.purl, nvd_match_criteria_ids.short_cpe_id, and the criteria-to-CVE key used by the new join when they are not already defined.
94-98: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAnchor
match_criteria_idmembership against delimiters.
CAST(c.match_criteria_ids AS TEXT) LIKE '%' || nmci.match_criteria_id || '%'matches ids as strings, not as delimited list elements, and interpolatesmatch_criteria_idinto aLIKEpattern so%or_change the predicate. Match it inside a normalized delimited representation before joining.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/models/vulns_purl.go` around lines 94 - 98, Update the join condition involving cves and nvd_match_criteria_ids so match_criteria_id is matched only as a complete element within the normalized delimited match_criteria_ids representation, not as an arbitrary substring. Ensure wildcard characters in match_criteria_id are treated literally rather than as LIKE pattern syntax, while preserving the existing short_cpe_purl filtering and ordering.pkg/models/version_range.go (1)
51-58: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRestrict the exact-match shortcut to unparseable versions.
Apply the
StartIncluding/EndIncludingmatch only whensemver.NewVersion(version)fails; parseable versions should still follow the full bound checks so constraints like{StartIncluding: "1.0.0", EndExcluding: "1.0.0"}can correctly reject1.0.0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/models/version_range.go` around lines 51 - 58, Update the exact-match shortcut in the version range check to require semver.NewVersion(version) to fail before comparing against StartIncluding or EndIncluding. Keep parseable versions on the existing full bound-validation path so conflicting inclusive and exclusive constraints are enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/models/versions.go`:
- Around line 85-91: Update pkg/models/versions.go lines 85-91 and
pkg/models/licenses.go lines 104-108 so the versions.id and licenses.id schema
definitions generate IDs, then remove COALESCE(id, 0) from all affected
RETURNING clauses and the corresponding queries at versions.go line 63 and
licenses.go lines 64 and 83. Let generated IDs be returned directly without
query-level NULL masking.
---
Nitpick comments:
In `@pkg/models/version_range.go`:
- Around line 51-58: Update the exact-match shortcut in the version range check
to require semver.NewVersion(version) to fail before comparing against
StartIncluding or EndIncluding. Keep parseable versions on the existing full
bound-validation path so conflicting inclusive and exclusive constraints are
enforced.
In `@pkg/models/vulns_purl.go`:
- Around line 84-98: Replace the leading-wildcard criteria join in
vulnsForPurlQuery with an index-friendly cve_match_criteria lookup (or the
PostgreSQL array-containment equivalent), and populate or maintain that
representation during ingestion. Add indexes on short_cpe_purl.purl,
nvd_match_criteria_ids.short_cpe_id, and the criteria-to-CVE key used by the new
join when they are not already defined.
- Around line 94-98: Update the join condition involving cves and
nvd_match_criteria_ids so match_criteria_id is matched only as a complete
element within the normalized delimited match_criteria_ids representation, not
as an arbitrary substring. Ensure wildcard characters in match_criteria_id are
treated literally rather than as LIKE pattern syntax, while preserving the
existing short_cpe_purl filtering and ordering.
In `@pkg/utils/only_date_db_test.go`:
- Around line 29-61: Add a dedicated test for OnlyDate.Value covering both write
cases: assert a zero OnlyDate returns nil, and assert a non-zero value returns
the string "2020-01-15". Follow the existing table-driven style used by
TestOnlyDateScan and validate returned values and errors.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 37f4f009-33be-4c4a-b8f3-c51eabbcdede
📒 Files selected for processing (41)
go.modpkg/adapters/vulnerability_support_test.gopkg/models/common.gopkg/models/common_test.gopkg/models/cpe_purl_test.gopkg/models/epss_test.gopkg/models/licenses.gopkg/models/licenses_test.gopkg/models/mines_test.gopkg/models/projects_test.gopkg/models/test_schema.gopkg/models/tests/all_urls.sqlpkg/models/tests/bad_sql.sqlpkg/models/tests/cpe.sqlpkg/models/tests/cpe_cve.sqlpkg/models/tests/cve.sqlpkg/models/tests/epss.sqlpkg/models/tests/golang_projects.sqlpkg/models/tests/licenses.sqlpkg/models/tests/mines.sqlpkg/models/tests/ndv_match_criteria_ids.sqlpkg/models/tests/projects.sqlpkg/models/tests/purl.sqlpkg/models/tests/short_cpe.sqlpkg/models/tests/short_cpe_purl.sqlpkg/models/tests/versions.sqlpkg/models/tests/vulns_scenario.sqlpkg/models/version_range.gopkg/models/version_range_test.gopkg/models/versions.gopkg/models/versions_test.gopkg/models/vulns_purl.gopkg/models/vulns_purl_scenario_test.gopkg/models/vulns_purl_test.gopkg/service/vulnerability_service_test.gopkg/usecase/cpe_test.gopkg/usecase/local_use_case_scenario_test.gopkg/usecase/local_use_case_test.gopkg/usecase/vulnerability_use_case_test.gopkg/utils/only_date.gopkg/utils/only_date_db_test.go
💤 Files with no reviewable changes (1)
- pkg/models/tests/cpe_cve.sql
Comment on lines
+85
to
+91
| // The row is written without an id: the schema declares versions.id as a plain | ||
| // column with no autoincrement, so there is nothing to generate one. No caller | ||
| // reaches this path today, and none reads the returned ID. | ||
| err := m.db.QueryRowxContext(ctx, | ||
| "INSERT INTO versions (version_name, semver) VALUES($1, $2)"+ | ||
| " RETURNING id, version_name, semver", | ||
| name, "", false, false, | ||
| " RETURNING COALESCE(id, 0) AS id, version_name, semver", | ||
| name, "", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
NULL ids written by the insert paths, then masked by COALESCE(id, 0). Both tables declare id without generation, so each insert stores a NULL id and every read normalizes it to 0. The shared fix is schema-side id generation, not per-query normalization.
pkg/models/versions.go#L85-L91: declareversions.idas generated, then removeCOALESCE(id, 0)from theRETURNINGclause and from Line 63.pkg/models/licenses.go#L104-L108: declarelicenses.idas generated, then removeCOALESCE(id, 0)from Line 64 and Line 83.
📍 Affects 2 files
pkg/models/versions.go#L85-L91(this comment)pkg/models/licenses.go#L104-L108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/models/versions.go` around lines 85 - 91, Update pkg/models/versions.go
lines 85-91 and pkg/models/licenses.go lines 104-108 so the versions.id and
licenses.id schema definitions generate IDs, then remove COALESCE(id, 0) from
all affected RETURNING clauses and the corresponding queries at versions.go line
63 and licenses.go lines 64 and 83. Let generated IDs be returned directly
without query-level NULL masking.
…emver Verified against the production PostgreSQL database, the semver comparison returned fewer CVEs than the SQL it replaced on 3 of 48 sampled purl/version pairs, losing 27 CVEs. Version matching now ports natural_sort_order faithfully, and the same sample comes back with lost=0, gained=0. The data does not support semantic version ordering. 48% of the version names and 39.5% of the version bounds in production are not valid semver - "*", "-", "00.00.01a", "0.001.00.060", buildbot branch names. Semver has to drop those, and dropping them loses real vulnerabilities: the two anuko/time-tracker versions "*" and "-" went from 12 CVEs each to none. Pre-releases were the other divergence. natural_sort_order places 1.10.0-rc1 after 1.10.0, since it shares the prefix and carries extra characters, so a pre-release inherits the vulnerabilities of its release. Semver orders it before and excludes it, which cost 3 CVEs on apache/ant 1.10.0-rc1. Matching production is the safer default for a vulnerability service. Worth noting the zero padding preserves numeric ordering, so 9.0.0 still sorts before 10.0.0 - the bug in the original string comparison this was meant to fix does not exist. Padding to 20 digits, as every call site did, handles it. covers now mirrors the boolean shape of the original SQL exactly, quirks included: an exact match on an inclusive bound short-circuits before the range tests, so it wins even when the opposite bound would exclude the version. Tests ----- - naturalSortKey is pinned against 20 values produced by the real natural_sort_order function, including the overflow case where a number exceeds max_length and the extra digit is emitted as a plain character - pg_parity_test.go compares both rewritten queries against the originals on a real database. Skipped unless PG_DSN is set, so CI is unaffected. This is the test that caught the regression, and the one to run after touching either query Also confirms the queries are sound on PostgreSQL: cves.match_criteria_ids is a real array there, so the CAST is required; published and modified are date columns, which OnlyDate.Scan handles; and id columns are integer, so COALESCE(id, 0) is well typed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/models/pg_parity_test.go`:
- Around line 120-127: The parity test currently reports query failures and lost
CVEs without failing. In the parity test function, replace the OLD ERROR and NEW
ERROR fmt.Printf paths with t.Error or t.Fatal as appropriate, and use t.Errorf
when the versioned comparison finds len(lost) > 0; preserve the existing
diagnostic details in the test messages.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f25cae7-3a58-438d-9875-921e4c9d6050
📒 Files selected for processing (3)
pkg/models/pg_parity_test.gopkg/models/version_range.gopkg/models/version_range_test.go
CI caught these because the linter could not be run locally earlier: the installed version was 1.62.2 against a config targeting 2.10.1. Verified now with the exact version the workflow installs, which reports 0 issues. - lll: wrapped the three CREATE TABLE statements in testSchemaDDL that exceeded 180 characters. Same DDL, split across lines - unparam: naturalSortKey took a maxLength every caller passed 20 for, so it now uses the nsoMaxLength constant directly. The original function guarded against out-of-range widths, which is unreachable at a fixed 20 and is no longer reproduced - unused: dropped versionBounds.isOpen. Porting natural_sort_order removed its last caller, since an open range now falls out of the bound comparisons on its own - unused: moved loadTestSQLDataFilesWithSchema into a _test.go file. Only tests in this package call it, so it does not belong in the binary. LoadTestSchema and LoadTestSQLData stay in common.go because tests in other packages use them Dropped the two tests covering the removed maxLength parameter and isOpen along with them. Go Unit Test already passed on the previous commit; the earlier run failures on 1a386ea were infrastructure, with both jobs waiting 15 minutes for a hosted runner that never arrived. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Flagged in review, and correctly: the test printed lost CVEs and query errors but still exited successfully, so it was a comparison tool rather than a guard. A future change could lose vulnerabilities and the test would stay green. - lost CVEs now fail the test, in both the version and version-less variants - a query error fails instead of being skipped past; if the original query cannot run there is no parity to verify, which is not a pass - a run that compares nothing fails too, since a silent pass over an empty sample looks identical to a clean run Re-run against the production database: PASS, 48 pairs, no false positives from the new assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What this does
The service could not run against SQLite, and the two vulnerability queries could not run against the real schema at all. Both are rewritten as portable SQL valid on PostgreSQL and SQLite, with version matching moved into Go. The tests now run on the production schema, so this class of breakage cannot pass CI again.
Why the suite was green before
The fixtures shipped their own
CREATE TABLEstatements describing tables that do not exist —DATETIMEdates, aninteger[] cpe_idsarray, surrogateidcolumns withAUTOINCREMENT. Queries were verified against invented tables, so everything passed while the main query referenced columns the schema never had. The existing vulnerability tests only covered error paths (empty purl, closed connection), never a successful query.Defects fixed
GetVulnsByPurlNamejoinedcpes.idandnvd_match_criteria_ids.cpe_idsno such column: c.id— neither column existsGetVulnsByPurlVersionusedarray_agg,&&,natural_sort_orderutils.OnlyDatedid not implementsql.Scannercves.publishedfailed on SQLitesaveLicenseinsertedis_sanitizedlicensessaveVersionpassed 4 args to a 2-placeholder insertApproach
One shared query for both variants, following the real path:
purl → short_cpe_purl.cpe_id → nvd_match_criteria_ids.short_cpe_id → match_criteria_id → cves.match_criteria_ids.CASTkeeps the membership test working whethermatch_criteria_idsis TEXT or an array;COALESCEguards the nullable columns.Version bounds compared in Go (
version_range.go) withMasterminds/semver, the approachFilterCpesByRequirementalready takes. Besides being portable this is more correct: versions order semantically, so9.0.0no longer sorts after10.0.0as it did undernatural_sort_order's string sort. An exact match on an inclusive bound still short-circuits, preserving the old behaviour for versions that are not valid semver. An unparseable bound is skipped rather than treated as a mismatch — missing a real vulnerability is worse than reporting a borderline one.Schema as a Go constant (
pkg/models/test_schema.go). Fixtures carry data only, with all 960 inserts converted to the real column sets.Test driver unified on
modernc.org/sqlite, the one production uses.go-sqlite3needed CGO and converted dates by decltype — precisely what hid theOnlyDatedefect. It is no longer a direct dependency.A deterministic scenario fixture whose match criteria cover each version-bound combination (inclusive/exclusive, upper/lower), so the filtering is asserted at its edges. The old fixtures could not exercise this path at all: every
ndv_match_criteria_idsrow hasshort_cpe_id = null, because the previous query joined through the non-existentcpe_idscolumn.Deployment note
epss_datahad to be created in the database (it is in the schema constant now). While it was absent,enrichWithEPSSfailed on every request and each vulnerability came back withepss.probabilityandepss.percentilereading0, indistinguishable from a genuine zero. Already applied.To point the service at SQLite:
DB_DRIVER=sqliteandDB_DSN=<file>. No code change needed —OpenDBConnectionis driver-agnostic andmodernc.org/sqlitewas already imported.Known limitations, not addressed here
Both predate this work and are called out rather than fixed:
vulnerabilityWorkerswallows query errors: it logs and emits the component with no vulnerabilities, so the service answersSuccesswith zero CVEs on failure. This is why a completely broken query produced a successful-looking response.cvesis scanned in full on every lookup (EXPLAIN QUERY PLAN), because matching a criterion inside a delimited list needs a leading-wildcardLIKE, which cannot useidx_cves_match_criteria_ids. Worth measuring against the production dataset; the structural fix is acve → match_criteria_idbridge table.test_schema.gorecords thesqlite3 <db> .schemacommand to diff against a live one.Verification
go build,go vetand the full suite pass. The project linter could not be run locally: the installed golangci-lint is v1.62.2/go1.23 while the config targets v2.10.1/go1.25.gofmtapplied throughout.Summary by CodeRabbit
New Features
Bug Fixes
Update: verified against the production PostgreSQL database
The original description reasoned about PostgreSQL compatibility without testing it. Since then the rewritten queries were run against the real database (PostgreSQL 15.14), and the results changed two conclusions.
The old query was returning nothing on PostgreSQL
GetVulnsByPurlNamereturned 0 CVEs for every purl tested, where the rewrite returns 8, 6, 12 and 6. Its third join comparednmci.cpe_ids(numeric IDs) againstnmci.match_criteria_id(a UUID), which never matched.Correcting something stated earlier:
cpes.idandnvd_match_criteria_ids.cpe_idsdo exist in PostgreSQL — they are absent only from the SQLite schema. The query was executable there; it just produced no rows. So the version-less lookup is not a risky rewrite, it is the repair of a dead path.The semver comparison was a regression, and has been replaced
Comparing both variants against the SQL they replaced, over 48 purl/version pairs, the semver implementation lost 27 CVEs across 3 cases:
Two causes, both rooted in the data: 48% of version names and 39.5% of version bounds in production are not valid semver (
*,-,00.00.01a,0.001.00.060, buildbot branch names). Semver has to drop those. And for pre-releases the two orderings genuinely disagree —natural_sort_orderplaces1.10.0-rc1after1.10.0, so a pre-release inherits its release's CVEs, while semver excludes it.version_range.gonow portsnatural_sort_orderfaithfully instead. The same 48 pairs come back lost=0, gained=0.Note that the zero padding preserves numeric ordering, so
9.0.0still sorts before10.0.0. The ordering bug the semver change was meant to fix did not exist.Type compatibility confirmed
cves.match_criteria_idsis a real array in PG → theCAST(... AS TEXT)is required, not defensivecves.published/modifiedaredate→ the driver returnstime.Time, whichOnlyDate.Scanhandleslicenses.id,versions.id,cpes.idareinteger→COALESCE(id, 0)is well typedepss_data.epss/percentilearenumeric→ convert tofloat32correctlyHow to re-run this
pg_parity_test.gocompares both rewritten queries against the originals on a live database. It skips unlessPG_DSNis set, so CI is unaffected:Expect
lost=0. Anything lost means fewer vulnerabilities reported than production does — a regression, regardless of whether the new answer is arguably more correct.