diff --git a/internal/manifest/fixtures/maven/parent/pom.xml b/internal/manifest/fixtures/maven/parent/pom.xml new file mode 100644 index 0000000000..a673b2faea --- /dev/null +++ b/internal/manifest/fixtures/maven/parent/pom.xml @@ -0,0 +1,19 @@ + + com.mycompany.app + my-app + 1.0 + + + org.upstream + parent-pom + 1.0 + + + + + org.dave + dave + 4.0.0 + + + diff --git a/internal/manifest/fixtures/maven/with-parent.xml b/internal/manifest/fixtures/maven/with-parent.xml new file mode 100644 index 0000000000..602b8b877f --- /dev/null +++ b/internal/manifest/fixtures/maven/with-parent.xml @@ -0,0 +1,54 @@ + + com.mycompany.app + my-app + 1.0 + + + org.local + parent-pom + 1.0 + ./parent/pom.xml + + + + 2.0.0 + + + + + org.alice + alice + 1.0.0 + + + org.bob + bob + ${bob.version} + + + org.chuck + chuck + + + org.frank + frank + + + + + + + org.chuck + chuck + 3.0.0 + + + org.import + import + 1.2.3 + pom + import + + + + diff --git a/internal/manifest/fixtures/universe/basic-universe.yaml b/internal/manifest/fixtures/universe/basic-universe.yaml index 4252d450ee..3a23d791b3 100644 --- a/internal/manifest/fixtures/universe/basic-universe.yaml +++ b/internal/manifest/fixtures/universe/basic-universe.yaml @@ -7,8 +7,16 @@ schema: | 4.1.42.Final junit:junit 4.12 + org.alice:alice + 1.0.0 org.apache.maven:maven-artifact 1.0.0 + org.bob:bob + 2.0.0 + org.chuck:chuck + 3.0.0 + org.dave:dave + 4.0.0 org.direct:alice 1.0.0 org.transitive:chuck@1.1.1 @@ -16,6 +24,10 @@ schema: | org.direct:bob 2.0.0 org.transitive:eve@3.3.3 + org.eve:eve + 5.0.0 + org.frank:frank + 6.0.0 org.mine:my.package 2.3.4 org.mine:mypackage diff --git a/internal/manifest/maven.go b/internal/manifest/maven.go index bd1bfcd344..09b38d6736 100644 --- a/internal/manifest/maven.go +++ b/internal/manifest/maven.go @@ -3,7 +3,9 @@ package manifest import ( "context" "encoding/xml" + "errors" "fmt" + "os" "path/filepath" "deps.dev/util/maven" @@ -11,6 +13,7 @@ import ( "deps.dev/util/resolve/dep" mavenresolve "deps.dev/util/resolve/maven" "github.com/google/osv-scanner/internal/resolution/client" + "github.com/google/osv-scanner/internal/resolution/datasource" "github.com/google/osv-scanner/internal/resolution/util" "github.com/google/osv-scanner/pkg/lockfile" "golang.org/x/exp/maps" @@ -18,6 +21,7 @@ import ( type MavenResolverExtractor struct { client.DependencyClient + datasource.MavenRegistryAPIClient } func (e MavenResolverExtractor) ShouldExtract(path string) bool { @@ -31,9 +35,23 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD if err := xml.NewDecoder(f).Decode(&project); err != nil { return []lockfile.PackageDetails{}, fmt.Errorf("could not extract from %s: %w", f.Path(), err) } - if err := project.Interpolate(); err != nil { - return []lockfile.PackageDetails{}, fmt.Errorf("could not interpolate Maven project %s: %w", project.ProjectKey.Name(), err) + // Merging parents data by parsing local parent pom.xml or fetching from upstream. + if err := e.mergeParents(ctx, &project, project.Parent, 1, true, f.Path()); err != nil { + return []lockfile.PackageDetails{}, 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 := e.mergeParents(ctx, &result, root, 0, false, f.Path()); err != nil { + return maven.DependencyManagement{}, err + } + + return result.DependencyManagement, nil + }) overrideClient := client.NewOverrideClient(e.DependencyClient) resolver := mavenresolve.NewResolver(overrideClient) @@ -95,12 +113,73 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD return maps.Values(details), nil } -func ParseMavenWithResolver(depClient client.DependencyClient, pathToLockfile string) ([]lockfile.PackageDetails, error) { +// MaxParent sets a limit on the number of parents to avoid indefinite loop. +const MaxParent = 100 + +// mergeParents parses local accessible parent pom.xml or fetches it from +// upstream, merges into root project, then interpolate the properties. +// result holds the merged Maven project. +// current holds the current parent project to merge. +// start indicates the index of the current parent project, which is used to +// check if the packaging has to be `pom`. +// allowLocal indicates whether parsing local parent pom.xml is allowed. +// path holds the path to the current pom.xml, which is used to compute the +// relative path of parent. +func (e MavenResolverExtractor) mergeParents(ctx context.Context, result *maven.Project, current maven.Parent, start int, allowLocal bool, path string) error { + currentPath := path + visited := make(map[maven.ProjectKey]bool, MaxParent) + for n := start; n < MaxParent; n++ { + if current.GroupID == "" || current.ArtifactID == "" || current.Version == "" { + break + } + if visited[current.ProjectKey] { + // A cycle of parents is detected + return errors.New("a cycle of parents is detected") + } + visited[current.ProjectKey] = true + + var proj maven.Project + if allowLocal && current.RelativePath != "" { + currentPath = filepath.Join(filepath.Dir(currentPath), string(current.RelativePath)) + if filepath.Base(currentPath) != "pom.xml" { + // If the base is not pom.xml, this path is a directory but not a file. + currentPath = filepath.Join(currentPath, "pom.xml") + } + f, err := os.Open(currentPath) + if err != nil { + return fmt.Errorf("failed to open parent file %s: %w", current.RelativePath, err) + } + if err := xml.NewDecoder(f).Decode(&proj); err != nil { + return fmt.Errorf("failed to unmarshal project: %w", err) + } + } else { + // Once we fetch a parent pom.xml from upstream, we should not + // allow parsing parent pom.xml locally anymore. + allowLocal = false + + var err error + proj, err = e.MavenRegistryAPIClient.GetProject(ctx, string(current.GroupID), string(current.ArtifactID), string(current.Version)) + if err != nil { + return fmt.Errorf("failed to get Maven project %s:%s:%s: %w", current.GroupID, current.ArtifactID, current.Version, err) + } + if n > 0 && proj.Packaging != "pom" { + // A parent project should only be of "pom" packaging type. + return fmt.Errorf("invalid packaging for parent project %s", proj.Packaging) + } + } + result.MergeParent(proj) + current = proj.Parent + } + // Interpolate the project to resolve the properties. + return result.Interpolate() +} + +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 } defer f.Close() - return MavenResolverExtractor{DependencyClient: depClient}.Extract(f) + return MavenResolverExtractor{DependencyClient: depClient, MavenRegistryAPIClient: mavenClient}.Extract(f) } diff --git a/internal/manifest/maven_test.go b/internal/manifest/maven_test.go index 06a8665922..29bad443cf 100644 --- a/internal/manifest/maven_test.go +++ b/internal/manifest/maven_test.go @@ -6,6 +6,8 @@ import ( "github.com/google/osv-scanner/internal/manifest" "github.com/google/osv-scanner/internal/resolution/clienttest" + "github.com/google/osv-scanner/internal/resolution/datasource" + "github.com/google/osv-scanner/internal/testutility" "github.com/google/osv-scanner/pkg/lockfile" ) @@ -64,7 +66,7 @@ func TestMavenResolverExtractor_ShouldExtract(t *testing.T) { func TestParseMavenWithResolver_FileDoesNotExist(t *testing.T) { t.Parallel() - packages, err := manifest.ParseMavenWithResolver(nil, "fixtures/maven/does-not-exist") + packages, err := manifest.ParseMavenWithResolver(nil, datasource.MavenRegistryAPIClient{}, "fixtures/maven/does-not-exist") expectErrIs(t, err, fs.ErrNotExist) expectPackages(t, packages, []lockfile.PackageDetails{}) @@ -73,7 +75,7 @@ func TestParseMavenWithResolver_FileDoesNotExist(t *testing.T) { func TestParseMavenWithResolver_Invalid(t *testing.T) { t.Parallel() - packages, err := manifest.ParseMavenWithResolver(nil, "fixtures/maven/not-pom.txt") + packages, err := manifest.ParseMavenWithResolver(nil, datasource.MavenRegistryAPIClient{}, "fixtures/maven/not-pom.txt") expectErrContaining(t, err, "could not extract from") expectPackages(t, packages, []lockfile.PackageDetails{}) @@ -82,7 +84,7 @@ func TestParseMavenWithResolver_Invalid(t *testing.T) { func TestParseMavenWithResolver_InvalidSyntax(t *testing.T) { t.Parallel() - packages, err := manifest.ParseMavenWithResolver(nil, "fixtures/maven/invalid-syntax.xml") + packages, err := manifest.ParseMavenWithResolver(nil, datasource.MavenRegistryAPIClient{}, "fixtures/maven/invalid-syntax.xml") expectErrContaining(t, err, "XML syntax error") expectPackages(t, packages, []lockfile.PackageDetails{}) @@ -91,7 +93,7 @@ func TestParseMavenWithResolver_InvalidSyntax(t *testing.T) { func TestParseMavenWithResolver_NoPackages(t *testing.T) { t.Parallel() - packages, err := manifest.ParseMavenWithResolver(nil, "fixtures/maven/empty.xml") + packages, err := manifest.ParseMavenWithResolver(nil, datasource.MavenRegistryAPIClient{}, "fixtures/maven/empty.xml") if err != nil { t.Errorf("Got unexpected error: %v", err) } @@ -103,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, "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) } @@ -122,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, "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) } @@ -147,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, "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) } @@ -172,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, "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) } @@ -203,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, "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) } @@ -219,11 +221,94 @@ func TestParseMavenWithResolver_WithScope(t *testing.T) { }) } +func TestParseMavenWithResolver_WithParent(t *testing.T) { + t.Parallel() + + srv := testutility.NewMockHTTPServer(t) + srv.SetResponse(t, "org/upstream/parent-pom/1.0/parent-pom-1.0.pom", []byte(` + + org.upstream + parent-pom + 1.0 + pom + + + org.eve + eve + 5.0.0 + + + + `)) + srv.SetResponse(t, "org/import/import/1.2.3/import-1.2.3.pom", []byte(` + + org.import + import + 1.2.3 + pom + + + + org.frank + frank + 6.0.0 + + + + + `)) + + resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml") + packages, err := manifest.ParseMavenWithResolver(resolutionClient, *datasource.NewMavenRegistryAPIClient(srv.URL), "fixtures/maven/with-parent.xml") + if err != nil { + t.Errorf("Got unexpected error: %v", err) + } + + expectPackages(t, packages, []lockfile.PackageDetails{ + { + Name: "org.alice:alice", + Version: "1.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + { + Name: "org.bob:bob", + Version: "2.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + { + Name: "org.chuck:chuck", + Version: "3.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + { + Name: "org.dave:dave", + Version: "4.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + { + Name: "org.eve:eve", + Version: "5.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + { + Name: "org.frank:frank", + Version: "6.0.0", + Ecosystem: lockfile.MavenEcosystem, + CompareAs: lockfile.MavenEcosystem, + }, + }) +} + func TestParseMavenWithResolver_Transitive(t *testing.T) { t.Parallel() resolutionClient := clienttest.NewMockResolutionClient(t, "fixtures/universe/basic-universe.yaml") - packages, err := manifest.ParseMavenWithResolver(resolutionClient, "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) }