Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move Maven utility to a separate package #1193

Merged
merged 7 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cmd/osv-scanner/__snapshots__/main_test.snap
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ Scanned <rootdir>/fixtures/sbom-insecure/postgres-stretch.cdx.xml as CycloneDX S
| https://osv.dev/GHSA-v95c-p5hm-xq8f | 6.0 | Go | github.com/opencontainers/runc | v1.0.1 | fixtures/sbom-insecure/postgres-stretch.cdx.xml |
| https://osv.dev/GO-2022-0274 | | | | | |
| https://osv.dev/GHSA-f3fp-gc8g-vw66 | 5.9 | Go | github.com/opencontainers/runc | v1.0.1 | fixtures/sbom-insecure/postgres-stretch.cdx.xml |
| https://osv.dev/GO-2022-0452 | | | | | |
| https://osv.dev/GHSA-g2j6-57v7-gm8c | 6.1 | Go | github.com/opencontainers/runc | v1.0.1 | fixtures/sbom-insecure/postgres-stretch.cdx.xml |
| https://osv.dev/GO-2023-1683 | | | | | |
| https://osv.dev/GHSA-m8cg-xc2p-r3fc | 2.5 | Go | github.com/opencontainers/runc | v1.0.1 | fixtures/sbom-insecure/postgres-stretch.cdx.xml |
Expand Down
122 changes: 4 additions & 118 deletions internal/manifest/maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ package manifest
import (
"context"
"encoding/xml"
"errors"
"fmt"
"os"
"path/filepath"

"deps.dev/util/maven"
Expand All @@ -15,17 +13,11 @@ import (
"github.com/google/osv-scanner/internal/resolution/client"
"github.com/google/osv-scanner/internal/resolution/datasource"
"github.com/google/osv-scanner/internal/resolution/util"
mavenutil "github.com/google/osv-scanner/internal/utility/maven"
"github.com/google/osv-scanner/pkg/lockfile"
"golang.org/x/exp/maps"
)

const (
OriginManagement = "management"
OriginParent = "parent"
OriginPlugin = "plugin"
OriginProfile = "profile"
)

type MavenResolverExtractor struct {
client.DependencyClient
datasource.MavenRegistryAPIClient
Expand All @@ -43,7 +35,7 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
return []lockfile.PackageDetails{}, fmt.Errorf("could not extract from %s: %w", f.Path(), err)
}
// Merging parents data by parsing local parent pom.xml or fetching from upstream.
if err := MergeMavenParents(ctx, e.MavenRegistryAPIClient, &project, project.Parent, 1, f.Path(), true); err != nil {
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)
}
// Process the dependencies:
Expand All @@ -53,7 +45,7 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
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 := MergeMavenParents(ctx, e.MavenRegistryAPIClient, &result, root, 0, f.Path(), false); err != nil {
if err := mavenutil.MergeParents(ctx, e.MavenRegistryAPIClient, &result, root, 0, f.Path(), false); err != nil {
return maven.DependencyManagement{}, err
}

Expand Down Expand Up @@ -97,7 +89,7 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
VersionType: resolve.Requirement,
Version: string(d.Version),
},
Type: resolve.MavenDepType(d, OriginManagement),
Type: resolve.MavenDepType(d, mavenutil.OriginManagement),
}
}
overrideClient.AddVersion(root, reqs)
Expand Down Expand Up @@ -133,112 +125,6 @@ func (e MavenResolverExtractor) Extract(f lockfile.DepFile) ([]lockfile.PackageD
return maps.Values(details), nil
}

// MaxParent sets a limit on the number of parents to avoid indefinite loop.
const MaxParent = 100

// MergeMavenParents 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 MergeMavenParents(ctx context.Context, mavenClient datasource.MavenRegistryAPIClient, result *maven.Project, current maven.Parent, start int, path string, allowLocal bool) 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
parentFound := false
if parentPath := MavenParentPOMPath(currentPath, string(current.RelativePath)); allowLocal && parentPath != "" {
currentPath = parentPath
f, err := os.Open(parentPath)
if err != nil {
return fmt.Errorf("failed to open parent file %s: %w", parentPath, err)
}
if err := xml.NewDecoder(f).Decode(&proj); err != nil {
return fmt.Errorf("failed to unmarshal project: %w", err)
}
if MavenProjectKey(proj) == current.ProjectKey && proj.Packaging == "pom" {
// Only mark parent is found when the identifiers and packaging are exptected.
parentFound = true
}
}
if !parentFound {
// 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 = mavenClient.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)
}
if MavenProjectKey(proj) != current.ProjectKey {
// The identifiers in parent does not match what we want.
return fmt.Errorf("parent identifiers mismatch: %v, expect %v", proj.ProjectKey, current.ProjectKey)
}
}
// Empty JDK and ActivationOS indicates merging the default profiles.
if err := proj.MergeProfiles("", maven.ActivationOS{}); err != nil {
return err
}
result.MergeParent(proj)
current = proj.Parent
}
// Interpolate the project to resolve the properties.
return result.Interpolate()
}

// MavenProjectKey returns a project key with empty groupId/version
// filled by corresponding fields in parent.
func MavenProjectKey(proj maven.Project) maven.ProjectKey {
if proj.GroupID == "" {
proj.GroupID = proj.Parent.GroupID
}
if proj.Version == "" {
proj.Version = proj.Parent.Version
}

return proj.ProjectKey
}

// Maven looks for the parent POM first in 'relativePath',
// then the local repository '../pom.xml',
// and lastly in the remote repo.
func MavenParentPOMPath(currentPath, relativePath string) string {
if relativePath == "" {
relativePath = "../pom.xml"
}
path := filepath.Join(filepath.Dir(currentPath), relativePath)
if info, err := os.Stat(path); err == nil {
if !info.IsDir() {
return path
}
// Current path is a directory, so look for pom.xml in the directory.
path = filepath.Join(path, "pom.xml")
if _, err := os.Stat(path); err == nil {
return path
}
}

return ""
}

func ParseMavenWithResolver(depClient client.DependencyClient, mavenClient datasource.MavenRegistryAPIClient, pathToLockfile string) ([]lockfile.PackageDetails, error) {
f, err := lockfile.OpenLocalDepFile(pathToLockfile)
if err != nil {
Expand Down
64 changes: 0 additions & 64 deletions internal/manifest/maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package manifest_test

import (
"io/fs"
"os"
"path/filepath"
"testing"

"github.com/google/osv-scanner/internal/manifest"
Expand Down Expand Up @@ -360,65 +358,3 @@ func TestParseMavenWithResolver_Transitive(t *testing.T) {
},
})
}

