Skip to content

Scope server credentials and validate legacy relocation coordinates - #12976

Merged
gnodet merged 3 commits into
masterfrom
pr/legacy-compat-credential-scope-master
Sep 2, 2026
Merged

Scope server credentials and validate legacy relocation coordinates#12976
gnodet merged 3 commits into
masterfrom
pr/legacy-compat-credential-scope-master

Conversation

@gnodet

@gnodet gnodet commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Forward-port of #12954 (maven-3.10.x) to master, adapted for the 4.x module layout (compat/ and impl/ prefixes) and API changes (SLF4J logging, constructor injection).

Three fixes:

  1. OriginBoundAuthenticationSelector — scopes server credentials to the origins (protocol+host+port) declared for the same server id in settings mirrors/repositories, preventing credential leakage to repositories that merely share a server id. Controlled by the new maven.repository.credentialScope user property (origin | strict | id).

  2. Relocation coordinate validation in MavenMetadataSource — rejects relocation groupId / artifactId / version values that contain path-traversal characters (/, \, ..) or control characters before they are applied to the artifact being resolved.

  3. Exact server ID matching in DefaultWagonManager — replaces equalsIgnoreCase with equals for consistency with LegacyRepositorySystem.injectAuthentication and the resolver's authentication selector.

Adapted paths

3.10.x path master path
maven-compat/src/…/DefaultWagonManager.java compat/maven-compat/src/…/DefaultWagonManager.java
maven-core/src/…/DefaultRepositorySystemSessionFactory.java impl/maven-core/src/…/DefaultRepositorySystemSessionFactory.java
maven-core/src/…/OriginBoundAuthenticationSelector.java impl/maven-core/src/…/OriginBoundAuthenticationSelector.java
maven-core/src/…/MavenMetadataSource.java compat/maven-compat/src/…/MavenMetadataSource.java

API adaptations for master

  • Uses org.slf4j.Logger instead of org.codehaus.plexus.logging.Logger
  • Uses mergedProps.getOrDefault() instead of ConfigUtils.getString() (not available on master)
  • Test uses constructor injection instead of reflection-based field injection (matching master's MavenMetadataSource constructor)
  • @since 4.0.0 on the new constant (instead of 3.10.0)

Test plan

  • OriginBoundAuthenticationSelectorTest — 7 tests covering origin/strict/id scopes, origin normalization, and unknown scope rejection
  • MavenMetadataSourceRelocationTest — 2 tests verifying invalid relocations are rejected and well-formed relocations are applied
  • mvn verify -pl impl/maven-core — 642 tests pass
  • mvn verify -pl compat/maven-compat — 162 tests pass

🤖 Generated with Claude Code

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Clean and correct forward-port of the approved #12954 security-hardening fixes to master. All five adaptations for the 4.x module layout are properly done.

Adaptations verified:

  • Module paths correctly use compat/ and impl/ prefixes
  • OriginBoundAuthenticationSelector uses SLF4J logging instead of Plexus logging
  • Relocation test uses constructor injection (matching @Inject on master) instead of 3.10.x reflection-based field setting
  • Credential scope property read via mergedProps.getOrDefault() (consistent with other user properties on master)
  • Session builder variable naming follows master conventions
  • All three fixes present: credential scoping, relocation validation, server ID matching

Observation (non-blocking):

⚠️ [medium] Pre-existing gap: relocation validation missing on new resolver code paths — The requireValidCoordinateComponent / isInvalidCoordinateComponent validation is only applied to legacy MavenMetadataSource in compat/maven-compat. The new resolver-based DistributionManagementArtifactRelocationSource (in both impl/maven-impl and compat/maven-resolver-provider) passes relocation coordinates to RelocatedArtifact without equivalent path-traversal/control-character validation. This is not introduced by this PR, but since the impl path is the primary relocation code path in 4.x non-compat mode, consider hardening it in a follow-up (ideally in RelocatedArtifact's constructor so all relocation sources benefit).

📋 PR Metadata

Aspect Current Suggested
Labels (none) bug
Milestone (none) 4.1.0

🔀 Backport Status

✅ All branches covered:

  • maven-3.10.x#12954 (OPEN)
  • maven-4.0.x#12977 (OPEN)

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

Claude Code on behalf of Guillaume Nodet

@gnodet gnodet added this to the 4.1.0 milestone Sep 1, 2026
Forward-port of #12954 (maven-3.10.x) to master, adapted for the 4.x
module layout (compat/ and impl/ prefixes) and API changes (SLF4J
logging, constructor injection, api.Constants).

Three fixes:

1. OriginBoundAuthenticationSelector — scopes server credentials to
   the origins (protocol+host+port) declared for the same server id
   in settings mirrors/repositories, preventing credential leakage to
   repositories that merely share a server id. Controlled by the new
   maven.repository.credentialScope user property (origin | strict | id).

2. Relocation coordinate validation in MavenMetadataSource — rejects
   relocation groupId / artifactId / version values that contain
   path-traversal characters (/, \, ..) or control characters before
   they are applied to the artifact being resolved.

3. Exact server ID matching in DefaultWagonManager — replaces
   equalsIgnoreCase with equals for consistency with
   LegacyRepositorySystem.injectAuthentication and the resolver's
   authentication selector.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet force-pushed the pr/legacy-compat-credential-scope-master branch from 7fbde51 to 095b80d Compare September 1, 2026 21:10
Remove unused import and extra blank lines left by rebase.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The relocatedTarget method now declares ArtifactDescriptorException
(added by the relocation validation merge). The test must declare it.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet
gnodet merged commit 616bc0d into master Sep 2, 2026
23 checks passed
@gnodet
gnodet deleted the pr/legacy-compat-credential-scope-master branch September 2, 2026 06:16
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

@gnodet Please assign appropriate label to PR according to the type of change.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant