google / osv-scanner

Vulnerability scanner written in Go which uses the data provided by https://osv.dev
https://google.github.io/osv-scanner/
Apache License 2.0
6.02k stars 337 forks source link

Add a new Maven pom.xml extractor #982

Closed cuixq closed 1 month ago

cuixq commented 1 month ago

The new Maven lockfile extractor aims to resolve the full Maven dependency graph to provide better transitive support https://github.com/google/osv-scanner/issues/35. This is an experimental feature for now.

This PR uses deps.dev util package to parse Maven pom.xml, also calls deps.dev API for available versions when resolving a range requirement.

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 77.50000% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 64.24%. Comparing base (5eed7e8) to head (609e62d). Report is 5 commits behind head on main.

Files Patch % Lines
internal/manifest/maven.go 77.50% 12 Missing and 6 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #982 +/- ## ========================================== + Coverage 64.05% 64.24% +0.18% ========================================== Files 146 148 +2 Lines 11977 12088 +111 ========================================== + Hits 7672 7766 +94 - Misses 3853 3864 +11 - Partials 452 458 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

G-Rath commented 1 month ago

fwiw I'm not sure that it makes sense to land this in its current form given that it's not being actually being used and violates the "offline" contract, which I don't think we can necessarily address with the current interface so landing it as-is might prematurely lock us into a public API we immediately want to change.

oliverchang commented 1 month ago

fwiw I'm not sure that it makes sense to land this in its current form given that it's not being actually being used and violates the "offline" contract, which I don't think we can necessarily address with the current interface so landing it as-is might prematurely lock us into a public API we immediately want to change.

That's a good point. Is there an easy way we can keep this interface private, while being able to use it ourselves?

Alternatively, we can just add some clear documentation to the relevant functions to say: this is experimental and can break.

oliverchang commented 1 month ago

Following up on this, the right way to do this for now is probably:

WDYT @cuixq @G-Rath ?

cuixq commented 1 month ago

For now, I would prefer keeping the new extractor in internal folder so we can experiment it without breaking anything.

When it's ready enough, we can think about how to enable this with the offline mode, which could be something similar to the second option.

G-Rath commented 1 month ago

fwiw if you want to enable experimental support for it without a lot of work, you should be able to add it as a custom parser here