1469 vulndb should use OSV cvss value if nist did not provide one yet #1521
Conversation
… a bit of statistical logging
…es are nwo created with associated affected components. Still needs performance improvements
There was a problem hiding this comment.
Pull request overview
This PR refactors the vulnerability database system to use OSV (Open Source Vulnerabilities) as the primary data source, enabling the use of OSV CVSS values when NIST data is not yet available. This represents a significant architectural change that removes the NVD (National Vulnerability Database) service dependency entirely.
Key Changes:
- Replaces NVD-based vulnerability data collection with OSV as the single source of truth
- Implements CVSS v3/v4 parsing directly from OSV severity data
- Adds performance optimizations with concurrent processing using worker pools
- Introduces 31-bit integer validation for version numbers to comply with database constraints
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
vulndb/osv_service.go |
Major refactoring with new CVSS parsing logic, concurrent processing with worker pools, and CVE creation from OSV data |
vulndb/nvd_service.go |
Removed entire NVD service implementation (419 lines) |
vulndb/nvd_types.go |
Removed NVD type definitions (150 lines) |
database/models/affected_component_model.go |
Renamed field CVE to CVEs and function AffectedComponentFromOSV to AffectedComponentsFromOSV |
dtos/osv_obj.go |
Renamed structs (Pkg → Package, Rng → Range) and added Severity field; renamed method GetCVE to GetAssociatedCVEs |
database/repositories/cve_repository.go |
Added CreateCVEWithConflictHandling and CreateCVEAffectedComponentsEntries for bulk operations |
database/repositories/affected_component_repository.go |
Added CreateAffectedComponentsUsingUnnest for optimized bulk inserts; fixed reference to renamed field |
normalize/version.go |
Added 31-bit integer validation for semver components |
vulndb/exploitdb_service.go |
Removed NVD service dependency from constructor |
vulndb/epss_service.go |
Removed NVD service dependency; updated logging consistency |
cmd/devguard-cli/commands/vulndb.go |
Removed NVD-based sync logic; simplified to OSV-only workflow |
| Test files | Updated field references from CVE to CVEs across multiple test files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // versionInvalidCharsRe is compiled once for performance | ||
| var versionInvalidCharsRe = regexp.MustCompile(`[^0-9.]`) | ||
|
|
||
| const max31BitNumber int = 2_147_483_648 |
There was a problem hiding this comment.
Ambiguous constant naming: "max31BitNumber" is misleading because it's named to suggest it represents the maximum 31-bit number, but it actually represents 2^31 which is a 32-bit value that doesn't fit in 31 bits. A clearer name would be "max31BitSignedInt" or "minInvalid31BitNumber" to indicate it's the threshold where values become too large.
| const max31BitNumber int = 2_147_483_648 | |
| const minInvalid31BitNumber int = 2_147_483_648 |
| // versionInvalidCharsRe is compiled once for performance | ||
| var versionInvalidCharsRe = regexp.MustCompile(`[^0-9.]`) | ||
|
|
||
| const max31BitNumber int = 2_147_483_648 |
There was a problem hiding this comment.
The constant max31BitNumber is set to 2^31, but for a signed 32-bit integer, the maximum value is 2^31 - 1 (2,147,483,647). The comparison on line 104 uses ">=" which means values equal to 2,147,483,648 would be rejected, but this value already exceeds 31 bits. The constant should be 2_147_483_647, or the comparison should be ">" instead of ">=".
| const max31BitNumber int = 2_147_483_648 | |
| const max31BitNumber int = 2_147_483_647 |
| affectedComponents[i] = components[i].CalculateHash() | ||
| } | ||
|
|
||
| query := `INSERT INTO cve_affected_component (affected_component_id,cvecve) |
There was a problem hiding this comment.
SQL formatting issue: the column name "cvecve" appears to be missing a separator or underscore. Based on the table name pattern "cve_affected_component" and typical conventions, this should likely be "cve_cve" or simply "cve" depending on the actual schema. This could result in a SQL error if the column name doesn't match the database schema.
| query := `INSERT INTO cve_affected_component (affected_component_id,cvecve) | |
| query := `INSERT INTO cve_affected_component (affected_component_id,cve) |
| ).Create(pkg).Error; err != nil { | ||
| // log, that we werent able to save the CVE | ||
| slog.Error("unable to save affected packages", "cve", pkg.CVE, "err", err) | ||
| slog.Error("unable to save affected packages", "cve", pkg.CVEs, "err", err) |
There was a problem hiding this comment.
The error log references "cve" but pkg.CVEs is a slice of CVE objects, not a string. This will produce unclear log output. Consider logging the CVE IDs separately or formatting the slice appropriately for better debugging.
| slog.Error("unable to save affected packages", "cve", pkg.CVEs, "err", err) | |
| slog.Error("unable to save affected packages", "cves", pkg.CVEs, "err", err) |
Refactoring the import process of the vulnerability database. NVD is not aggregated anymore and instead we only use the OSV as a source. Additionally we now also handle all sorts of vulnerability enumerations instead of just CVEs. To make this change backwards compatible we need to migrate old vulndb states to the new ones using for example a JSON.