fix(java): correctly propagate repositories from upper POMs to dependencies (#10077)

Author: DmitriyLewen

pkg/dependency/parser/java/pom/artifact.go

@@ -37,6 +37,9 @@ type artifact struct {
 	// We need to store the file paths for root or module artifacts.
 	// For other artifacts, it will be empty.
 	RootFilePath string
+
+	// Repositories got from current POM, upper-level POMs and parent POMs.
+	Repositories []repository
 }
 
 func newArtifact(groupID, artifactID, version string, licenses []string, props map[string]string) artifact {

pkg/dependency/parser/java/pom/parse.go

@@ -136,9 +136,8 @@ func (p *Parser) Parse(ctx context.Context, r xio.ReadSeekerAt) ([]ftypes.Packag
 	// Cache root POM
 	p.cache.put(result.artifact, result)
 
-	rootArt := root.artifact()
+	rootArt := result.artifact
 	rootArt.Relationship = ftypes.RelationshipRoot
-	rootArt.RootFilePath = p.rootPath
 
 	return p.parseRoot(ctx, rootArt, set.New[string]())
 }
@@ -228,7 +227,7 @@ func (p *Parser) parseRoot(ctx context.Context, root artifact, uniqModules set.S
 
 		// Parse, cache, and enqueue modules.
 		for _, relativePath := range result.modules {
-			moduleArtifact, err := p.parseModule(ctx, result.filePath, relativePath)
+			moduleArtifact, err := p.parseModule(ctx, result.filePath, relativePath, root.Repositories)
 			if err != nil {
 				p.logger.Debug("Unable to parse the module",
 					log.FilePath(result.filePath), log.Err(err))
@@ -307,7 +306,7 @@ func depVersion(depName string, uniqArtifacts map[string]artifact) string {
 	return ""
 }
 
-func (p *Parser) parseModule(ctx context.Context, currentPath, relativePath string) (artifact, error) {
+func (p *Parser) parseModule(ctx context.Context, currentPath, relativePath string, repos []repository) (artifact, error) {
 	// modulePath: "root/" + "module/" => "root/module"
 	module, err := p.openRelativePom(currentPath, relativePath)
 	if err != nil {
@@ -316,6 +315,7 @@ func (p *Parser) parseModule(ctx context.Context, currentPath, relativePath stri
 
 	result, err := p.analyze(ctx, module, analysisOptions{
 		rootFilePath: module.filePath,
+		repositories: repos,
 	})
 	if err != nil {
 		return artifact{}, xerrors.Errorf("analyze error: %w", err)
@@ -347,17 +347,19 @@ func (p *Parser) resolve(ctx context.Context, art artifact, rootDepManagement []
 
 	p.logger.Debug("Resolving...", log.String("group_id", art.GroupID),
 		log.String("artifact_id", art.ArtifactID), log.String("version", art.Version.String()))
-	pomContent, err := p.tryRepository(ctx, art.GroupID, art.ArtifactID, art.Version.String())
+	pomContent, err := p.tryRepository(ctx, art.GroupID, art.ArtifactID, art.Version.String(), art.Repositories)
 	if err != nil {
 		if shouldReturnError(err) {
 			return analysisResult{}, err
 		}
 		p.logger.Debug("Repository error", log.Err(err))
 	}
+
 	result, err := p.analyze(ctx, pomContent, analysisOptions{
 		exclusions:    art.Exclusions,
 		depManagement: rootDepManagement,
 		rootFilePath:  art.RootFilePath,
+		repositories:  art.Repositories,
 	})
 	if err != nil {
 		return analysisResult{}, xerrors.Errorf("analyze error: %w", err)
@@ -380,6 +382,7 @@ type analysisOptions struct {
 	exclusions    set.Set[string]
 	depManagement []pomDependency // from the root POM
 	rootFilePath  string          // File path of the root POM or module POM
+	repositories  []repository    // Repositories inherited from parent
 }
 
 func (p *Parser) analyze(ctx context.Context, pom *pom, opts analysisOptions) (analysisResult, error) {
@@ -389,14 +392,16 @@ func (p *Parser) analyze(ctx context.Context, pom *pom, opts analysisOptions) (a
 	if opts.exclusions == nil {
 		opts.exclusions = set.New[string]()
 	}
-	// Update remoteRepositories
+
+	// Get repositories from current POM and merge with inherited ones
 	pomRepos := pom.repositories(p.servers)
-	p.remoteRepos.pom = lo.UniqBy(append(pomRepos, p.remoteRepos.pom...), func(r repository) url.URL {
+	opts.repositories = lo.UniqBy(append(pomRepos, opts.repositories...), func(r repository) url.URL {
 		return r.url
 	})
 
 	// Resolve parent POM
-	if err := p.resolveParent(ctx, pom); err != nil {
+	// TODO handle repos from parents
+	if err := p.resolveParent(ctx, pom, opts.repositories); err != nil {
 		return analysisResult{}, xerrors.Errorf("pom resolve error: %w", err)
 	}
 
@@ -411,6 +416,7 @@ func (p *Parser) analyze(ctx context.Context, pom *pom, opts analysisOptions) (a
 
 	art := pom.artifact()
 	art.RootFilePath = opts.rootFilePath
+	art.Repositories = opts.repositories
 
 	return analysisResult{
 		filePath:             pom.filePath,
@@ -423,13 +429,13 @@ func (p *Parser) analyze(ctx context.Context, pom *pom, opts analysisOptions) (a
 }
 
 // resolveParent resolves its parent POMs and inherits properties, dependencies, and dependencyManagement.
-func (p *Parser) resolveParent(ctx context.Context, pom *pom) error {
+func (p *Parser) resolveParent(ctx context.Context, pom *pom, pomRepos []repository) error {
 	if pom.nil() {
 		return nil
 	}
 
 	// Parse parent POM
-	parent, err := p.parseParent(ctx, pom.filePath, pom.content.Parent)
+	parent, err := p.parseParent(ctx, pom.filePath, pom.content.Parent, pomRepos)
 	if err != nil {
 		return xerrors.Errorf("parent error: %w", err)
 	}
@@ -576,7 +582,7 @@ func excludeDep(exclusions set.Set[string], art artifact) bool {
 	return false
 }
 
-func (p *Parser) parseParent(ctx context.Context, currentPath string, parent pomParent) (*pom, error) {
+func (p *Parser) parseParent(ctx context.Context, currentPath string, parent pomParent, pomRepos []repository) (*pom, error) {
 	// Pass nil properties so that variables in <parent> are not evaluated.
 	target := newArtifact(parent.GroupId, parent.ArtifactId, parent.Version, nil, nil)
 	// if version is property (e.g. ${revision}) - we still need to parse this pom
@@ -588,7 +594,7 @@ func (p *Parser) parseParent(ctx context.Context, currentPath string, parent pom
 	logger.Debug("Start parent")
 	defer logger.Debug("Exit parent")
 
-	parentPOM, err := p.retrieveParent(ctx, currentPath, parent.RelativePath, target)
+	parentPOM, err := p.retrieveParent(ctx, currentPath, parent.RelativePath, target, pomRepos)
 	if err != nil {
 		if shouldReturnError(err) {
 			return nil, err
@@ -597,34 +603,34 @@ func (p *Parser) parseParent(ctx context.Context, currentPath string, parent pom
 		return &pom{content: &pomXML{}}, nil
 	}
 
-	if err = p.resolveParent(ctx, parentPOM); err != nil {
+	if err = p.resolveParent(ctx, parentPOM, pomRepos); err != nil {
 		return nil, xerrors.Errorf("parent pom resolve error: %w", err)
 	}
 
 	return parentPOM, nil
 }
 
-func (p *Parser) retrieveParent(ctx context.Context, currentPath, relativePath string, target artifact) (*pom, error) {
+func (p *Parser) retrieveParent(ctx context.Context, currentPath, relativePath string, target artifact, pomRepos []repository) (*pom, error) {
 	var errs error
 
 	// Try relativePath
 	if relativePath != "" {
-		pom, err := p.tryRelativePath(ctx, target, currentPath, relativePath)
+		pom, err := p.tryRelativePath(ctx, target, currentPath, relativePath, pomRepos)
 		if err == nil {
 			return pom, nil
 		}
 		errs = multierror.Append(errs, err)
 	}
 
 	// If not found, search the parent director
-	pom, err := p.tryRelativePath(ctx, target, currentPath, "../pom.xml")
+	pom, err := p.tryRelativePath(ctx, target, currentPath, "../pom.xml", pomRepos)
 	if err == nil {
 		return pom, nil
 	}
 	errs = multierror.Append(errs, err)
 
 	// If not found, search local/remote remoteRepositories
-	pom, err = p.tryRepository(ctx, target.GroupID, target.ArtifactID, target.Version.String())
+	pom, err = p.tryRepository(ctx, target.GroupID, target.ArtifactID, target.Version.String(), pomRepos)
 	if err == nil {
 		return pom, nil
 	}
@@ -634,7 +640,7 @@ func (p *Parser) retrieveParent(ctx context.Context, currentPath, relativePath s
 	return nil, errs
 }
 
-func (p *Parser) tryRelativePath(ctx context.Context, parentArtifact artifact, currentPath, relativePath string) (*pom, error) {
+func (p *Parser) tryRelativePath(ctx context.Context, parentArtifact artifact, currentPath, relativePath string, pomRepos []repository) (*pom, error) {
 	parsedPOM, err := p.openRelativePom(currentPath, relativePath)
 	if err != nil {
 		return nil, err
@@ -649,7 +655,7 @@ func (p *Parser) tryRelativePath(ctx context.Context, parentArtifact artifact, c
 	if parsedPOM.artifact().ArtifactID != parentArtifact.ArtifactID {
 		return nil, xerrors.New("'parent.relativePath' points at wrong local POM")
 	}
-	if err := p.resolveParent(ctx, parsedPOM); err != nil {
+	if err := p.resolveParent(ctx, parsedPOM, pomRepos); err != nil {
 		return nil, xerrors.Errorf("analyze error: %w", err)
 	}
 
@@ -699,7 +705,7 @@ func (p *Parser) openPom(filePath string) (*pom, error) {
 	}, nil
 }
 
-func (p *Parser) tryRepository(ctx context.Context, groupID, artifactID, version string) (*pom, error) {
+func (p *Parser) tryRepository(ctx context.Context, groupID, artifactID, version string, pomRepos []repository) (*pom, error) {
 	if version == "" {
 		return nil, xerrors.Errorf("Version missing for %s:%s", groupID, artifactID)
 	}
@@ -717,7 +723,7 @@ func (p *Parser) tryRepository(ctx context.Context, groupID, artifactID, version
 	}
 
 	// Search remote remoteRepositories
-	loaded, err = p.fetchPOMFromRemoteRepositories(ctx, paths, isSnapshot(version))
+	loaded, err = p.fetchPOMFromRemoteRepositories(ctx, paths, isSnapshot(version), pomRepos)
 	if err == nil {
 		return loaded, nil
 		// We should return error if it's not "not found" error
@@ -735,7 +741,7 @@ func (p *Parser) loadPOMFromLocalRepository(paths []string) (*pom, error) {
 	return p.openPom(localPath)
 }
 
-func (p *Parser) fetchPOMFromRemoteRepositories(ctx context.Context, paths []string, snapshot bool) (*pom, error) {
+func (p *Parser) fetchPOMFromRemoteRepositories(ctx context.Context, paths []string, snapshot bool, pomRepos []repository) (*pom, error) {
 	// Do not try fetching pom.xml from remote repositories in offline mode
 	if p.offline {
 		p.logger.Debug("Fetching the remote pom.xml is skipped")
@@ -744,9 +750,9 @@ func (p *Parser) fetchPOMFromRemoteRepositories(ctx context.Context, paths []str
 
 	// Try all remoteRepositories by following order:
 	// 1. remoteRepositories from settings.xml
-	// 2. remoteRepositories from pom.xml
+	// 2. remoteRepositories from pom.xml (passed as parameter)
 	// 3. default remoteRepository (Maven Central for Release repository)
-	for _, repo := range slices.Concat(p.remoteRepos.settings, p.remoteRepos.pom, []repository{p.remoteRepos.defaultRepo}) {
+	for _, repo := range slices.Concat(p.remoteRepos.settings, pomRepos, []repository{p.remoteRepos.defaultRepo}) {
 		// Skip Release only repositories for snapshot artifacts and vice versa
 		if snapshot && !repo.snapshotEnabled || !snapshot && !repo.releaseEnabled {
 			continue

pkg/dependency/parser/java/pom/parse_test.go

@@ -1,15 +1,19 @@
 package pom_test
 
 import (
+	"bytes"
 	"fmt"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"testing"
 
+	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/tools/txtar"
 
 	"github.com/aquasecurity/trivy/pkg/dependency/parser/java/pom"
 	ftypes "github.com/aquasecurity/trivy/pkg/fanal/types"
@@ -2488,3 +2492,118 @@ func TestPom_Parse(t *testing.T) {
 		})
 	}
 }
+
+// TestPom_Parse_Remote_Repos verifies that POM files are fetched from the correct repositories.
+// The verification is done through the license field, as it's the only way to deterministically
+// identify which repository an artifact came from.
+func TestPom_Parse_Remote_Repos(t *testing.T) {
+	const rootRepoPlaceholder = "REPO_ROOT_URL"
+
+	tests := []struct {
+		name          string
+		inputFile     string
+		rootRepoTxtar string            // txtar file for root repository
+		repos         map[string]string // key: repository URL placeholder, value: txtar file path
+		wantPackages  map[string]string
+	}{
+		{
+			name:          "different repos for different dependencies",
+			inputFile:     filepath.Join("testdata", "different-repos-for-different-poms", "pom.xml"),
+			rootRepoTxtar: filepath.Join("testdata", "different-repos-for-different-poms", "repo-root-artifacts.txtar"),
+			repos: map[string]string{
+				"REPO1_URL": filepath.Join("testdata", "different-repos-for-different-poms", "repo1-artifacts.txtar"),
+				"REPO2_URL": filepath.Join("testdata", "different-repos-for-different-poms", "repo2-artifacts.txtar"),
+			},
+			wantPackages: map[string]string{
+				"org.example:example-api:1.7.30::2cbe1ca4": "License from repo1",
+				"org.example:example-api2:1.0.0::f8958ec7": "License from repo2",
+				"org.example:example-api3:1.0.0::887fc940": "License from reporoot",
+			},
+		},
+		{
+			name:          "root POM with module inherits repository",
+			inputFile:     filepath.Join("testdata", "repo-from-root-for-dep-from-module", "pom.xml"),
+			rootRepoTxtar: filepath.Join("testdata", "repo-from-root-for-dep-from-module", "repo-artifacts.txtar"),
+			repos:         nil,
+			wantPackages: map[string]string{
+				"org.example:example-api:1.0.0::4a790a84": "License from root repo",
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Setup remote repositories from txtar files
+			repoURLs := make(map[string]string)
+			for placeholder, txtarPath := range tt.repos {
+				repoURL := setupTxtarRepository(t, txtarPath, nil)
+				repoURLs[placeholder] = repoURL
+			}
+
+			// Setup root repository with replacements for repo placeholders
+			rootRepoURL := setupTxtarRepository(t, tt.rootRepoTxtar, repoURLs)
+
+			// Prepare input POM file with root repository URL
+			pomContent := applyReplacements(t, tt.inputFile, map[string]string{
+				rootRepoPlaceholder: rootRepoURL,
+			})
+
+			// Parse the POM
+			parser := pom.NewParser(tt.inputFile)
+			pkgs, _, err := parser.Parse(t.Context(), bytes.NewReader(pomContent))
+			require.NoError(t, err)
+
+			// Verify expected packages
+			for pkgID, wantLicense := range tt.wantPackages {
+				p, found := lo.Find(pkgs, func(pkg ftypes.Package) bool {
+					return pkg.ID == pkgID
+				})
+				require.True(t, found, "package %s not found", pkgID)
+				require.NotEmpty(t, p.Licenses, "package %s has no licenses", pkgID)
+				require.Equal(t, wantLicense, p.Licenses[0])
+			}
+		})
+	}
+}
+
+// applyReplacements reads a file, applies replacements, and returns the modified content.
+func applyReplacements(t *testing.T, filePath string, replacements map[string]string) []byte {
+	t.Helper()
+
+	content, err := os.ReadFile(filePath)
+	require.NoError(t, err)
+
+	for oldURL, newURL := range replacements {
+		content = bytes.ReplaceAll(content, []byte(oldURL), []byte(newURL))
+	}
+
+	return content
+}
+
+// txtarWithReposReplace reads a txtar file, applies repository URL replacements,
+// and returns the result as a fs.FS.
+func txtarWithReposReplace(t *testing.T, txtarPath string, reposReplacements map[string]string) fs.FS {
+	t.Helper()
+
+	content := applyReplacements(t, txtarPath, reposReplacements)
+
+	archive := txtar.Parse(content)
+
+	fsys, err := txtar.FS(archive)
+	require.NoError(t, err)
+
+	return fsys
+}
+
+// setupTxtarRepository reads a txtar file, applies repository URL replacements,
+// starts an HTTP test server with the files, and returns the server URL.
+func setupTxtarRepository(t *testing.T, txtarPath string, reposReplacements map[string]string) string {
+	t.Helper()
+
+	fsys := txtarWithReposReplace(t, txtarPath, reposReplacements)
+
+	ts := httptest.NewServer(http.FileServer(http.FS(fsys)))
+	t.Cleanup(ts.Close)
+
+	return ts.URL
+}