Skip to content

Scope server credentials and validate legacy relocation coordinates - #12954

Open
slachiewicz wants to merge 3 commits into
apache:maven-3.10.xfrom
slachiewicz:pr/core-and-compat-3.10.x
Open

Scope server credentials and validate legacy relocation coordinates#12954
slachiewicz wants to merge 3 commits into
apache:maven-3.10.xfrom
slachiewicz:pr/core-and-compat-3.10.x

Conversation

@slachiewicz

@slachiewicz slachiewicz commented Aug 30, 2026

Copy link
Copy Markdown
Member

Three fixes in maven-core and maven-compat.

  • Relocation coordinates on the legacy metadata path. Relocation coordinates read from resolved project metadata are validated before use, matching the check already applied in the resolver provider.
  • Credential scope. Server credentials are scoped to the origin (protocol, host and port) of a repository or mirror the operator declared for that same server id. Ids with no declared origin keep the previous behaviour and emit a warning naming the target origin; maven.repository.credentialScope=strict refuses them instead. Defaults to origin.
  • Server id matching. DefaultWagonManager matched settings server ids case-insensitively while the rest of the code matches exactly; it now matches exactly.

Each change is a separate commit.


Draft while related code paths are reviewed — the same guard may be needed in sibling implementations of these interfaces, and I would rather establish that before asking for review time.

slachiewicz and others added 3 commits August 30, 2026 19:56
Credentials configured in settings for a server id are keyed only by
that id. A repository definition can also arrive from a POM resolved
out of a remote repository and reuse the id of a server the operator
holds credentials for, at a different origin than the one the operator
configured. Wrap the authentication selector so credentials for an id
are offered only to a repository whose origin (scheme, host, port)
matches a repository or mirror the operator declared, in settings or
on the command line, with that same id; other origins get a
once-per-id/origin warning naming the id and are refused.

Ids with no operator-declared repository (for example a deploy-only
server whose URL comes from the project's distributionManagement)
keep serving credentials as before, with a warning, so mvn deploy
using settings credentials plus a POM's distributionManagement keeps
working. The new maven.repository.credentialScope user property
selects the policy: "origin" (default), "strict" (also refuse
undeclared ids), or legacy "id".
Aligns the legacy WagonManager credential lookup with the exact-match semantics
used by every other id-keyed credential path; server ids have never been
documented as case-insensitive.
@slachiewicz slachiewicz added the bug Something isn't working label Aug 30, 2026
@slachiewicz slachiewicz added this to the 3.10.0 milestone Aug 30, 2026
@slachiewicz
slachiewicz marked this pull request as draft August 30, 2026 19:05

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three well-implemented security-hardening fixes. The credential-scoping wrapper is correctly designed with good defaults (origin-based by default, strict opt-in, id fallback), proper thread safety (ConcurrentHashMap.newKeySet() for reported, read-only declaredOrigins), and clean escape hatches. All CI checks pass.

Observations (informational):

  1. Credential scoping design — The three-tier policy (origin/strict/id) with origin as default is well balanced. The undeclared-ID path emitting a warning rather than refusing is the right call for the distributionManagement deployment case.

  2. DAV URLsdav:http://host/path URLs produce a null origin because java.net.URI parses them as opaque URIs with no host. In origin mode this works (credentials served with a warning), but in strict mode credentials would be refused. DAV URLs are extremely rare in practice, but worth documenting in originOf's Javadoc.

  3. Relocation validation — The denylist approach (.., /, \, :, control chars) is adequate for blocking path traversal on the legacy metadata path. Note: the PR description states it "matches the check already applied in the resolver provider" but the resolver provider doesn't appear to have this check — the model validator uses a stricter whitelist ([a-zA-Z0-9._-]). The denylist is fine for this purpose.

  4. Style nitassertEquals(true, exception.getMessage().contains("a/b")) could be assertTrue(...) for readability.

🔀 Backport / Forward-port Status

⚠️ The same issues exist on other branches:

  • maven-3.9.x — has the equalsIgnoreCase server-ID bug and the relocation validation gap
  • master — has the equalsIgnoreCase in compat/maven-compat/.../DefaultWagonManager.java

The three commits are cleanly separated, making selective cherry-picking straightforward.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

@slachiewicz

Copy link
Copy Markdown
Member Author

Related work in maven-resolver, for reviewer awareness: apache/maven-resolver#2090 adds a RemoteRepositoryManager.aggregateRepositories overload that carries the provenance of the recessive repository definitions, so an implementation can apply session authentication only to repositories the operator configured rather than to repositories declared by a downloaded artifact descriptor.

It approaches the same question as this PR from the resolver side. The default implementation there delegates to the existing method, so nothing changes for callers that do not opt in.

This comment was created with AI assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants