fix: Fix lookup of golang packages with major versions #2259
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
A concrete example. A golang package uses The database itself reports that |
|
Ah good find! This does seem like something we should resolve by getting the correct package name rather than working around it with purl queries though. I'll look into this a bit more. |
alexg-axis
changed the title
|
@another-rex Thanks for taking a look! Is there any particular reason we should refrain from using the PURL in API calls? From what I've gathered, the "bad" package name stems already from the SBOM, where Syft is used. Then it's just propagated, but unused throughout osv-scalibr and osv-scanner. As the use of Syft is already widespread, I assume we at least in the short term (assuming Syft is in the wrong and fixed) need to find a solution that fits in osv-scanner. I did play around with the idea of adding back the major to the package name, but given that PURL worked I wasn't eager to add a special case for the golang ecosystem, at least not in the matcher. Maybe it would make more sense if it was within scalibr, or the plugins in the osv-scanner if it was closer to the SBOM parsing? My knowledge of the codebase is obviously limited, so please tell me if there's a better solution out there. |
|
I am also surprised that purl queries work here? Looking at the records on osv.dev you can see that the purls also have /v123 as part of the purl: https://osv.dev/vulnerability/GHSA-p84v-gxvw-73pf EDIT: I just queried with your example, and it the purl doesn't seem to fix it, it still displays vulnerabilities that should be fixed... (e.g. query with v4.0.6, should have no vulns, but GO-2025-3485 stills hows up). Though for some reason making the same query with ESI returns 2 vulns. The main issue with just using purls is that it could have unintentional consequences for other ecosystems (e.g. we support more ecosystems that purl defines). If Syft is also generating the incorrect purl already, I think we have two options here.
|
|
@another-rex Thanks again for taking a look. Interesting that my (admittedly shallow) testing made it look like purls were working. I'm not sure if Syft is alone in using this format for purls. I'm guessing the format may differ between tools. So to not leak that too far I guess working around this in osv-scanner makes more sense than doing it in the API? Like a compatibility layer / quirk? Do you have any pointers on how that would be handled? I guess we, when we identify this case, should change the identified package name to include the major suffix (e.g. /v4) and invoke the API as is. Do you see that being done as soon as possible, or just before performing the API requests? |
Yeah I think that is probably the best way to do this (check for golang ecosystem, check for major version number, if bigger than 1, check suffix of package. If it doesn't have /v as a suffix, add it.) |
alexg-axis
force-pushed
the
fix/golang-major-false-positives
branch
from
f29bdf6 to
03e42bc
Compare
|
@another-rex I've updated the code to add the suffix now, with the intention of handling the case of PURLs formatted this way. There could be other tools already adding the suffix. Taking care of all cases could prove quite verbose. Whilst looking into this change, I noticed that for some packages, the PURL subpath not only includes the major version but the actual path to the package within a repository. I tried some of them with the API, but as there were no known vulnerabilities I'm unsure how the API would behave. Essentially, the issue is that we have PURLs like these:
Which in turn are for Go packages like these:
My current approach is to always add the full subpath, as we'd otherwise lose this important context. But I can't prove that it works with the API. The question is basically, is the API expecting the full, proper Go package path, or just the "repository"? |
|
Apologies for the late reply, been travelling the last month. For subpaths that point all the way to the package level, the answer is that it won't match, as Go vuln records are all at a module level I believe. (e.g. https://osv.dev/vulnerability/GO-2025-4096 , go then puts the actual imported package in database specific) |
|
I think for now we can just do a simple regex for In the future we can add an enricher plugin or similar in osv-scalibr to filter on the package path against the database specific imports to filter out non matching packages further. |
Fix a bug causing to false positives for all golang packages with a major version. The bug is caused by the name of golang packages not including the major version. This leads the osv query to look up vulnerabilities to look up the right version, but for the wrong major. E.g. [email protected] instead of go-jose/[email protected]. Solve this by including the subpath of Go PURLs.
alexg-axis
force-pushed
the
fix/golang-major-false-positives
branch
from
03e42bc to
10ddafb
Compare
|
@another-rex thanks for taking another look! I've updated the code to use a regex for getting the module version from the PURL subpath and ignore the Go package's subpath. It works well now for the previously failing go-jose example. |
another-rex
left a comment
another-rex
left a comment
There was a problem hiding this comment.
Thanks! Just a small comment
| maxConcurrentRequests = 1000 | ||
| ) | ||
|
|
||
| var goVersionSuffixRegexp = regexp.MustCompile(`^/?(v\d+)`) |
There was a problem hiding this comment.
Please add a few examples about what type of url this is supposed to match, and what it is supposed to reject. (Just copying one of the links you posted as reject would be great!)
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2259 +/- ##
==========================================
- Coverage 67.83% 67.81% -0.02%
==========================================
Files 171 171
Lines 13019 13024 +5
==========================================
+ Hits 8831 8832 +1
- Misses 3503 3506 +3
- Partials 685 686 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Fix a bug causing to false positives for all golang packages with a
major version.
The bug is caused by the name of golang packages not including the major
version. This leads the osv query to look up vulnerabilities to look up
the right version, but for the wrong major. E.g. [email protected] instead
of go-jose/[email protected].
Solve this by including the subpath of Go PURLs.