[MNG-8419][MNG-8424] Too aggressive warning for pre-Maven4 passwords (#1970)

Toned down the Maven4 sec dispatcher messages. Also, IF maven3 passwords detected AND there was `.mvn/extensions.xml` the warnings were doubled.

Examples:

Failure to start Maven (due non-decryptable passwords):
```
$ mvn clean
[ERROR] Error executing Maven.
[ERROR] Error building settings
 * FATAL: Could not decrypt password (fix the corrupted password or remove it, if unused) {xL6L/HbmrY++sNkphnq3fguYepTpM04WlIXb8nB1pk=}
 * WARNING: Detected 2 pre-Maven 4 legacy encrypted password(s) - configure password encryption with the help of mvnenc for increased security.
$
```

Warning at start (due Maven3 passwords):
```
$ mvn clean
[INFO]
[INFO] Some problems were encountered while building the effective settings (use -X to see details)
[INFO]
[INFO] Scanning for projects...
[INFO] --------------------------------------------------------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO]
[INFO] Apache Maven
...
```

---

https://issues.apache.org/jira/browse/MNG-8424
https://issues.apache.org/jira/browse/MNG-8419
This commit is contained in:
Tamas Cservenak 2024-12-12 17:43:08 +01:00 committed by GitHub
parent 79d7739dcc
commit 50aecc7432
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 118 additions and 43 deletions

View File

@ -0,0 +1,68 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.maven.api.services;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import org.apache.maven.api.annotations.Experimental;
/**
* Base class for all maven exceptions carrying {@link BuilderProblem}s.
*
* @since 4.0.0
*/
@Experimental
public abstract class MavenBuilderException extends MavenException {
private final List<BuilderProblem> problems;
public MavenBuilderException(String message, Throwable cause) {
super(message, cause);
problems = List.of();
}
public MavenBuilderException(String message, List<BuilderProblem> problems) {
super(buildMessage(message, problems), null);
this.problems = problems;
}
/**
* Formats message out of problems: problems are sorted (in natural order of {@link BuilderProblem.Severity})
* and then a list is built. These exceptions are usually thrown in "fatal" cases (and usually prevent Maven
* from starting), and these exceptions may end up very early on output.
*/
protected static String buildMessage(String message, List<BuilderProblem> problems) {
StringBuilder msg = new StringBuilder(message);
ArrayList<BuilderProblem> sorted = new ArrayList<>(problems);
sorted.sort(Comparator.comparing(BuilderProblem::getSeverity));
for (BuilderProblem problem : sorted) {
msg.append("\n * ")
.append(problem.getSeverity().name())
.append(": ")
.append(problem.getMessage());
}
return msg.toString();
}
public List<BuilderProblem> getProblems() {
return problems;
}
}

View File

@ -20,7 +20,6 @@ package org.apache.maven.api.services;
import java.io.Serial;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.maven.api.annotations.Experimental;
@ -30,28 +29,20 @@ import org.apache.maven.api.annotations.Experimental;
* @since 4.0.0
*/
@Experimental
public class SettingsBuilderException extends MavenException {
public class SettingsBuilderException extends MavenBuilderException {
@Serial
private static final long serialVersionUID = 4714858598345418083L;
private final List<BuilderProblem> problems;
/**
* @param message the message to give
* @param e the {@link Exception}
*/
public SettingsBuilderException(String message, Exception e) {
super(message, e);
this.problems = List.of();
}
public SettingsBuilderException(String message, List<BuilderProblem> problems) {
super(message + ": " + problems.stream().map(BuilderProblem::toString).collect(Collectors.joining(", ")), null);
this.problems = List.copyOf(problems);
}
public List<BuilderProblem> getProblems() {
return problems;
super(message, problems);
}
}

View File

@ -20,7 +20,6 @@ package org.apache.maven.api.services;
import java.io.Serial;
import java.util.List;
import java.util.stream.Collectors;
import org.apache.maven.api.annotations.Experimental;
@ -30,28 +29,20 @@ import org.apache.maven.api.annotations.Experimental;
* @since 4.0.0
*/
@Experimental
public class ToolchainsBuilderException extends MavenException {
public class ToolchainsBuilderException extends MavenBuilderException {
@Serial
private static final long serialVersionUID = 7899871809665729349L;
private final List<BuilderProblem> problems;
/**
* @param message the message to give
* @param e the {@link Exception}
*/
public ToolchainsBuilderException(String message, Exception e) {
super(message, e);
this.problems = List.of();
}
public ToolchainsBuilderException(String message, List<BuilderProblem> problems) {
super(message + ": " + problems.stream().map(BuilderProblem::toString).collect(Collectors.joining(", ")), null);
this.problems = List.copyOf(problems);
}
public List<BuilderProblem> getProblems() {
return problems;
super(message, problems);
}
}

View File

@ -465,10 +465,18 @@ public abstract class LookupInvoker<C extends LookupContext> implements Invoker
}
protected void settings(C context) throws Exception {
settings(context, context.lookup.lookup(SettingsBuilder.class));
settings(context, true, context.lookup.lookup(SettingsBuilder.class));
}
protected void settings(C context, SettingsBuilder settingsBuilder) throws Exception {
/**
* This method is invoked twice during "normal" LookupInvoker level startup: once when (if present) extensions
* are loaded up during Plexus DI creation, and once afterward as "normal" boot procedure.
* <p>
* If there are Maven3 passwords presents in settings, this results in doubled warnings emitted. So Plexus DI
* creation call keeps "emitSettingsWarnings" false. If there are fatal issues, it will anyway "die" at that
* spot before warnings would be emitted.
*/
protected void settings(C context, boolean emitSettingsWarnings, SettingsBuilder settingsBuilder) throws Exception {
Options mavenOptions = context.invokerRequest.options();
Path userSettingsFile = null;
@ -557,14 +565,17 @@ public abstract class LookupInvoker<C extends LookupContext> implements Invoker
context.interactive = mayDisableInteractiveMode(context, context.effectiveSettings.isInteractiveMode());
context.localRepositoryPath = localRepositoryPath(context);
if (!settingsResult.getProblems().isEmpty()) {
context.logger.warn("");
context.logger.warn("Some problems were encountered while building the effective settings");
if (emitSettingsWarnings && !settingsResult.getProblems().isEmpty()) {
context.logger.info("");
context.logger.info(
"Some problems were encountered while building the effective settings (use -X to see details)");
for (BuilderProblem problem : settingsResult.getProblems()) {
context.logger.warn(problem.getMessage() + " @ " + problem.getLocation());
if (context.invokerRequest.options().verbose().orElse(false)) {
for (BuilderProblem problem : settingsResult.getProblems()) {
context.logger.warn(problem.getMessage() + " @ " + problem.getLocation());
}
}
context.logger.warn("");
context.logger.info("");
}
}

View File

@ -75,7 +75,7 @@ public class PlexusContainerCapsuleFactory<C extends LookupContext> implements C
return new PlexusContainerCapsule(
context, Thread.currentThread().getContextClassLoader(), container(invoker, context));
} catch (Exception e) {
throw new InvokerException("Failed to create plexus container capsule", e);
throw new InvokerException("Failed to create Plexus DI Container", e);
}
}
@ -279,7 +279,7 @@ public class PlexusContainerCapsuleFactory<C extends LookupContext> implements C
container.getLoggerManager().setThresholds(toPlexusLoggingLevel(context.loggerLevel));
Thread.currentThread().setContextClassLoader(container.getContainerRealm());
invoker.settings(context, container.lookup(SettingsBuilder.class));
invoker.settings(context, false, container.lookup(SettingsBuilder.class));
MavenExecutionRequest mer = new DefaultMavenExecutionRequest();
invoker.populateRequest(context, new DefaultLookup(container), mer);

View File

@ -29,6 +29,7 @@ import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import java.util.function.Supplier;
@ -273,18 +274,12 @@ public class DefaultSettingsBuilder implements SettingsBuilder {
return settings;
}
SecDispatcher secDispatcher = new DefaultSecDispatcher(dispatchers, getSecuritySettings(request.getSession()));
final AtomicInteger preMaven4Passwords = new AtomicInteger(0);
Function<String, String> decryptFunction = str -> {
if (str != null && !str.isEmpty() && !str.contains("${") && secDispatcher.isAnyEncryptedString(str)) {
if (secDispatcher.isLegacyEncryptedString(str)) {
// add a problem
problems.add(new DefaultBuilderProblem(
settingsSource.getLocation(),
-1,
-1,
null,
"Pre-Maven 4 legacy encrypted password detected "
+ " - configure password encryption with the help of mvnenc to be compatible with Maven 4.",
BuilderProblem.Severity.WARNING));
preMaven4Passwords.incrementAndGet();
}
try {
return secDispatcher.decrypt(str);
@ -295,12 +290,23 @@ public class DefaultSettingsBuilder implements SettingsBuilder {
-1,
e,
"Could not decrypt password (fix the corrupted password or remove it, if unused) " + str,
BuilderProblem.Severity.ERROR));
BuilderProblem.Severity.FATAL));
}
}
return str;
};
return new SettingsTransformer(decryptFunction).visit(settings);
Settings result = new SettingsTransformer(decryptFunction).visit(settings);
if (preMaven4Passwords.get() > 0) {
problems.add(new DefaultBuilderProblem(
settingsSource.getLocation(),
-1,
-1,
null,
"Detected " + preMaven4Passwords.get() + " pre-Maven 4 legacy encrypted password(s) "
+ "- configure password encryption with the help of mvnenc for increased security.",
BuilderProblem.Severity.WARNING));
}
return result;
}
private Path getSecuritySettings(ProtoSession session) {

View File

@ -21,6 +21,7 @@ package org.apache.maven.it;
import java.io.File;
import java.util.List;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -33,7 +34,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
*
* @author jdcasey
*
* NOTE (cstamas): this IT was written to test that settings.xml is STRICT, while later changes modified
* this very IT into the opposite: to test that parsing is LENIENT.
*/
@Disabled("This is archaic test; we should strive to make settings.xml parsing strict again")
public class MavenITmng3748BadSettingsXmlTest extends AbstractMavenIntegrationTestCase {
public MavenITmng3748BadSettingsXmlTest() {

View File

@ -47,7 +47,8 @@ class MavenITmng8379SettingsDecryptTest extends AbstractMavenIntegrationTestCase
verifier.verifyErrorFreeLog();
// there is a warning and all fields decrypted
verifier.verifyTextInLog("[WARNING] Pre-Maven 4 legacy encrypted password detected");
verifier.verifyTextInLog(
"[INFO] Some problems were encountered while building the effective settings (use -X to see details)");
verifier.verifyTextInLog("<password>testtest</password>");
verifier.verifyTextInLog("<value>testtest</value>");
}
@ -69,6 +70,9 @@ class MavenITmng8379SettingsDecryptTest extends AbstractMavenIntegrationTestCase
verifier.verifyErrorFreeLog();
// there is no warning and all fields decrypted
verifier.verifyTextNotInLog("[WARNING]");
verifier.verifyTextNotInLog(
"[INFO] Some problems were encountered while building the effective settings (use -X to see details)");
verifier.verifyTextInLog("<password>testtest</password>");
verifier.verifyTextInLog("<value>secretHeader</value>");
}