Validate path components in DefaultLocalPathComposer

[gnodet]

Summary

• Add validation in DefaultLocalPathComposer to reject coordinate components (groupId, artifactId, version, classifier, extension) that contain path traversal sequences (..) or path separator characters (/, \) before composing the local repository path.
• This is consistent with the existing path validation in FileTransporter.getPath() (line 166).
• Add DefaultLocalPathComposerTest with tests for normal and malformed coordinates.

Test plan

• New DefaultLocalPathComposerTest passes (7 tests — normal paths, traversal rejection, slash rejection, backslash rejection)
• Existing SimpleLocalRepositoryManagerTest passes (3/3)
• Existing EnhancedLocalRepositoryManagerTest passes (12/12)

🤖 Generated with Claude Code

[gnodet]

AI-Assisted Review — PR #1958 (Validate path components in DefaultLocalPathComposer)

This is a well-scoped security hardening PR — nice work! The validation is correct, purely additive, and more thorough than the existing precedent in FileTransporter.getPath().

What works well

• Correct validation logic: Checking for .., /, and \ independently catches traversal sequences, forward-slash injection, and backslash-based traversal. This is more robust than the existing FileTransporter.getPath() check which only catches "../".
• Null/empty safety: validatePathComponent correctly short-circuits on null and empty values, avoiding false rejections on legitimate empty classifiers.
• Good test coverage: The 7 new tests cover the core attack vectors (traversal sequences, slashes, backslashes) and verify that normal version strings with dots and dashes remain unaffected.

Minor observations (non-blocking)

1. metadata.getType() and repositoryKey are not validated

Both flow into the composed path but aren't passed through validatePathComponent. However, these are internally derived rather than attacker-controlled: repositoryKey comes from RepositoryIdHelper (which uses PathUtils.stringToPathSegment() for sanitization) or is the constant "local", and metadata.getType() is typically "maven-metadata.xml" or internal constants. This is a defense-in-depth gap rather than an exploitable issue — worth noting for a future hardening pass.

2. Test coverage for non-version fields

The traversal rejection tests only exercise the version field. Since all fields use the same validatePathComponent method the logic is covered, but adding at least one test per field (groupId, artifactId, classifier, extension) would strengthen completeness.

Overall this is ready to merge — a clean security improvement at the right layer.

🤖 This review was generated with AI assistance (reviewer + independent verifier pattern). Findings were independently verified before posting.

[cstamas]

I propose alt solution #1959
And once 2.0.21 is out, Maven implementations of DefaultVersionResolver and DefaultVersionRangeResolver should reuse it as well.