reject path-traversal segments in coordinate ids and versions by jmestwa-coder · Pull Request #12410 · apache/maven

jmestwa-coder · 2026-07-01T09:40:08Z

Coordinate ids and versions equal to . or .. pass model validation but land as raw directory segments when an artifact is mapped to its local repository path (groupId/artifactId/version/...):

isValidCoordinatesId/isValidId accept them because every character is individually allowed, so an artifactId of .. is a path-traversal segment
validateVersion only bans the filesystem metacharacters, so a dependency version of .. slips through the same way
same gap in the compat maven-model-builder validator and in the wildcard-id variant

Reject . and .. at each check. Regression test added in both modules; it fails on the unpatched validators and passes after.

Your pull request should address just one issue, without pulling in other changes.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Each commit in the pull request should have a meaningful subject line and body.
Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
Run mvn verify to make sure basic checks pass.
You have run the Core IT successfully.
I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

gnodet

Review: Reject path-traversal segments in coordinate ids and versions

This is a genuine and valuable security fix — thank you for working on it! I have a few suggestions to strengthen it before it can be merged.

Findings

1. [High] Incomplete coverage of groupId path-traversal

The current check only rejects exact matches of "." and ".." as the entire coordinate string. However, for groupId, Maven maps dots to path separators — so a groupId like org..evil becomes org/../evil/ in the local repository layout, which is a path traversal that bypasses this check.

Consider either:

Extending the fix to check each dot-delimited segment of groupIds for "." and "..", or
Filing a follow-up issue explicitly documenting this remaining gap.

2. [Medium] No Jira issue referenced

The project's commit convention requires [MNG-XXXX] <description> for fixes. Please create a Jira issue and reference it in the commit message — especially for security-related changes.

3. [Medium] Test coverage gaps

Tests cover ".." but not "." — both are rejected by the code and should be tested.
Tests don't cover groupId traversal (only artifactId and version).
Consider adding a test for "." as a version as well.

4. [Low] CI has not run

No checks have been reported on the branch. Please run mvn verify (and ideally mvn -Prun-its verify for Core ITs) before resubmitting.

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

Reviewed with Claude Code on behalf of @gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

jmestwa-coder · 2026-07-04T12:39:23Z

thanks for the review, went through each point:

groupId traversal: the local repo layout maps groupId dots to path separators (groupId.replace('.', '/')), so org..evil becomes org//evil, an empty segment rather than a .. component. since . is the delimiter, a groupId can't produce a literal .. path segment through that mapping, so there's no separate per-segment gap to close. the raw-segment vectors are artifactId and version (used verbatim as path components), and both are already rejected. i did add a groupId .. case to the tests to lock that behavior in.
tests: added coverage for . as an artifactId and as a dependency version, plus groupId .., alongside the existing .. cases. impl is now 83 tests, compat 72, both green.
ci: ran mvn test on both modules and spotless:check locally, all pass. full mvn verify / core ITs will run on the branch.
issue ref: recent commits here reference GitHub issues ([#nnnn]) rather than MNG. happy to open a tracking issue and reference it in the commit, or file under MNG instead if you prefer that for security fixes. let me know which and i'll amend.

elharo

This needs an issue that clearly describes and provides an example fo the problem

…ersions Signed-off-by: Syed Mohammed Nayyar <jmestwa@gmail.com>

…ersal segments covers '.' as artifactId and dependency version, plus groupId '..', in both the maven-impl and compat maven-model-builder validators.

jmestwa-coder · 2026-07-07T10:21:07Z

filed #12427 with the description and a worked example (artifactId .. maps to org/example/../1.0/..-1.0.jar in the default local repo layout). commit messages now reference it per the [#nnnn] convention; branch content is unchanged from the last review.

elharo · 2026-07-07T11:01:42Z

    }

+    /**
+     * An id or version made up only of {@code .} or {@code ..} is a filesystem path-traversal segment. The dotted


I don't think this is true. The problem is mapping straight into a path traversal segment, not that these are the names. If you can find evidence that these are not legal names, please post it.

fair point, I don't have evidence that . or .. are illegal names as such. the actual problem is the default local repository layout using artifactId/version verbatim as directory names, which turns these values into literal ./.. path segments. reworded the javadoc in both validators to say the rejection is due to that mapping, not the names themselves.

On Unix and probably other systems . and .. are legal filenames. Dealing with this is a classic Unix sysadmin interview question.

agreed. the reworded javadoc makes no claim the names are illegal, only that the default layout uses them verbatim as directory names so they resolve as the current/parent directory. the rejection is pinned entirely on that mapping.

The names are illegal, see https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html, section 4.13:

The special filename dot shall refer to the directory specified by its predecessor. The special filename dot-dot shall refer to the parent directory of its predecessor directory. As a special case, in the root directory, dot-dot may refer to the root directory itself.

You can't create a file named . or ...

Or did you meant reserved instead of illegal maybe ?

The section you cite refers to pathname resolution, not to pathnames themselves. In practice on most (all?) Unix and Unix like systems it is possible to create a file named "." or ".." as unwise as that might be.

Can you give me a reproducible way to create a file or directory named . or .. ?

echo > . zsh: is a directory: .

In python, open() returns 21 (EISDIR) on both . and ...

…not the names

gnodet requested changes Jul 3, 2026

View reviewed changes

elharo reviewed Jul 5, 2026

View reviewed changes

jmestwa-coder mentioned this pull request Jul 7, 2026

model validator accepts '.' and '..' as artifactId/version, creating path-traversal segments in local repository paths #12427

Open

jmestwa-coder added 2 commits July 7, 2026 15:50

[apache#12427] reject path-traversal segments in coordinate ids and v…

c56ab89

…ersions Signed-off-by: Syed Mohammed Nayyar <jmestwa@gmail.com>

[apache#12427] add test coverage for single-dot and groupId path-trav…

c8ac7a0

…ersal segments covers '.' as artifactId and dependency version, plus groupId '..', in both the maven-impl and compat maven-model-builder validators.

jmestwa-coder force-pushed the coordinate-path-traversal branch from 9955586 to c8ac7a0 Compare July 7, 2026 10:20

elharo requested changes Jul 7, 2026

View reviewed changes

[apache#12427] reword javadoc: rejection is due to the path mapping, …

1c0b584

…not the names

Uh oh!

Conversation

jmestwa-coder commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Review: Reject path-traversal segments in coordinate ids and versions

Findings

Uh oh!

jmestwa-coder commented Jul 4, 2026

Uh oh!

elharo left a comment

Choose a reason for hiding this comment

Uh oh!

jmestwa-coder commented Jul 7, 2026

Uh oh!

elharo Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jmestwa-coder commented Jul 1, 2026 •

edited

Loading

elharo Jul 7, 2026 •

edited

Loading