fix(nvd): repo from commit in refs different from CPE cache resulting in failed conversion#4770
Conversation
…sulting in failed conversion
another-rex
left a comment
There was a problem hiding this comment.
Add a comment around
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm not sure this is the best fix. What is the point of the first section where we check fixed commits against repos if we are going to do another loop without caring about the repo?
There was a problem hiding this comment.
- Can we fix the root cause of the repos being mismatched
- If 1 is inevitable, do we need to change the logic everywhere HasFixedCommits(repo) is used? I see it called in a few different places
- Either way I don't think we need both loops? We can just remove the first loop right?
There was a problem hiding this comment.
TBH I want to rewrite this whole thing to rely natively on the OSV schema fields instead and make the logic a lot clearer, but it's not a high priority rn.
Removed the first block.
| } | ||
|
|
||
| v := vulns.FromNVDCVE(cve.ID, cve) | ||
| versions := cves.ExtractVersionInfo(cve, nil, http.DefaultClient, metrics) |
There was a problem hiding this comment.
nit: can we rename this to affectedVersionsAndCommits. I was quite confused for a bit, had to look at the definitions to understand what this actually contained.
There was a problem hiding this comment.
Yeah it's confusing cause it's referencing the VersionInfo struct (which consists of AffectedCommits and AffectedVersions). I can refactor to rename this but eventually I do want to move completely off the VersionInfo struct
Actual root cause of what #4770 was trying to solve is actually linkrot. This may involve more requests but also less failures due to linkrot. ~Also refactored FindRepos to do less repetitive work.~ if I make it less repetitive, vp cache doesn't work ):
CVE-2026-23522 has a different repository in its CPE cache than in the commit extracted from references, causing a weird bug where it considers it a failed conversion.