feat: add transitive extraction for Maven pom.xml #399
Conversation
| AuthDigest | ||
| ) | ||
|
|
||
| // HTTPAuthentication holds the information needed for general HTTP Authentication support. |
There was a problem hiding this comment.
Does this have OSV-Scanner specific code in it? It sounds like something that'd be available in third-party libraries.
If so maybe we can add to the comment why we're using this instead of an existing lib.
There was a problem hiding this comment.
I actually struggled to find third-party libraries that did this HTTP authentication for clients.
But regardless, this HTTPAuthentication struct was made to be configurable enough to be able to copy the how the native mvn/npm tooling approaches authentication. We could mention something to that effect in the comment.
|
Mostly had a couple of style-related comments (consistency with rest of OSV-SCALIBR and google-internal linters), I'll defer to Rex for the functional parts of the migration from the OSV-Scanner. |
| return nil, fmt.Errorf("failed resolving %v: %w", root, err) | ||
| } | ||
| for i, e := range g.Edges { | ||
| e.Type = dep.Type{} |
There was a problem hiding this comment.
Can you add a comment here about why you are wiping e.Type here?
There was a problem hiding this comment.
this was for comparing the resolved graph - we don't need this here.
|
|
||
| // ToPURL converts an inventory created by this extractor into a PURL. | ||
| func (e Extractor) ToPURL(i *extractor.Inventory) *purl.PackageURL { | ||
| return &purl.PackageURL{ |
There was a problem hiding this comment.
This might be missing some of the purl requirements.
https://github.com/package-url/purl-spec/blob/master/PURL-TYPES.rst#maven
- Check name the format that purl expects?
- Add repository_url parameter if neccessary
- Maybe add type as well?
There was a problem hiding this comment.
Currently classifier and type are not available in the node. I made #426 as a TODO for the future.
| func (e Extractor) Requirements() *plugin.Capabilities { | ||
| return &plugin.Capabilities{ | ||
| Network: true, | ||
| DirectFS: true, |
There was a problem hiding this comment.
I don't think this needs DirectFS, as all fs operations are done through the VirtualFS interface.
There was a problem hiding this comment.
I am a bit confused about when to use the VirtualFS and when to use DirectFS. There was another comment suggesting to mark DirectFS as true. Do you mind elaborating this more?
There was a problem hiding this comment.
From what I understand you need DirectFS if you access the disk directly (e.g. via os.Open, or via a C library ...etc), if you only access the disk through our virtual filesystem, you don't need DirectFS, since it doesn't matter to the extractor whether we are actually reading from disk, streaming the file over a network connection, or reading a container for example.
@erikvarga can you double check that my understanding is correct?
This PR adds a new
pomxmlnetextractor to first parse pom.xml then perform dependency resolution to extract both direct and transitive dependencies. Most code are migrated from OSV-Scanner but switch to use the virtual file system.To achieve this goal, this PR also adds some
internalpackages:mavenutil: Maven utilities to help parse and resolve dependenciesdatasource: clients to talk to registries for package and version informationresolution: clients (and tests) needed for dependency resolutionWe still need to figure out how to registry this extractor to the list and only enable it with some flags.