Skip to content

Handle unresolved Maven credential placeholders gracefully#6845

Merged
sambsnyd merged 12 commits intomainfrom
timtebeek/maven-credential-placeholders
Mar 3, 2026
Merged

Handle unresolved Maven credential placeholders gracefully#6845
sambsnyd merged 12 commits intomainfrom
timtebeek/maven-credential-placeholders

Conversation

@timtebeek
Copy link
Copy Markdown
Member

@timtebeek timtebeek commented Feb 27, 2026

What's changed

When a Maven settings.xml uses environment variable placeholders for server credentials (e.g. ${env.MAVEN_USER}) and those variables are not set, the PropertyPlaceholderHelper preserves the literal "${env.MAVEN_USER}" string. This was then sent as actual Basic Auth credentials, causing 403 errors when downloading artifact JARs.

POM downloads were unaffected because MavenPomDownloader has requestAsAuthenticatedOrAnonymous() fallback logic (added for #1859), but MavenArtifactDownloader lacked this fallback.

In MavenArtifactDownloader, when credentials are applied and the server responds with a 4xx client error, retry the download anonymously — matching the existing MavenPomDownloader behavior.

  • Fixes moderneinc/customer-requests#1928

When Maven settings.xml uses ${env.MAVEN_USER} placeholders for server
credentials and those env vars are not set, the literal placeholder
string is sent as the username, causing 403 errors on artifact downloads.

These tests verify:
- Unresolved credential placeholders are nulled out by the Interpolator
- MavenArtifactDownloader falls back to anonymous on 403 with credentials
When Maven settings.xml uses ${env.MAVEN_USER} / ${env.MAVEN_PASSWORD}
placeholders and those env vars are not set, the PropertyPlaceholderHelper
preserves the literal "${env.FOO}" string. This was then sent as actual
credentials, causing 403 errors.

Now the Interpolator checks for remaining "${" in server credentials
after interpolation and nulls them out, so no auth header is sent and
the download proceeds anonymously — matching Apache Maven behavior.

Also adds @nullable to Server.username and Server.password since
credentials can genuinely be absent.

Fixes moderneinc/customer-requests#1928
When credentials are applied and the server responds with a 4xx client
error, retry the download without authentication. This mirrors the
existing behavior in MavenPomDownloader.requestAsAuthenticatedOrAnonymous()
and matches how Apache Maven handles credential failures gracefully.

This is defense-in-depth alongside the Interpolator fix — it also
protects against other credential mismatch scenarios.
@timtebeek timtebeek requested a review from nmck257 February 27, 2026 13:20
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Feb 27, 2026
@timtebeek
Copy link
Copy Markdown
Member Author

Hi Nick, thanks for the helpful link to the earlier work in #1859 ; guess we had missed to do the same for artifacts at the time, which is a separate downloader. Now made consistent here to hopefully resolve your immediate issue.

if (!response.isSuccessful() || body == null) {
int code = response.getCode();
// If credentials caused a client-side error, retry anonymously
MavenRepository repository = dependency.getRepository();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Every artifact downloaded is going to cause a client-side error. That may trigger anomaly detection on that server.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

An alternative I'd considered was to not make an authenticated request when the username or password starts with ${, but that rules out some very special passwords.

The pattern to repeat with an unauthenticated request is taken from MavenPomDownloader, where we had added that four years ago

I'll restore skipping authenticated requests when there's still a likely placeholder left over, unless you'd wanted to nudge in a different direction still.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I suppose we could also make this a bit stateful -- toggle between trying authenticated or anonymous requests first, based on success of previous attempts.
if we land on a really good pattern, maybe also worth refactoring so the Pom and Artifact downloaders both share the same impl

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adjusted to logic to now only make a single unauthenticated request when the username/password still contain a placeholder. I've not yet replicated the same to the pom downloader, but could if desired.

@timtebeek timtebeek marked this pull request as draft February 27, 2026 16:06
@timtebeek timtebeek marked this pull request as ready for review March 2, 2026 17:04
@timtebeek timtebeek requested review from jkschneider and nmck257 March 2, 2026 17:04
@sambsnyd sambsnyd merged commit d0eda87 into main Mar 3, 2026
1 check passed
@sambsnyd sambsnyd deleted the timtebeek/maven-credential-placeholders branch March 3, 2026 22:44
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Mar 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants