Skip to content

Commit

Permalink
feat: fetch Maven metadata from specified repositories (#1286)
Browse files Browse the repository at this point in the history
#1045

There are [repositories](https://maven.apache.org/pom.html#Repositories)
defined in a Maven pom.xml. When looking for an artifact, these
repositories are searched one by one until the artifact is found. Maven
Central is the default registry to try at the last.

To support this behaviour, this PR:
- makes `MavenRegistryAPIClient` host a list of registries besides the
default registry
- adds `UpdateRegistries` to `DependencyClient` to update the registries
 - adds a new flag to specify the default maven registry for `fix`
- add new experimental options to `scan` to align with what we have for
`fix`

TODO: 
 - still need to update documentation for new options/flags
 - update deps.dev Maven resolver for mutil-registry resolution
 - record not found requests to optimize performance
  • Loading branch information
cuixq authored Oct 21, 2024
1 parent a5a1e29 commit 0c596bf
Show file tree
Hide file tree
Showing 24 changed files with 478 additions and 104 deletions.
36 changes: 36 additions & 0 deletions cmd/osv-scanner/__snapshots__/main_test.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2382,6 +2382,42 @@ No issues found

---

[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_datda_source - 1]
Scanned <rootdir>/fixtures/maven-transitive/registry.xml file as a pom.xml and found 59 packages
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+
| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | SOURCE |
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+
| https://osv.dev/GHSA-cm6r-892j-jv2g | 6.1 | Maven | com.google.android.gms:play-services-basement | 10.0.0 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-7rjr-3q55-vv33 | 9.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-8489-44mv-ggj8 | 6.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-jfh8-c2jp-5v3q | 10.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-p6xc-xr62-6r2g | 8.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+

---

[TestRun_MavenTransitive/resolve_transitive_dependencies_with_native_datda_source - 2]

---

[TestRun_MavenTransitive/scans_dependencies_from_multiple_registries - 1]
Scanned <rootdir>/fixtures/maven-transitive/registry.xml file as a pom.xml and found 59 packages
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+
| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | SOURCE |
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+
| https://osv.dev/GHSA-cm6r-892j-jv2g | 6.1 | Maven | com.google.android.gms:play-services-basement | 10.0.0 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-7rjr-3q55-vv33 | 9.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-8489-44mv-ggj8 | 6.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-jfh8-c2jp-5v3q | 10.0 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
| https://osv.dev/GHSA-p6xc-xr62-6r2g | 8.6 | Maven | org.apache.logging.log4j:log4j-core | 2.14.1 | fixtures/maven-transitive/registry.xml |
+-------------------------------------+------+-----------+-----------------------------------------------+---------+----------------------------------------+

---

[TestRun_MavenTransitive/scans_dependencies_from_multiple_registries - 2]

---

[TestRun_MavenTransitive/scans_pom.xml_with_non_UTF-8_encoding - 1]
Scanned <rootdir>/fixtures/maven-transitive/encoding.xml file as a pom.xml and found 2 packages
+-------------------------------------+------+-----------+-------------+---------+----------------------------------------+
Expand Down
9 changes: 6 additions & 3 deletions cmd/osv-scanner/fix/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/datasource"
"github.com/google/osv-scanner/internal/resolution/lockfile"
"github.com/google/osv-scanner/internal/resolution/manifest"
"github.com/google/osv-scanner/pkg/reporter"
Expand Down Expand Up @@ -68,6 +67,10 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
return nil
},
},
&cli.StringFlag{
Name: "maven-registry",
Usage: "URL of the default Maven registry to fetch metadata",
},
&cli.StringFlag{
Name: "relock-cmd",
Usage: "command to run to regenerate lockfile on disk after changing the manifest",
Expand Down Expand Up @@ -279,7 +282,7 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
}

if opts.Manifest != "" {
rw, err := manifest.GetReadWriter(opts.Manifest)
rw, err := manifest.GetReadWriter(opts.Manifest, ctx.String("maven-registry"))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -312,7 +315,7 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
}
opts.Client.DependencyClient = cl
case resolve.Maven:
cl, err := client.NewMavenRegistryClient(datasource.MavenCentral)
cl, err := client.NewMavenRegistryClient(ctx.String("maven-registry"))
if err != nil {
return nil, err
}
Expand Down
16 changes: 16 additions & 0 deletions cmd/osv-scanner/fix/noninteractive.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,22 @@ func autoOverride(ctx context.Context, r reporter.Reporter, opts osvFixOptions,
return err
}

if opts.ManifestRW.System() == resolve.Maven {
// Update Maven registries based on the repositories defined in pom.xml,
// as well as the repositories merged from parent pom.xml.
// TODO: add registries defined in settings.xml
// https://github.com/google/osv-scanner/issues/1269
specific, ok := manif.EcosystemSpecific.(manifest.MavenManifestSpecific)
if ok {
registries := make([]client.Registry, len(specific.Repositories))
for i, repo := range specific.Repositories {
registries[i] = client.Registry{URL: string(repo.URL)}
}
if err := opts.Client.DependencyClient.AddRegistries(registries); err != nil {
return err
}
}
}
client.PreFetch(ctx, opts.Client, manif.Requirements, manif.FilePath)
res, err := resolution.Resolve(ctx, opts.Client, manif, opts.ResolveOpts)
if err != nil {
Expand Down
32 changes: 32 additions & 0 deletions cmd/osv-scanner/fixtures/maven-transitive/parent.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?xml version="1.0" encoding="UTF-8"?>

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.mycompany.app</groupId>
<artifactId>parent</artifactId>
<version>1.0.0</version>

<name>my-app</name>

<packaging>pom</packaging>


<dependencies>
<dependency>
<!-- depends on com.google.android.gms:play-services v10.0.0-->
<groupId>com.google.android.gms</groupId>
<artifactId>play-services</artifactId>
<version>10.0.0</version>
</dependency>
</dependencies>

<repositories>
<repository>
<id>google-android</id>
<url>https://dl.google.com/dl/android/maven2</url>
</repository>
</repositories>

</project>
28 changes: 28 additions & 0 deletions cmd/osv-scanner/fixtures/maven-transitive/registry.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?xml version="1.0" encoding="UTF-8"?>

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1.0.0</version>

<name>my-app</name>

<parent>
<groupId>com.mycompany.app</groupId>
<artifactId>parent</artifactId>
<version>1.0.0</version>
<relativePath>./parent.xml</relativePath>
</parent>

<dependencies>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-web</artifactId>
<version>2.14.1</version>
</dependency>
</dependencies>

</project>
10 changes: 10 additions & 0 deletions cmd/osv-scanner/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,16 @@ func TestRun_MavenTransitive(t *testing.T) {
args: []string{"", "--config=./fixtures/osv-scanner-empty-config.toml", "--experimental-offline", "--experimental-download-offline-databases", "./fixtures/maven-transitive/pom.xml"},
exit: 0,
},
{
name: "scans dependencies from multiple registries",
args: []string{"", "--config=./fixtures/osv-scanner-empty-config.toml", "-L", "pom.xml:./fixtures/maven-transitive/registry.xml"},
exit: 1,
},
{
name: "resolve transitive dependencies with native datda source",
args: []string{"", "--config=./fixtures/osv-scanner-empty-config.toml", "--experimental-resolution-data-source=native", "-L", "pom.xml:./fixtures/maven-transitive/registry.xml"},
exit: 1,
},
}

