Skip to content

Commit 426fc03

Browse files
dnegreiraclaude
andcommitted
feat(maven/analyze): surface property source file and add path traversal boundary
Maven projects commonly declare version properties in a parent POM referenced via <parent><relativePath>. 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 <parent><relativePath> 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) <noreply@anthropic.com>
1 parent 7aed66d commit 426fc03

7 files changed

Lines changed: 479 additions & 43 deletions

File tree

cmd/omnibump/analyze.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,13 @@ func outputText(analysis *analyzer.AnalysisResult, strategy *analyzer.Strategy)
242242
fmt.Println("---------------")
243243
for prop, count := range analysis.PropertyUsage {
244244
currentValue := analysis.Properties[prop]
245+
source := analysis.PropertySources[prop]
246+
suffix := ""
247+
if source != "" {
248+
suffix = fmt.Sprintf(" [manifest: %s]", source)
249+
}
245250
if currentValue != "" {
246-
fmt.Printf(" %s = %s (used by %d dependencies)\n", prop, currentValue, count)
251+
fmt.Printf(" %s = %s (used by %d dependencies)%s\n", prop, currentValue, count, suffix)
247252
} else {
248253
fmt.Printf(" %s (used by %d dependencies) - NOT DEFINED\n", prop, count)
249254
}
@@ -416,6 +421,9 @@ func printPropertyUpdates(analysis *analyzer.AnalysisResult, strategy *analyzer.
416421
fmt.Println("Property Updates:")
417422
fmt.Println("-----------------")
418423
for prop, version := range strategy.PropertyUpdates {
424+
if source := analysis.PropertySources[prop]; source != "" {
425+
fmt.Printf(" manifest: %s\n", source)
426+
}
419427
currentValue := analysis.Properties[prop]
420428
if currentValue != "" {
421429
fmt.Printf(" %s: %s -> %s\n", prop, currentValue, version)

pkg/analyzer/interface.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ type AnalysisResult struct {
6161
// Properties maps property names to their current values
6262
Properties map[string]string
6363

64+
// PropertySources maps property names to the manifest file that declares them,
65+
// expressed as a path relative to the analyzed project root (e.g. "pom.xml",
66+
// "build/config/pom.xml"). Only populated for language ecosystems that support it.
67+
PropertySources map[string]string
68+
6469
// PropertyUsage tracks how many dependencies use each property
6570
PropertyUsage map[string]int
6671

pkg/languages/java/maven/analyzer.go

Lines changed: 142 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ import (
2525
//nolint:revive // Explicit name preferred for clarity
2626
type MavenAnalyzer struct{}
2727

28+
// pomFileProperties holds the properties declared in a single POM file.
29+
type pomFileProperties struct {
30+
// PomFile is the path to the POM file (absolute when returned by searchForProperties,
31+
// relative to the analysis root when returned by resolveUnknownProperties).
32+
PomFile string
33+
// Properties maps property names to their current values for this file.
34+
Properties map[string]string
35+
}
36+
2837
// Analyze performs dependency analysis on a Maven project.
2938
func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*analyzer.AnalysisResult, error) {
3039
log := clog.FromContext(ctx)
@@ -47,16 +56,23 @@ func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*anal
4756
return nil, fmt.Errorf("failed to parse POM file: %w", err)
4857
}
4958

59+
baseDir := filepath.Dir(absPath)
60+
5061
result := &analyzer.AnalysisResult{
51-
Language: mavenLanguageName,
52-
Dependencies: make(map[string]*analyzer.DependencyInfo),
53-
Properties: make(map[string]string),
54-
PropertyUsage: make(map[string]int),
55-
Metadata: make(map[string]any),
62+
Language: mavenLanguageName,
63+
Dependencies: make(map[string]*analyzer.DependencyInfo),
64+
Properties: make(map[string]string),
65+
PropertySources: make(map[string]string),
66+
PropertyUsage: make(map[string]int),
67+
Metadata: make(map[string]any),
5668
}
5769

58-
// Extract properties
70+
// Extract properties from the target POM and record their source file.
5971
result.Properties = extractPropertiesFromProject(project)
72+
propertySource := filepath.Base(absPath)
73+
for k := range result.Properties {
74+
result.PropertySources[k] = propertySource
75+
}
6076

6177
// Analyze regular dependencies
6278
if project.Dependencies != nil {
@@ -75,15 +91,28 @@ func (ma *MavenAnalyzer) Analyze(ctx context.Context, projectPath string) (*anal
7591
log.Infof("Analysis complete: found %d dependencies, %d using properties",
7692
len(result.Dependencies), countPropertiesUsage(result))
7793

78-
// Search for additional properties in project tree
79-
additionalProps := searchForProperties(ctx, filepath.Dir(absPath), absPath)
80-
log.Debugf("Property search found %d additional properties", len(additionalProps))
94+
// Search for additional properties in project tree and record their source files.
95+
// baseDir bounds the upward walk so we do not read outside the analyzed project.
96+
additionalPomFiles := searchForProperties(ctx, filepath.Dir(absPath), absPath, baseDir)
97+
log.Debugf("Property search found properties in %d nearby POMs", len(additionalPomFiles))
8198

82-
// Merge additional properties
83-
for k, v := range additionalProps {
84-
if _, exists := result.Properties[k]; !exists {
85-
result.Properties[k] = v
86-
log.Infof("Found property %s = %s in nearby POM", k, v)
99+
// Merge additional properties; first definition wins.
100+
for _, pf := range additionalPomFiles {
101+
for k, v := range pf.Properties {
102+
if mergeProperty(ctx, result, k, v, relPath(baseDir, pf.PomFile)) {
103+
log.Infof("Found property %s = %s in nearby POM", k, v)
104+
}
105+
}
106+
}
107+
108+
// For any property referenced by dependencies but still not found, follow the
109+
// <parent><relativePath> chain using the same resolver the updater uses — so
110+
// the analyzer and updater always agree on where a property lives.
111+
for _, pf := range resolveUnknownProperties(ctx, result.PropertyUsage, result.Properties, absPath, baseDir) {
112+
for propName, value := range pf.Properties {
113+
result.Properties[propName] = value
114+
result.PropertySources[propName] = pf.PomFile
115+
log.Infof("Found property %s = %s in parent POM %s", propName, value, pf.PomFile)
87116
}
88117
}
89118

@@ -248,39 +277,100 @@ func getAffectedDependencies(analysis *analyzer.AnalysisResult, propertyName str
248277
}
249278

250279
// searchForProperties recursively searches for properties in the Maven project tree.
251-
func searchForProperties(ctx context.Context, startDir string, excludePath string) map[string]string {
280+
// Returns one entry per POM file that declares at least one property, each carrying
281+
// the file's absolute path and its full property map. rootDir bounds the upward walk.
282+
func searchForProperties(ctx context.Context, startDir, excludePath, rootDir string) []pomFileProperties {
252283
log := clog.FromContext(ctx)
253-
properties := make(map[string]string)
284+
var results []pomFileProperties
254285

255-
projectRoot := findProjectRoot(startDir)
286+
projectRoot := findProjectRoot(startDir, rootDir)
256287
log.Debugf("Starting property search from project root: %s", projectRoot)
257288

258289
pomFilesChecked := 0
259290
for _, path := range walkXMLFiles(projectRoot) {
260-
if absPath, _ := filepath.Abs(path); absPath == excludePath {
291+
absPath, _ := filepath.Abs(path)
292+
if absPath == excludePath {
261293
continue
262294
}
263295
project, err := gopom.Parse(path)
264296
if err != nil {
265297
continue
266298
}
267-
pomFilesChecked++
268-
for k, v := range extractPropertiesFromProject(project) {
269-
if _, exists := properties[k]; !exists {
270-
properties[k] = v
271-
}
299+
props := extractPropertiesFromProject(project)
300+
if len(props) == 0 {
301+
continue
272302
}
303+
pomFilesChecked++
304+
results = append(results, pomFileProperties{PomFile: absPath, Properties: props})
273305
}
274306

275307
if log.Enabled(context.Background(), slog.LevelDebug) {
276-
log.Debugf("Property search checked %d POMs, found %d properties", pomFilesChecked, len(properties))
308+
log.Debugf("Property search checked %d POMs, found properties in %d", pomFilesChecked, len(results))
277309
}
278310

279-
return properties
311+
return results
312+
}
313+
314+
// relPath returns filePath relative to baseDir, falling back to the absolute
315+
// path if the relative form cannot be computed.
316+
func relPath(baseDir, filePath string) string {
317+
rel, err := filepath.Rel(baseDir, filePath)
318+
if err != nil {
319+
return filePath
320+
}
321+
return rel
280322
}
281323

282-
// findProjectRoot finds the root of the Maven project.
283-
func findProjectRoot(startDir string) string {
324+
// mergeProperty adds key→value/sourceFile to result if key is not already present.
325+
// When the key already exists with a different value, a warning is logged and the
326+
// existing definition is kept. Returns true when the property was newly added.
327+
func mergeProperty(ctx context.Context, result *analyzer.AnalysisResult, key, value, sourceFile string) bool {
328+
if existing, exists := result.Properties[key]; exists {
329+
if existing != value {
330+
clog.WarnContextf(ctx, "Property %s is defined with different values in %s (%s) and %s (%s); keeping first definition",
331+
key, result.PropertySources[key], existing, sourceFile, value)
332+
}
333+
return false
334+
}
335+
result.Properties[key] = value
336+
result.PropertySources[key] = sourceFile
337+
return true
338+
}
339+
340+
// resolveUnknownProperties looks up every property in usage that is not already
341+
// in known, following the <parent><relativePath> chain from startPom. Returns
342+
// one pomFileProperties per resolved property (PomFile relative to baseDir).
343+
// Individual lookup failures are logged at debug level and skipped.
344+
func resolveUnknownProperties(ctx context.Context, usage map[string]int, known map[string]string, startPom, baseDir string) []pomFileProperties {
345+
var results []pomFileProperties
346+
for propName := range usage {
347+
if _, found := known[propName]; found {
348+
continue
349+
}
350+
ownerPath, err := resolvePropertyPomPath(ctx, startPom, propName, baseDir)
351+
if err != nil {
352+
clog.FromContext(ctx).Debugf("Property %s not found in parent chain: %v", propName, err)
353+
continue
354+
}
355+
ownerProject, err := ParsePom(ownerPath)
356+
if err != nil {
357+
clog.FromContext(ctx).Debugf("Could not parse parent POM %s for property %s: %v", ownerPath, propName, err)
358+
continue
359+
}
360+
// resolvePropertyPomPath guarantees the property exists in ownerPath via
361+
// projectHasProperty, so Properties is non-nil and the key is present.
362+
results = append(results, pomFileProperties{
363+
PomFile: relPath(baseDir, ownerPath),
364+
Properties: map[string]string{propName: ownerProject.Properties.Entries[propName]},
365+
})
366+
}
367+
return results
368+
}
369+
370+
// findProjectRoot finds the root of the Maven project by walking up the directory
371+
// tree as long as each parent contains a pom.xml. The walk stops when it would
372+
// escape rootDir, preventing reads outside the project boundary.
373+
func findProjectRoot(startDir, rootDir string) string {
284374
current := startDir
285375
projectRoot := startDir
286376

@@ -290,6 +380,11 @@ func findProjectRoot(startDir string) string {
290380
break
291381
}
292382

383+
// Stop climbing if the parent escapes the project root boundary.
384+
if err := validatePathWithinRoot(rootDir, parent); err != nil {
385+
break
386+
}
387+
293388
parentPom := filepath.Join(parent, "pom.xml")
294389
if _, err := os.Stat(parentPom); err == nil {
295390
projectRoot = parent
@@ -375,11 +470,12 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a
375470
}
376471

377472
result := &analyzer.AnalysisResult{
378-
Language: mavenLanguageName,
379-
Dependencies: make(map[string]*analyzer.DependencyInfo),
380-
Properties: make(map[string]string),
381-
PropertyUsage: make(map[string]int),
382-
Metadata: make(map[string]any),
473+
Language: mavenLanguageName,
474+
Dependencies: make(map[string]*analyzer.DependencyInfo),
475+
Properties: make(map[string]string),
476+
PropertySources: make(map[string]string),
477+
PropertyUsage: make(map[string]int),
478+
Metadata: make(map[string]any),
383479
}
384480

385481
for _, pomPath := range pomPaths {
@@ -390,9 +486,7 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a
390486
continue
391487
}
392488
for k, v := range extractPropertiesFromProject(project) {
393-
if _, exists := result.Properties[k]; !exists {
394-
result.Properties[k] = v
395-
}
489+
mergeProperty(ctx, result, k, v, relPath(rootDir, pomPath))
396490
}
397491
if project.Dependencies != nil {
398492
for _, dep := range *project.Dependencies {
@@ -409,6 +503,19 @@ func (ma *MavenAnalyzer) analyzeAllPoms(ctx context.Context, rootDir string) (*a
409503
log.Infof("Analysis complete: found %d Maven POMs, %d dependencies, %d using properties",
410504
len(pomPaths), len(result.Dependencies), countPropertiesUsage(result))
411505

506+
// For any property referenced by dependencies but not found in the scanned
507+
// POMs, follow the <parent><relativePath> chain from the root pom.xml using
508+
// the same resolver the updater uses. This surfaces properties in parent POMs
509+
// outside the project tree (e.g. ../pom.xml).
510+
rootPom := filepath.Join(rootDir, DefaultManifestFile)
511+
for _, pf := range resolveUnknownProperties(ctx, result.PropertyUsage, result.Properties, rootPom, rootDir) {
512+
for propName, value := range pf.Properties {
513+
result.Properties[propName] = value
514+
result.PropertySources[propName] = pf.PomFile
515+
log.Infof("Found property %s = %s in parent POM %s", propName, value, pf.PomFile)
516+
}
517+
}
518+
412519
return result, nil
413520
}
414521

0 commit comments

Comments
 (0)