fix: filter possibly sensitive use properties

Author: ppkarwasz

src/main/java/org/apache/commons/release/plugin/internal/BuildDefinitions.java

@@ -21,9 +21,11 @@
 import java.lang.management.ManagementFactory;
 import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
 import java.util.TreeMap;
@@ -37,8 +39,18 @@
  */
 public final class BuildDefinitions {
 
+    /**
+     * User-property names containing any of these substrings (case-insensitive) are omitted from attestations.
+     *
+     * <p>The Maven GPG plugin discourages passing credentials on the command line, but a stray {@code -Dgpg.passphrase=...} must not be captured in the
+     * attestation if someone does it anyway.</p>
+     */
+    private static final List<String> SENSITIVE_KEYWORDS =
+            Arrays.asList("secret", "password", "passphrase", "token", "credential");
+
     /**
      * Reconstructs the Maven command line string from the given execution request.
+     * User properties whose name matches {@link #SENSITIVE_KEYWORDS} are omitted.
      *
      * @param request the Maven execution request
      * @return a string representation of the Maven command line
@@ -49,10 +61,26 @@ static String commandLine(final MavenExecutionRequest request) {
         if (!profiles.isEmpty()) {
             args.add("-P" + profiles);
         }
-        request.getUserProperties().forEach((key, value) -> args.add("-D" + key + "=" + value));
+        request.getUserProperties().forEach((key, value) -> {
+            final String k = key.toString();
+            if (isNotSensitive(k)) {
+                args.add("-D" + k + "=" + value);
+            }
+        });
         return String.join(" ", args);
     }
 
+    /**
+     * Checks if a property key is not sensitive.
+     *
+     * @param property A property key
+     * @return {@code true} if the property is not considered sensitive
+     */
+    private static boolean isNotSensitive(final String property) {
+        final String lower = property.toLowerCase(Locale.ROOT);
+        return SENSITIVE_KEYWORDS.stream().noneMatch(lower::contains);
+    }
+
     /**
      * Returns a map of external build parameters captured from the current JVM and Maven session.
      *
@@ -65,7 +93,7 @@ public static Map<String, Object> externalParameters(final MavenSession session)
         final MavenExecutionRequest request = session.getRequest();
         params.put("maven.goals", request.getGoals());
         params.put("maven.profiles", request.getActiveProfiles());
-        params.put("maven.user.properties", request.getUserProperties());
+        params.put("maven.user.properties", getUserProperties(request));
         params.put("maven.cmdline", commandLine(request));
         final Map<String, Object> env = new HashMap<>();
         params.put("env", env);
@@ -78,6 +106,23 @@ public static Map<String, Object> externalParameters(final MavenSession session)
         return params;
     }
 
+    /**
+     * Returns a filtered map of user properties.
+     *
+     * @param request A Maven request
+     * @return A map of user properties.
+     */
+    private static TreeMap<String, String> getUserProperties(final MavenExecutionRequest request) {
+        final TreeMap<String, String> properties = new TreeMap<>();
+        request.getUserProperties().forEach((k, value) -> {
+            final String key = k.toString();
+            if (isNotSensitive(key)) {
+                properties.put(key, value.toString());
+            }
+        });
+        return properties;
+    }
+
     /**
      * Creates a {@link ResourceDescriptor} for the JDK used during the build.
      *

src/test/java/org/apache/commons/release/plugin/internal/BuildDefinitionsTest.java

@@ -40,15 +40,23 @@ static Stream<Arguments> commandLineArguments() {
                 Arguments.of("multiple goals", asList("clean", "verify"), emptyList(), new Properties(), "clean verify"),
                 Arguments.of("single profile", singletonList("verify"), singletonList("release"), new Properties(), "verify -Prelease"),
                 Arguments.of("multiple profiles", singletonList("verify"), asList("release", "sign"), new Properties(), "verify -Prelease,sign"),
-                Arguments.of("user property", singletonList("verify"), emptyList(), singletonProperties("foo", "bar"), "verify -Dfoo=bar"),
-                Arguments.of("goals, profile and property", singletonList("verify"), singletonList("release"), singletonProperties("foo", "bar"),
-                        "verify -Prelease -Dfoo=bar")
+                Arguments.of("user property", singletonList("verify"), emptyList(), toProperties("foo", "bar"), "verify -Dfoo=bar"),
+                Arguments.of("goals, profile and property", singletonList("verify"), singletonList("release"), toProperties("foo", "bar"),
+                        "verify -Prelease -Dfoo=bar"),
+                Arguments.of("redacts gpg.passphrase", singletonList("verify"), emptyList(), toProperties("gpg.passphrase", "s3cr3t"), "verify"),
+                Arguments.of("redacts passphrase case-insensitively", singletonList("verify"), emptyList(), toProperties("GPG_PASSPHRASE", "s3cr3t"), "verify"),
+                Arguments.of("redacts any *password*", singletonList("verify"), emptyList(), toProperties("my.db.password", "hunter2"), "verify"),
+                Arguments.of("redacts *token*", singletonList("verify"), emptyList(), toProperties("github.token", "ghp_xxx"), "verify"),
+                Arguments.of("keeps safe property, drops sensitive one", singletonList("verify"), emptyList(),
+                        toProperties("foo", "bar", "gpg.passphrase", "s3cr3t"), "verify -Dfoo=bar")
         );
     }
 
-    private static Properties singletonProperties(final String key, final String value) {
+    private static Properties toProperties(final String... keysAndValues) {
         final Properties p = new Properties();
-        p.setProperty(key, value);
+        for (int i = 0; i < keysAndValues.length; i += 2) {
+            p.setProperty(keysAndValues[i], keysAndValues[i + 1]);
+        }
         return p;
     }