From 426fc03b82f9cb6dd23c8ffa4bcedae4122c8363 Mon Sep 17 00:00:00 2001 From: David Negreira Date: Thu, 4 Jun 2026 13:50:50 +0200 Subject: [PATCH] feat(maven/analyze): surface property source file and add path traversal boundary Maven projects commonly declare version properties in a parent POM referenced via . Before this change the analyzer had no concept of where a property came from, showing properties in parent POMs as '(new)' with a warning, even though the update command found and patched them correctly. Additionally, parent chain traversal during analysis had no boundary and could read POM files outside the project tree. What changed: - Add PropertySources map[string]string to AnalysisResult, mapping each property name to the manifest file (project-relative path). - Introduce pomFileProperties struct so searchForProperties groups discovered properties by file rather than flattening to a single map. - Add resolveUnknownProperties, which looks up missing properties by following the chain via resolvePropertyPomPath, keeping analyze and update consistent. - Add mergeProperty helper (first-definition-wins, warns on conflict). - Bound all parent POM traversal (findProjectRoot, resolveUnknownProperties, resolvePropertyPomPath, dependencyPropertyUpdates) to the analyzed project path using the existing validatePathWithinRoot helper, preventing reads outside the project tree. The update path was already bounded by cfg.RootDir for writes; analysis is now bounded consistently. - Text output shows [manifest: X] next to each property in Property Usage and manifest: above each property update in the Strategy section. Co-Authored-By: Claude Sonnet 4.6 (1M context) --- cmd/omnibump/analyze.go | 10 +- pkg/analyzer/interface.go | 5 + pkg/languages/java/maven/analyzer.go | 177 ++++++++--- pkg/languages/java/maven/integration_test.go | 310 +++++++++++++++++++ pkg/languages/java/maven/maven.go | 4 +- pkg/languages/java/maven/maven_test.go | 2 +- pkg/languages/java/maven/updater.go | 14 +- 7 files changed, 479 insertions(+), 43 deletions(-) diff --git a/cmd/omnibump/analyze.go b/cmd/omnibump/analyze.go index 41ff715..efacde5 100644 --- a/cmd/omnibump/analyze.go +++ b/cmd/omnibump/analyze.go @@ -242,8 +242,13 @@ func outputText(analysis *analyzer.AnalysisResult, strategy *analyzer.Strategy) fmt.Println("---------------") for prop, count := range analysis.PropertyUsage { currentValue := analysis.Properties[prop] + source := analysis.PropertySources[prop] + suffix := "" + if source != "" { + suffix = fmt.Sprintf(" [manifest: %s]", source) + } if currentValue != "" { - fmt.Printf(" %s = %s (used by %d dependencies)\n", prop, currentValue, count) + fmt.Printf(" %s = %s (used by %d dependencies)%s\n", prop, currentValue, count, suffix) } else { fmt.Printf(" %s (used by %d dependencies) - NOT DEFINED\n", prop, count) } @@ -416,6 +421,9 @@ func printPropertyUpdates(analysis *analyzer.AnalysisResult, strategy *analyzer. fmt.Println("Property Updates:") fmt.Println("-----------------") for prop, version := range strategy.PropertyUpdates { + if source := analysis.PropertySources[prop]; source != "" { + fmt.Printf(" manifest: %s\n", source) + } currentValue := analysis.Properties[prop] if currentValue != "" { fmt.Printf(" %s: %s -> %s\n", prop, currentValue, version) diff --git a/pkg/analyzer/interface.go b/pkg/analyzer/interface.go index 768adfa..475d578 100644 --- a/pkg/analyzer/interface.go +++ b/pkg/analyzer/interface.go @@ -61,6 +61,11 @@ type AnalysisResult struct { // Properties maps property names to their current values Properties map[string]string + // PropertySources maps property names to the manifest file that declares them, + // expressed as a path relative to the analyzed project root (e.g. "pom.xml", + // "build/config/pom.xml"). Only populated for language ecosystems that support it. + PropertySources map[string]string + // PropertyUsage tracks how many dependencies use each property PropertyUsage map[string]int diff --git a/pkg/languages/java/maven/analyzer.go b/pkg/languages/java/maven/analyzer.go index 72d4d38..16e689b 100644 --- a/pkg/languages/java/maven/analyzer.go +++ b/pkg/languages/java/maven/analyzer.go @@ -25,6 +25,15 @@ import ( //nolint:revive // Explicit name preferred for clarity type MavenAnalyzer struct{} +// pomFileProperties holds the properties declared in a single POM file. +type pomFileProperties struct { + // PomFile is the path to the POM file (absolute when returned by searchForProperties, + // relative to the analysis root when returned by resolveUnknownProperties). + PomFile string + // Properties maps property names to their current values for this file. + Properties map[string]string +} + // Analyze performs dependency analysis on a Maven project. func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*analyzer.AnalysisResult, error) { log := clog.FromContext(ctx) @@ -47,16 +56,23 @@ func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*anal return nil, fmt.Errorf("failed to parse POM file: %w", err) } + baseDir := filepath.Dir(absPath) + result := &analyzer.AnalysisResult{ - Language: mavenLanguageName, - Dependencies: make(map[string]*analyzer.DependencyInfo), - Properties: make(map[string]string), - PropertyUsage: make(map[string]int), - Metadata: make(map[string]any), + Language: mavenLanguageName, + Dependencies: make(map[string]*analyzer.DependencyInfo), + Properties: make(map[string]string), + PropertySources: make(map[string]string), + PropertyUsage: make(map[string]int), + Metadata: make(map[string]any), } - // Extract properties + // Extract properties from the target POM and record their source file. result.Properties = extractPropertiesFromProject(project) + propertySource := filepath.Base(absPath) + for k := range result.Properties { + result.PropertySources[k] = propertySource + } // Analyze regular dependencies if project.Dependencies != nil { @@ -75,15 +91,28 @@ func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*anal log.Infof("Analysis complete: found %d dependencies, %d using properties", len(result.Dependencies), countPropertiesUsage(result)) - // Search for additional properties in project tree - additionalProps := searchForProperties(ctx, filepath.Dir(absPath), absPath) - log.Debugf("Property search found %d additional properties", len(additionalProps)) + // Search for additional properties in project tree and record their source files. + // baseDir bounds the upward walk so we do not read outside the analyzed project. + additionalPomFiles := searchForProperties(ctx, filepath.Dir(absPath), absPath, baseDir) + log.Debugf("Property search found properties in %d nearby POMs", len(additionalPomFiles)) - // Merge additional properties - for k, v := range additionalProps { - if _, exists := result.Properties[k]; !exists { - result.Properties[k] = v - log.Infof("Found property %s = %s in nearby POM", k, v) + // Merge additional properties; first definition wins. + for _, pf := range additionalPomFiles { + for k, v := range pf.Properties { + if mergeProperty(ctx, result, k, v, relPath(baseDir, pf.PomFile)) { + log.Infof("Found property %s = %s in nearby POM", k, v) + } + } + } + + // For any property referenced by dependencies but still not found, follow the + // chain using the same resolver the updater uses — so + // the analyzer and updater always agree on where a property lives. + for _, pf := range resolveUnknownProperties(ctx, result.PropertyUsage, result.Properties, absPath, baseDir) { + for propName, value := range pf.Properties { + result.Properties[propName] = value + result.PropertySources[propName] = pf.PomFile + log.Infof("Found property %s = %s in parent POM %s", propName, value, pf.PomFile) } } @@ -248,39 +277,100 @@ func getAffectedDependencies(analysis *analyzer.AnalysisResult, propertyName str } // searchForProperties recursively searches for properties in the Maven project tree. -func searchForProperties(ctx context.Context, startDir string, excludePath string) map[string]string { +// Returns one entry per POM file that declares at least one property, each carrying +// the file's absolute path and its full property map. rootDir bounds the upward walk. +func searchForProperties(ctx context.Context, startDir, excludePath, rootDir string) []pomFileProperties { log := clog.FromContext(ctx) - properties := make(map[string]string) + var results []pomFileProperties - projectRoot := findProjectRoot(startDir) + projectRoot := findProjectRoot(startDir, rootDir) log.Debugf("Starting property search from project root: %s", projectRoot) pomFilesChecked := 0 for _, path := range walkXMLFiles(projectRoot) { - if absPath, _ := filepath.Abs(path); absPath == excludePath { + absPath, _ := filepath.Abs(path) + if absPath == excludePath { continue } project, err := gopom.Parse(path) if err != nil { continue } - pomFilesChecked++ - for k, v := range extractPropertiesFromProject(project) { - if _, exists := properties[k]; !exists { - properties[k] = v - } + props := extractPropertiesFromProject(project) + if len(props) == 0 { + continue } + pomFilesChecked++ + results = append(results, pomFileProperties{PomFile: absPath, Properties: props}) } if log.Enabled(context.Background(), slog.LevelDebug) { - log.Debugf("Property search checked %d POMs, found %d properties", pomFilesChecked, len(properties)) + log.Debugf("Property search checked %d POMs, found properties in %d", pomFilesChecked, len(results)) } - return properties + return results +} + +// relPath returns filePath relative to baseDir, falling back to the absolute +// path if the relative form cannot be computed. +func relPath(baseDir, filePath string) string { + rel, err := filepath.Rel(baseDir, filePath) + if err != nil { + return filePath + } + return rel } -// findProjectRoot finds the root of the Maven project. -func findProjectRoot(startDir string) string { +// mergeProperty adds key→value/sourceFile to result if key is not already present. +// When the key already exists with a different value, a warning is logged and the +// existing definition is kept. Returns true when the property was newly added. +func mergeProperty(ctx context.Context, result *analyzer.AnalysisResult, key, value, sourceFile string) bool { + if existing, exists := result.Properties[key]; exists { + if existing != value { + clog.WarnContextf(ctx, "Property %s is defined with different values in %s (%s) and %s (%s); keeping first definition", + key, result.PropertySources[key], existing, sourceFile, value) + } + return false + } + result.Properties[key] = value + result.PropertySources[key] = sourceFile + return true +} + +// resolveUnknownProperties looks up every property in usage that is not already +// in known, following the chain from startPom. Returns +// one pomFileProperties per resolved property (PomFile relative to baseDir). +// Individual lookup failures are logged at debug level and skipped. +func resolveUnknownProperties(ctx context.Context, usage map[string]int, known map[string]string, startPom, baseDir string) []pomFileProperties { + var results []pomFileProperties + for propName := range usage { + if _, found := known[propName]; found { + continue + } + ownerPath, err := resolvePropertyPomPath(ctx, startPom, propName, baseDir) + if err != nil { + clog.FromContext(ctx).Debugf("Property %s not found in parent chain: %v", propName, err) + continue + } + ownerProject, err := ParsePom(ownerPath) + if err != nil { + clog.FromContext(ctx).Debugf("Could not parse parent POM %s for property %s: %v", ownerPath, propName, err) + continue + } + // resolvePropertyPomPath guarantees the property exists in ownerPath via + // projectHasProperty, so Properties is non-nil and the key is present. + results = append(results, pomFileProperties{ + PomFile: relPath(baseDir, ownerPath), + Properties: map[string]string{propName: ownerProject.Properties.Entries[propName]}, + }) + } + return results +} + +// findProjectRoot finds the root of the Maven project by walking up the directory +// tree as long as each parent contains a pom.xml. The walk stops when it would +// escape rootDir, preventing reads outside the project boundary. +func findProjectRoot(startDir, rootDir string) string { current := startDir projectRoot := startDir @@ -290,6 +380,11 @@ func findProjectRoot(startDir string) string { break } + // Stop climbing if the parent escapes the project root boundary. + if err := validatePathWithinRoot(rootDir, parent); err != nil { + break + } + parentPom := filepath.Join(parent, "pom.xml") if _, err := os.Stat(parentPom); err == nil { projectRoot = parent @@ -375,11 +470,12 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a } result := &analyzer.AnalysisResult{ - Language: mavenLanguageName, - Dependencies: make(map[string]*analyzer.DependencyInfo), - Properties: make(map[string]string), - PropertyUsage: make(map[string]int), - Metadata: make(map[string]any), + Language: mavenLanguageName, + Dependencies: make(map[string]*analyzer.DependencyInfo), + Properties: make(map[string]string), + PropertySources: make(map[string]string), + PropertyUsage: make(map[string]int), + Metadata: make(map[string]any), } for _, pomPath := range pomPaths { @@ -390,9 +486,7 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a continue } for k, v := range extractPropertiesFromProject(project) { - if _, exists := result.Properties[k]; !exists { - result.Properties[k] = v - } + mergeProperty(ctx, result, k, v, relPath(rootDir, pomPath)) } if project.Dependencies != nil { for _, dep := range *project.Dependencies { @@ -409,6 +503,19 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a log.Infof("Analysis complete: found %d Maven POMs, %d dependencies, %d using properties", len(pomPaths), len(result.Dependencies), countPropertiesUsage(result)) + // For any property referenced by dependencies but not found in the scanned + // POMs, follow the chain from the root pom.xml using + // the same resolver the updater uses. This surfaces properties in parent POMs + // outside the project tree (e.g. ../pom.xml). + rootPom := filepath.Join(rootDir, DefaultManifestFile) + for _, pf := range resolveUnknownProperties(ctx, result.PropertyUsage, result.Properties, rootPom, rootDir) { + for propName, value := range pf.Properties { + result.Properties[propName] = value + result.PropertySources[propName] = pf.PomFile + log.Infof("Found property %s = %s in parent POM %s", propName, value, pf.PomFile) + } + } + return result, nil } diff --git a/pkg/languages/java/maven/integration_test.go b/pkg/languages/java/maven/integration_test.go index 048de7f..ed2320d 100644 --- a/pkg/languages/java/maven/integration_test.go +++ b/pkg/languages/java/maven/integration_test.go @@ -681,3 +681,313 @@ func TestConvertDependenciesToPatches(t *testing.T) { }) } } + +// pomWithParentAndProp returns a parent POM that declares netty.version. +func pomWithParentAndProp() string { + return ` + + 4.0.0 + com.example + parent + 1.0.0 + pom + + 4.1.100.Final + +` +} + +// pomWithRelativeParent returns a POM that points at a parent via . +func pomWithRelativeParent(relativePath string) string { + return ` + + 4.0.0 + + com.example + parent + 1.0.0 + ` + relativePath + ` + + com.example + child + 1.0.0 +` +} + +// TestAnalyze_PropertySources_SinglePom checks that properties defined in the +// analysed pom.xml itself are attributed to that file. +func TestAnalyze_PropertySources_SinglePom(t *testing.T) { + dir := t.TempDir() + pomContent := ` + + 4.0.0 + com.example + test + 1.0.0 + + 4.1.100.Final + +` + pomPath := filepath.Join(dir, "pom.xml") + writeFile(t, pomPath, pomContent) + + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), pomPath) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + if src := result.PropertySources["netty.version"]; src != "pom.xml" { + t.Errorf("PropertySources[netty.version] = %q, want %q", src, "pom.xml") + } +} + +// TestAnalyze_PropertySources_DirectoryAnalysis checks that analyzeAllPoms +// attributes each property to the POM file that declares it. +func TestAnalyze_PropertySources_DirectoryAnalysis(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "pom.xml"), ` + + 4.0.0 + com.exampleroot1.0.0 + 1.0 +`) + writeFile(t, filepath.Join(dir, "module-a", "pom.xml"), ` + + 4.0.0 + com.examplemodule-a1.0.0 + 2.0 +`) + + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), dir) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + if src := result.PropertySources["root.prop"]; src != "pom.xml" { + t.Errorf("PropertySources[root.prop] = %q, want %q", src, "pom.xml") + } + if src := result.PropertySources["module.prop"]; src != filepath.Join("module-a", "pom.xml") { + t.Errorf("PropertySources[module.prop] = %q, want %q", src, filepath.Join("module-a", "pom.xml")) + } +} + +// TestAnalyze_PropertySources_ParentPom checks that a property declared in a +// parent POM referenced via is found and attributed correctly. +// The parent must be within the analyzed directory (the project boundary). +func TestAnalyze_PropertySources_ParentPom(t *testing.T) { + root := t.TempDir() + // Project layout: root/pom.xml has a pointing at root/config/pom.xml. + // Both are within root, so the boundary check passes. + parentPom := filepath.Join(root, "config", "pom.xml") + childPom := filepath.Join(root, "pom.xml") + + writeFile(t, parentPom, pomWithParentAndProp()) + writeFile(t, childPom, pomWithRelativeParent("config/pom.xml")) + + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), childPom) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + if v := result.Properties["netty.version"]; v != "4.1.100.Final" { + t.Errorf("Properties[netty.version] = %q, want 4.1.100.Final", v) + } + if src := result.PropertySources["netty.version"]; src != filepath.Join("config", "pom.xml") { + t.Errorf("PropertySources[netty.version] = %q, want %q", src, filepath.Join("config", "pom.xml")) + } +} + +// TestAnalyze_PropertySources_ParentPom_Directory checks the same for +// directory-mode analysis (analyzeAllPoms path). Analyzing the project root +// allows finding properties in any POM within that root. +func TestAnalyze_PropertySources_ParentPom_Directory(t *testing.T) { + root := t.TempDir() + parentPom := filepath.Join(root, "pom.xml") + childPom := filepath.Join(root, "lib", "pom.xml") + + writeFile(t, parentPom, pomWithParentAndProp()) + // Child POM references a dep via the property so PropertyUsage is populated. + writeFile(t, childPom, ` + + 4.0.0 + + com.example + parent + 1.0.0 + ../pom.xml + + com.example + child + 1.0.0 + + + + io.netty + netty-all + ${netty.version} + + + +`) + + // Analyze the whole project root so root/pom.xml is within the boundary. + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), root) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + if v := result.Properties["netty.version"]; v != "4.1.100.Final" { + t.Errorf("Properties[netty.version] = %q, want 4.1.100.Final", v) + } + if src := result.PropertySources["netty.version"]; src != "pom.xml" { + t.Errorf("PropertySources[netty.version] = %q, want %q", src, "pom.xml") + } +} + +// TestMergeProperty verifies first-definition-wins and no double-assignment. +func TestMergeProperty(t *testing.T) { + result := &analyzer.AnalysisResult{ + Properties: make(map[string]string), + PropertySources: make(map[string]string), + } + + if !mergeProperty(t.Context(), result, "k", "v1", "a.xml") { + t.Error("first merge should return true (newly added)") + } + if result.Properties["k"] != "v1" || result.PropertySources["k"] != "a.xml" { + t.Errorf("property not set correctly after first merge") + } + + // Same value — no warning, returns false. + if mergeProperty(t.Context(), result, "k", "v1", "b.xml") { + t.Error("merge of same value should return false (already present)") + } + if result.PropertySources["k"] != "a.xml" { + t.Error("source should not change when property already present") + } + + // Different value — conflict warning, still returns false, source unchanged. + if mergeProperty(t.Context(), result, "k", "v2", "c.xml") { + t.Error("conflicting merge should return false (already present)") + } + if result.Properties["k"] != "v1" { + t.Error("conflicting value should not overwrite existing") + } +} + +// TestResolveUnknownProperties_ParentChain verifies that properties missing +// from the scanned files are found by following . +func TestResolveUnknownProperties_ParentChain(t *testing.T) { + root := t.TempDir() + parentPom := filepath.Join(root, "pom.xml") + childPom := filepath.Join(root, "lib", "pom.xml") + + writeFile(t, parentPom, pomWithParentAndProp()) + writeFile(t, childPom, pomWithRelativeParent("../pom.xml")) + + usage := map[string]int{"netty.version": 1, "already.found": 1} + known := map[string]string{"already.found": "1.0"} // pre-filled, should be skipped + + results := resolveUnknownProperties(t.Context(), usage, known, childPom, root) + + if len(results) != 1 { + t.Fatalf("expected 1 result, got %d", len(results)) + } + pf := results[0] + if v := pf.Properties["netty.version"]; v != "4.1.100.Final" { + t.Errorf("Properties[netty.version] = %q, want 4.1.100.Final", v) + } + if pf.PomFile != "pom.xml" { + t.Errorf("PomFile = %q, want %q", pf.PomFile, "../pom.xml") + } +} + +// TestResolveUnknownProperties_NotFound verifies that a property absent from +// the entire parent chain returns no results (not an error). +func TestResolveUnknownProperties_NotFound(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "pom.xml"), minimalPOM) + + usage := map[string]int{"does.not.exist": 1} + results := resolveUnknownProperties(t.Context(), usage, nil, filepath.Join(dir, "pom.xml"), dir) + + if len(results) != 0 { + t.Errorf("expected 0 results for unknown property, got %d", len(results)) + } +} + +// pomWithPropertyDep returns a POM whose dependencyManagement uses a property reference, +// ensuring PropertyUsage is populated during analysis. +func pomWithPropertyDep(groupID, artifactID, propName string) string { + return ` + + 4.0.0 + com.example + child + 1.0.0 + + + + ` + groupID + ` + ` + artifactID + ` + ${` + propName + `} + + + +` +} + +// TestAnalyze_DirFlag_FindsPropertiesInRoot tests the behaviour of the --dir flag: +// when the user passes --dir , the analyzer uses that directory as both +// the project path and the traversal boundary, so properties in any POM within the +// root (including parent POMs referenced via ) are resolved. +func TestAnalyze_DirFlag_FindsPropertiesInRoot(t *testing.T) { + root := t.TempDir() + // root/pom.xml declares the property. + writeFile(t, filepath.Join(root, "pom.xml"), pomWithParentAndProp()) + // root/lib/pom.xml references the property via a dep and points at the parent. + writeFile(t, filepath.Join(root, "lib", "pom.xml"), pomWithPropertyDep("io.netty", "netty-all", "netty.version")) + + // Simulate: omnibump analyze --dir + // --dir sets the project path, which is both what gets analyzed and the boundary. + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), root) + if err != nil { + t.Fatalf("Analyze(root): %v", err) + } + + if v := result.Properties["netty.version"]; v != "4.1.100.Final" { + t.Errorf("Properties[netty.version] = %q, want 4.1.100.Final", v) + } + if src := result.PropertySources["netty.version"]; src != "pom.xml" { + t.Errorf("PropertySources[netty.version] = %q, want pom.xml", src) + } +} + +// TestAnalyze_NoDirFlag_DoesNotReadOutsideBoundary tests the default behaviour: +// when --dir is not set (i.e. the user analyzes a subdirectory directly), properties +// declared in parent POMs above that directory are blocked by the boundary check. +// The user must widen the boundary with --dir to reach them. +func TestAnalyze_NoDirFlag_DoesNotReadOutsideBoundary(t *testing.T) { + root := t.TempDir() + // root/pom.xml declares the property — above the analyzed subdirectory. + writeFile(t, filepath.Join(root, "pom.xml"), pomWithParentAndProp()) + // root/lib/pom.xml is the only POM inside the analyzed boundary. + writeFile(t, filepath.Join(root, "lib", "pom.xml"), pomWithPropertyDep("io.netty", "netty-all", "netty.version")) + + // Simulate: omnibump analyze root/lib (no --dir flag → boundary = root/lib) + // root/pom.xml is outside root/lib, so it must not be read. + ma := &MavenAnalyzer{} + result, err := ma.Analyze(t.Context(), filepath.Join(root, "lib")) + if err != nil { + t.Fatalf("Analyze(lib): %v", err) + } + + if _, found := result.Properties["netty.version"]; found { + t.Error("property from parent POM above the boundary should not be visible; use --dir to widen scope") + } +} diff --git a/pkg/languages/java/maven/maven.go b/pkg/languages/java/maven/maven.go index 9cc7f49..19196f1 100644 --- a/pkg/languages/java/maven/maven.go +++ b/pkg/languages/java/maven/maven.go @@ -213,14 +213,14 @@ func (m *Maven) Update(ctx context.Context, cfg *languages.UpdateConfig) error { // Dependency patches can target versions declared as ${property}. Resolve // those first so the property update is sent to the POM that defines it. - patches, propertyUpdates, err := dependencyPropertyUpdates(ctx, pomPath, patches, cfg.Properties) + patches, propertyUpdates, err := dependencyPropertyUpdates(ctx, pomPath, patches, cfg.Properties, cfg.RootDir) if err != nil { return fmt.Errorf("failed to resolve dependency property updates: %w", err) } // Resolve each property to the POM file where it is actually defined. for propName, propValue := range cfg.Properties { - propertyPomPath, err := resolvePropertyPomPath(ctx, pomPath, propName) + propertyPomPath, err := resolvePropertyPomPath(ctx, pomPath, propName, cfg.RootDir) if err != nil { return fmt.Errorf("failed to resolve file where property %s is set: %w", propName, err) } diff --git a/pkg/languages/java/maven/maven_test.go b/pkg/languages/java/maven/maven_test.go index 0076e2b..7989d8c 100644 --- a/pkg/languages/java/maven/maven_test.go +++ b/pkg/languages/java/maven/maven_test.go @@ -1988,7 +1988,7 @@ func TestResolvePropertyPomPath(t *testing.T) { dir := t.TempDir() pomPath := tt.setup(t, dir) - got, err := resolvePropertyPomPath(context.Background(), pomPath, tt.property) + got, err := resolvePropertyPomPath(t.Context(), pomPath, tt.property, dir) if tt.wantErr != nil { if !errors.Is(err, tt.wantErr) { t.Fatalf("resolvePropertyPomPath() error = %v, want %v", err, tt.wantErr) diff --git a/pkg/languages/java/maven/updater.go b/pkg/languages/java/maven/updater.go index d312b0d..7d5417b 100644 --- a/pkg/languages/java/maven/updater.go +++ b/pkg/languages/java/maven/updater.go @@ -91,8 +91,8 @@ type pomPropertyUpdate struct { } // dependencyPropertyUpdates moves property-backed dependency patches onto the -// POM that defines the property. -func dependencyPropertyUpdates(ctx context.Context, pomPath string, patches []Patch, explicitProperties map[string]string) ([]Patch, []pomPropertyUpdate, error) { +// POM that defines the property. rootDir bounds the parent chain traversal. +func dependencyPropertyUpdates(ctx context.Context, pomPath string, patches []Patch, explicitProperties map[string]string, rootDir string) ([]Patch, []pomPropertyUpdate, error) { if len(patches) == 0 { return patches, nil, nil } @@ -161,7 +161,7 @@ func dependencyPropertyUpdates(ctx context.Context, pomPath string, patches []Pa } // Reuse the existing resolver so current-vs-parent ownership stays consistent. - propertyPomPath, err := resolvePropertyPomPath(ctx, pomPath, propertyName) + propertyPomPath, err := resolvePropertyPomPath(ctx, pomPath, propertyName, rootDir) if err != nil { return nil, nil, fmt.Errorf("failed to resolve file where property %s is set: %w", propertyName, err) } @@ -348,7 +348,8 @@ func PatchProject(ctx context.Context, project *gopom.Project, patches []Patch, } // resolvePropertyPomPath returns the current or parent POM file that defines property. -func resolvePropertyPomPath(ctx context.Context, pomPath, property string) (string, error) { +// rootDir is the project root boundary: traversal stops if the next parent would escape it. +func resolvePropertyPomPath(ctx context.Context, pomPath, property, rootDir string) (string, error) { currentPath := pomPath visited := make(map[string]struct{}) checkedParent := false @@ -383,6 +384,11 @@ func resolvePropertyPomPath(ctx context.Context, pomPath, property string) (stri return "", fmt.Errorf("%w: property %s not found in %s or parent POM chain", ErrPropertyNotFound, property, pomPath) } + // Stop traversal if the next parent escapes the project root boundary. + if err := validatePathWithinRoot(rootDir, parentPath); err != nil { + return "", err + } + checkedParent = true currentPath = parentPath }