func TestParentPOMPath(t *testing.T) {
t.Parallel()
dir, err := os.Getwd()
if err != nil {
t.Fatalf("failed to get current directory: %v", err)
}
tests := []struct {
currentPath, relativePath string
want string
}{
// fixtures
// |- maven
// | |- my-app
// | | |- pom.xml
// | |- parent
// | | |- pom.xml
// |- pom.xml
{
// Parent path is specified correctly.
currentPath: filepath.Join(dir, "fixtures", "maven", "my-app", "pom.xml"),
relativePath: "../parent/pom.xml",
want: filepath.Join(dir, "fixtures", "maven", "parent", "pom.xml"),
},
{
// Wrong file name is specified in relative path.
currentPath: filepath.Join(dir, "fixtures", "maven", "my-app", "pom.xml"),
relativePath: "../parent/abc.xml",
want: "",
},
{
// Wrong directory is specified in relative path.
currentPath: filepath.Join(dir, "fixtures", "maven", "my-app", "pom.xml"),
relativePath: "../not-found/pom.xml",
want: "",
},
{
// Only directory is specified.
currentPath: filepath.Join(dir, "fixtures", "maven", "my-app", "pom.xml"),
relativePath: "../parent",
want: filepath.Join(dir, "fixtures", "maven", "parent", "pom.xml"),
},
{
// Parent relative path is default to '../pom.xml'.
currentPath: filepath.Join(dir, "fixtures", "maven", "my-app", "pom.xml"),
relativePath: "",
want: filepath.Join(dir, "fixtures", "maven", "pom.xml"),
},
{
// No pom.xml is found even in the default path.
currentPath: filepath.Join(dir, "fixtures", "maven", "pom.xml"),
relativePath: "",
want: "",
},
}
for _, test := range tests {
got := manifest.MavenParentPOMPath(test.currentPath, test.relativePath)
if got != test.want {
t.Errorf("parentPOMPath(%s, %s): got %s, want %s", test.currentPath, test.relativePath, got, test.want)
}
}
}
16 changes: 8 additions & 8 deletions internal/remediation/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import (

"deps.dev/util/resolve"
"deps.dev/util/resolve/dep"
"github.com/google/osv-scanner/internal/manifest"
"github.com/google/osv-scanner/internal/remediation/upgrade"
"github.com/google/osv-scanner/internal/resolution"
"github.com/google/osv-scanner/internal/resolution/client"
resolutionmanifest "github.com/google/osv-scanner/internal/resolution/manifest"
"github.com/google/osv-scanner/internal/resolution/manifest"
"github.com/google/osv-scanner/internal/resolution/util"
"github.com/google/osv-scanner/internal/utility/maven"
"github.com/google/osv-scanner/internal/utility/vulns"
)

Expand Down Expand Up @@ -79,9 +79,9 @@ func ComputeOverridePatches(ctx context.Context, cl client.ResolutionClient, res
// CalculateDiff does not compute override manifest patches correctly, manually fill it out.
// TODO: CalculateDiff maybe should not be reconstructing patches.
// Refactor CalculateDiff, Relaxer, Override to make patches in a more sane way.
diff.Deps = make([]resolutionmanifest.DependencyPatch, len(res.patches))
diff.Deps = make([]manifest.DependencyPatch, len(res.patches))
for i, p := range res.patches {
diff.Deps[i] = resolutionmanifest.DependencyPatch{
diff.Deps[i] = manifest.DependencyPatch{
Pkg: p.PackageKey,
Type: dep.Type{},
OrigRequire: "", // Using empty original to signal this is an override patch
Expand Down Expand Up @@ -280,9 +280,9 @@ func getVersionsGreater(ctx context.Context, cl client.DependencyClient, vk reso
}

// patchManifest applies the overridePatches to the manifest in-memory. Returns a copy of the manifest that has been patched.
func patchManifest(patches []overridePatch, m resolutionmanifest.Manifest) (resolutionmanifest.Manifest, error) {
func patchManifest(patches []overridePatch, m manifest.Manifest) (manifest.Manifest, error) {
if m.System() != resolve.Maven {
return resolutionmanifest.Manifest{}, errors.New("unsupported ecosystem")
return manifest.Manifest{}, errors.New("unsupported ecosystem")
}

// TODO: The overridePatch does not have an artifact's type or classifier, which is part of what uniquely identifies them.
Expand All @@ -301,7 +301,7 @@ func patchManifest(patches []overridePatch, m resolutionmanifest.Manifest) (reso
continue
}
origin, hasOrigin := r.Type.GetAttr(dep.MavenDependencyOrigin)
if !hasOrigin || origin == manifest.OriginManagement {
if !hasOrigin || origin == maven.OriginManagement {
found = true
r.Version = p.NewVersion
patched.Requirements[i] = r
Expand All @@ -317,7 +317,7 @@ func patchManifest(patches []overridePatch, m resolutionmanifest.Manifest) (reso
VersionType: resolve.Requirement,
},
}
newReq.Type.AddAttr(dep.MavenDependencyOrigin, manifest.OriginManagement)
newReq.Type.AddAttr(dep.MavenDependencyOrigin, maven.OriginManagement)
patched.Requirements = append(patched.Requirements, newReq)
}
}
Expand Down
Loading
Loading