Backport path composition and repository provenance changes to 1.9.x - #2092
Conversation
Port the 2.0.x coordinate validation to this line: PathUtils gains validatePathComponent (rejecting '..', separators and ':'), validateDotSeparatedPathComponent (rejecting empty dot-separated segments in dot-expanded components like the groupId) and the artifact/metadata component validators. DefaultLocalPathComposer applies them and adds a last-resort check that the composed path is relative with no parent-reference or empty segments; Maven2RepositoryLayoutFactory validates before composing remote locations; FileTransporter replaces its substring check with a fail-closed containment check under the repository base directory. stringToPathSegment maps '.' and '..' to explicit tokens, matching the existing single-character replacements. Unusual but documented-valid version strings like '1..' remain accepted, as versions are not dot-expanded.
The filesystem-friendly GAV/GAECV name mappers now pass every coordinate field through PathUtils.stringToPathSegment, and BasedirNameMapper rejects lock names that do not resolve under the locks base directory, so the resolved lock path is always contained in it. Port of the 2.0.x change to this line.
Port of the 2.0.x change: add an aggregateRepositories overload that distinguishes repositories declared by remote artifact descriptors from repositories supplied by the build. By default, session authentication matched by repository ID is no longer applied to descriptor-declared repositories unless an operator-defined mirror was selected for them; a warning names the repository and the aether.remoteRepositoryManager.authToDescriptorRepositories flag restores the previous behavior. Dependency collectors mark descriptor repositories accordingly; repositories supplied by the build itself are unaffected.
slachiewicz
changed the title
slachiewicz
changed the title
slachiewicz
added
maintenance
java
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Well-structured security backport porting three important fixes (coordinate validation, lock name sanitization, auth scoping) to the 1.9.x line. The adaptation from 2.x to 1.9.x APIs is well done throughout.
Issues to fix before merge:
- Two test files have duplicate ASF license headers (copy-paste error) — see inline comments.
Forward-port observations:
This backport is actually more comprehensive than what exists on the 2.x master, creating a maintenance asymmetry:
validatePathComponentadds colon (:) rejection not present on 2.x mastervalidateDotSeparatedPathComponentis entirely new and not on 2.xFileTransportercontainment check is materially stronger than 2.x'spath.contains("../")- Auth scoping by repository provenance is absent from 2.x entirely
Consider forward-porting these enhancements to the 2.x line as well.
📋 PR Metadata
| Aspect | Current | Suggested |
|---|---|---|
| Labels | (none) | fix |
| Milestone | (none) | 1.9.28 |
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Re-review after new commits: the duplicate license headers in DefaultLocalPathComposerTest.java and PathUtilsTest.java have been removed (commit 0b6d595). All @since tags correctly reference 1.9.28. The backport is well-adapted to 1.9.x APIs.
Sequencing note (informational): The three master PRs this backport references (#2087, #2091, #2090) are still open. If this PR merges first, the 1.9.x line will ship these security hardening changes before the 2.x line. The PR description acknowledges this: "review the three master pull requests first; this one is mechanical once those are settled." Merging master PRs first would reduce asymmetry risk.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
3 participants
Ports three changes from master to the
maven-resolver-1.9.xmaintenance line so the two linesbehave consistently:
under the locks base directory (Compose named lock keys one segment per coordinate field #2091),
RemoteRepositoryManager, defaulting to the existingbehaviour (Carry repository provenance into aggregateRepositories #2090).
PathUtilsdoes not exist on this line, so it is added here along with its tests. The rest followsthe master changes, adapted to the 1.9.x API.
Please review the three master pull requests first; this one is mechanical once those are settled.