[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

api/maven-api-core/src/main/java/org/apache/maven/api/services/MavenBuilderException.java

@@ -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;
+    }
+}

api/maven-api-core/src/main/java/org/apache/maven/api/services/SettingsBuilderException.java

@@ -20,7 +20,6 @@
 
 import java.io.Serial;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import org.apache.maven.api.annotations.Experimental;
 
@@ -30,28 +29,20 @@
  * @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);
     }
 }

api/maven-api-core/src/main/java/org/apache/maven/api/services/ToolchainsBuilderException.java

@@ -20,7 +20,6 @@
 
 import java.io.Serial;
 import java.util.List;
-import java.util.stream.Collectors;
 
 import org.apache.maven.api.annotations.Experimental;
 
@@ -30,28 +29,20 @@
  * @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);
     }
 }

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java

@@ -465,10 +465,18 @@ protected void postCommands(C context) throws Exception {
     }
 
     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 @@ protected void settings(C context, SettingsBuilder settingsBuilder) throws Excep
         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("");
         }
     }
 

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/PlexusContainerCapsuleFactory.java

@@ -75,7 +75,7 @@ public ContainerCapsule createContainerCapsule(LookupInvoker<C> invoker, C conte
             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 @@ protected void configure() {
             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);

impl/maven-impl/src/main/java/org/apache/maven/internal/impl/DefaultSettingsBuilder.java

@@ -29,6 +29,7 @@
 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 @@ private Settings decrypt(
             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 @@ private Settings decrypt(
                             -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) {

its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3748BadSettingsXmlTest.java

@@ -21,6 +21,7 @@
 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 @@
  *
  * @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() {

its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng8379SettingsDecryptTest.java

@@ -47,7 +47,8 @@ void testLegacy() throws Exception {
         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 @@ void testModern() throws Exception {
         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>");
     }