for _, tt := range tests {
Expand Down
20 changes: 20 additions & 0 deletions cmd/osv-scanner/scan/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,22 @@ func Command(stdout, stderr io.Writer, r *reporter.Reporter) *cli.Command {
TakesFile: true,
Hidden: true,
},
&cli.StringFlag{
Name: "experimental-resolution-data-source",
Usage: "source to fetch package information from; value can be: deps.dev, native",
Value: "deps.dev",
Action: func(_ *cli.Context, s string) error {
if s != "deps.dev" && s != "native" {
return fmt.Errorf("unsupported data-source \"%s\" - must be one of: deps.dev, native", s)
}

return nil
},
},
&cli.StringFlag{
Name: "experimental-maven-registry",
Usage: "URL of the default registry to fetch Maven metadata",
},
},
ArgsUsage: "[directory1 directory2...]",
Action: func(c *cli.Context) error {
Expand Down Expand Up @@ -228,6 +244,10 @@ func action(context *cli.Context, stdout, stderr io.Writer) (reporter.Reporter,
ScanLicensesSummary: context.Bool("experimental-licenses-summary"),
ScanLicensesAllowlist: context.StringSlice("experimental-licenses"),
ScanOCIImage: context.String("experimental-oci-image"),
TransitiveScanningActions: osvscanner.TransitiveScanningActions{
NativeDataSource: context.String("experimental-resolution-data-source") == "native",
MavenRegistry: context.String("experimental-maven-registry"),
},
},
}, r)

Expand Down
2 changes: 1 addition & 1 deletion cmd/osv-scanner/update/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func action(ctx *cli.Context, stdout, stderr io.Writer) (reporter.Reporter, erro
if err != nil {
return nil, err
}
options.ManifestRW, err = manifest.GetReadWriter(options.Manifest)
options.ManifestRW, err = manifest.GetReadWriter(options.Manifest, "")
if err != nil {
return nil, err
}
Expand Down
35 changes: 24 additions & 11 deletions internal/manifest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,38 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD

var project maven.Project
if err := datasource.NewMavenDecoder(f).Decode(&project); err != nil {
return []lockfile.PackageDetails{}, fmt.Errorf("could not extract from %s: %w", f.Path(), err)
return nil, fmt.Errorf("could not extract from %s: %w", f.Path(), err)
}
// Empty JDK and ActivationOS indicates merging the default profiles.
if err := project.MergeProfiles("", maven.ActivationOS{}); err != nil {
return nil, fmt.Errorf("failed to merge profiles: %w", err)
}
for _, repo := range project.Repositories {
if err := e.MavenRegistryAPIClient.AddRegistry(string(repo.URL)); err != nil {
return nil, fmt.Errorf("failed to add registry %s: %w", repo.URL, err)
}
}
// Merging parents data by parsing local parent pom.xml or fetching from upstream.
if err := mavenutil.MergeParents(ctx, e.MavenRegistryAPIClient, &project, project.Parent, 1, f.Path(), true); err != nil {
return []lockfile.PackageDetails{}, fmt.Errorf("failed to merge parents: %w", err)
return nil, fmt.Errorf("failed to merge parents: %w", err)
}
// Process the dependencies:
// - dedupe dependencies and dependency management
// - import dependency management
// - fill in missing dependency version requirement
project.ProcessDependencies(func(groupID, artifactID, version maven.String) (maven.DependencyManagement, error) {
root := maven.Parent{ProjectKey: maven.ProjectKey{GroupID: groupID, ArtifactID: artifactID, Version: version}}
var result maven.Project
if err := mavenutil.MergeParents(ctx, e.MavenRegistryAPIClient, &result, root, 0, f.Path(), false); err != nil {
return maven.DependencyManagement{}, err
}

return result.DependencyManagement, nil
return mavenutil.GetDependencyManagement(ctx, e.MavenRegistryAPIClient, groupID, artifactID, version)
})

if registries := e.MavenRegistryAPIClient.GetRegistries(); len(registries) > 0 {
clientRegs := make([]client.Registry, len(registries))
for i, reg := range registries {
clientRegs[i] = client.Registry{URL: reg}
}
if err := e.DependencyClient.AddRegistries(clientRegs); err != nil {
return nil, err
}
}
overrideClient := client.NewOverrideClient(e.DependencyClient)
resolver := mavenresolve.NewResolver(overrideClient)

Expand Down Expand Up @@ -93,9 +105,10 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
}
overrideClient.AddVersion(root, reqs)

client.PreFetch(ctx, overrideClient, reqs, f.Path())
g, err := resolver.Resolve(ctx, root.VersionKey)
if err != nil {
return []lockfile.PackageDetails{}, fmt.Errorf("failed resolving %v: %w", root, err)
return nil, fmt.Errorf("failed resolving %v: %w", root, err)
}
for i, e := range g.Edges {
e.Type = dep.Type{}
Expand Down Expand Up @@ -127,7 +140,7 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
func ParseMavenWithResolver(depClient client.DependencyClient, mavenClient *datasource.MavenRegistryAPIClient, pathToLockfile string) ([]lockfile.PackageDetails, error) {
f, err := lockfile.OpenLocalDepFile(pathToLockfile)
if err != nil {
return []lockfile.PackageDetails{}, err
return nil, err
}
defer f.Close()

Expand Down
18 changes: 10 additions & 8 deletions internal/manifest/maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ func TestParseMavenWithResolver_InvalidSyntax(t *testing.T) {
func TestParseMavenWithResolver_NoPackages(t *testing.T) {
t.Parallel()

packages, err := manifest.ParseMavenWithResolver(nil, nil, "fixtures/maven/empty.xml")
resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/empty.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand All @@ -104,7 +105,7 @@ func TestParseMavenWithResolver_OnePackage(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/one-package.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/one-package.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand All @@ -123,7 +124,7 @@ func TestParseMavenWithResolver_TwoPackages(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/two-packages.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/two-packages.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand All @@ -148,7 +149,7 @@ func TestParseMavenWithResolver_WithDependencyManagement(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/with-dependency-management.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/with-dependency-management.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand All @@ -173,7 +174,7 @@ func TestParseMavenWithResolver_Interpolation(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/interpolation.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/interpolation.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand Down Expand Up @@ -204,7 +205,7 @@ func TestParseMavenWithResolver_WithScope(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/with-scope.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/with-scope.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand Down Expand Up @@ -258,7 +259,8 @@ func TestParseMavenWithResolver_WithParent(t *testing.T) {
`))

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, datasource.NewMavenRegistryAPIClient(srv.URL), "fixtures/maven/with-parent.xml")
client, _ := datasource.NewMavenRegistryAPIClient(srv.URL)
packages, err := manifest.ParseMavenWithResolver(resolutionClient, client, "fixtures/maven/with-parent.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand Down Expand Up @@ -307,7 +309,7 @@ func TestParseMavenWithResolver_Transitive(t *testing.T) {
t.Parallel()

resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, nil, "fixtures/maven/transitive.xml")
packages, err := manifest.ParseMavenWithResolver(resolutionClient, &datasource.MavenRegistryAPIClient{}, "fixtures/maven/transitive.xml")
if err != nil {
t.Errorf("Got unexpected error: %v", err)
}
Expand Down
2 changes: 1 addition & 1 deletion internal/remediation/testhelpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
func parseRemediationFixture(t *testing.T, universePath, manifestPath string, opts resolution.ResolveOpts) (*resolution.Result, client.ResolutionClient) {
t.Helper()

rw, err := manifest.GetReadWriter(manifestPath)
rw, err := manifest.GetReadWriter(manifestPath, "")
if err != nil {
t.Fatalf("Failed to get ReadWriter: %v", err)
}
Expand Down
6 changes: 6 additions & 0 deletions internal/resolution/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ type DependencyClient interface {
WriteCache(filepath string) error
// LoadCache loads a manifest-specific resolution cache.
LoadCache(filepath string) error
// AddRegistries adds the specified registries to fetch data.
AddRegistries(registries []Registry) error
}

type Registry struct {
URL string
}

// PreFetch loads cache, then makes and caches likely queries needed for resolving a package with a list of requirements
Expand Down
2 changes: 2 additions & 0 deletions internal/resolution/client/depsdev_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ func NewDepsDevClient(addr string) (*DepsDevClient, error) {
return &DepsDevClient{APIClient: *resolve.NewAPIClient(c), c: c}, nil
}

func (d *DepsDevClient) AddRegistries(_ []Registry) error { return nil }

func (d *DepsDevClient) WriteCache(path string) error {
f, err := os.Create(path + depsDevCacheExt)
if err != nil {
Expand Down
Loading

0 comments on commit 0c596bf

Please sign in to comment.