(maven) branch master updated: Fix #13084 follow-up: fix mutable map leak in compat sandbox context, pre-filter missing()-condition profiles in CONSUMER_DEPENDENCY builds (#13164)

[gn...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git


The following commit(s) were added to refs/heads/master by this push:
     new cce49bc243 Fix #13084 follow-up: fix mutable map leak in compat sandbox context, pre-filter missing()-condition profiles in CONSUMER_DEPENDENCY builds (#13164)
cce49bc243 is described below

commit cce49bc2430a09b2c89a2ceaf13f252ac05726d7
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Thu Sep 17 18:34:31 2026 +0200

    Fix #13084 follow-up: fix mutable map leak in compat sandbox context, pre-filter missing()-condition profiles in CONSUMER_DEPENDENCY builds (#13164)
    
    Forward-port of findings from the review of PR #13158 (4.0.x backport):
    
    - compat: externalActivationContext() returned mutable map from getSystemProperties()
      on both the empty-projectProps branch (raw delegate.getSystemProperties()) and the
      non-empty branch (unwrapped HashMap). Wrap both with Collections.unmodifiableMap().
    
    - impl: the nonFileProfiles pre-filter only excluded profiles with file activation.
      A condition profile using missing(path) passes the filter and enters the sandbox
      where context.exists() always returns false, so missing() = !false = true fires
      unconditionally — the same footgun as <file><missing>. Add hasFileConditionExpression()
      helper and extend the filter to also exclude condition profiles containing exists()/missing().
    
    Tests: extended testProjectBuildEvaluatesAllActivators (missing-condition fires in
    project build) and testDependencyModelActivatesOnlyEnvironmentIndependentProfiles
    (missing-condition suppressed in CONSUMER_DEPENDENCY).
---
 .../maven/model/building/DefaultModelBuilder.java   |  9 +++++----
 .../maven/impl/model/DefaultModelBuilder.java       | 21 ++++++++++++++++++++-
 .../maven/impl/model/DefaultModelBuilderTest.java   |  7 +++++++
 .../poms/factory/resolved-model-with-profiles.xml   | 12 ++++++++++++
 4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java b/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
index 97f4291ce9..20179b80c2 100644
--- a/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
+++ b/compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java
@@ -567,11 +567,12 @@ public Map<String, String> getSystemProperties() {
                 }
                 Map<String, String> projectProps = delegate.getProjectProperties();
                 if (projectProps == null || projectProps.isEmpty()) {
-                    return delegate.getSystemProperties();
+                    mergedSystemProperties = Collections.unmodifiableMap(delegate.getSystemProperties());
+                } else {
+                    Map<String, String> merged = new HashMap<>(projectProps);
+                    merged.putAll(delegate.getSystemProperties()); // system wins
+                    mergedSystemProperties = Collections.unmodifiableMap(merged);
                 }
-                Map<String, String> merged = new HashMap<>(projectProps);
-                merged.putAll(delegate.getSystemProperties()); // system wins
-                mergedSystemProperties = merged;
                 return mergedSystemProperties;
             }
 
diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java
index d698c5dddb..8183eb5813 100644
--- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java
+++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java
@@ -1671,9 +1671,12 @@ private List<Profile> getActiveProfiles(
                     // project → dep1 → dep2 pattern. See #13100.
                     // TODO(#13146): repositories contributed by external-model profiles can shadow
                     // central; a WARN/FAIL policy for URL mismatches should be added separately.
+                    // Condition profiles using exists()/missing() are also pre-filtered for the
+                    // same reason as file profiles: in the sandbox, context.exists() always returns
+                    // false, so missing(path) evaluates to !false = true and fires unconditionally.
                     Collection<Profile> nonFileProfiles = interpolatedProfiles.stream()
                             .filter(p -> p.getActivation() == null
-                                    || p.getActivation().getFile() == null)
+                                    || (p.getActivation().getFile() == null && !hasFileConditionExpression(p)))
                             .toList();
                     ProfileActivationContext externalContext =
                             profileActivationContext.withoutUserPropertiesAndFilesystem();
@@ -1719,6 +1722,22 @@ private static boolean hasFileOrPropertyOrConditionActivation(Profile profile) {
                                     && !activation.getCondition().isBlank()));
         }
 
+        /**
+         * Returns {@code true} if the profile's condition expression calls {@code exists()} or
+         * {@code missing()}.  Such profiles must be pre-filtered out when building external
+         * (repository-resolved) models: inside the sandbox, {@code context.exists()} always
+         * returns {@code false}, so {@code missing(path)} evaluates to {@code !false = true}
+         * and fires unconditionally — the same footgun that file-activated profiles expose.
+         */
+        private static boolean hasFileConditionExpression(Profile profile) {
+            Activation a = profile.getActivation();
+            if (a == null || a.getCondition() == null) {
+                return false;
+            }
+            String c = a.getCondition();
+            return c.contains("exists(") || c.contains("missing(");
+        }
+
         /**
          * Packaging-activated profiles are handled separately by the consumer POM builder's
          * {@code inlinePackagingActivatedProfiles()} and must not be activated during
diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java
index 377e78cbd3..9a5af0e462 100644
--- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java
+++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java
@@ -170,6 +170,9 @@ public void testProjectBuildEvaluatesAllActivators() {
         assertEquals("activated", model.getProperties().get("profile.file"));
         assertEquals("activated", model.getProperties().get("profile.property"));
         assertEquals("activated", model.getProperties().get("profile.condition"));
+        // missing('/publisher/private/file') fires in a project build because that path
+        // genuinely doesn't exist on the developer's machine — normal project-build behaviour.
+        assertEquals("activated", model.getProperties().get("profile.missing.condition"));
         assertEquals("activated", model.getProperties().get("profile.jdk"));
         assertEquals("activated", model.getProperties().get("profile.negated.property"));
         assertTrue(model.getRepositories().stream().anyMatch(r -> "profile-repo".equals(r.getId())));
@@ -187,6 +190,10 @@ public void testDependencyModelActivatesOnlyEnvironmentIndependentProfiles() {
         assertNull(model.getProperties().get("profile.file"));
         assertNull(model.getProperties().get("profile.property"));
         assertNull(model.getProperties().get("profile.condition"));
+        // missing()-condition profiles must be pre-filtered in CONSUMER_DEPENDENCY builds:
+        // in the sandbox context.exists() always returns false, so missing(path) evaluates
+        // to !false = true and fires unconditionally — same footgun as <file><missing>.
+        assertNull(model.getProperties().get("profile.missing.condition"));
         assertEquals("activated", model.getProperties().get("profile.jdk"));
         // Negated-property profile (!skip.defaults) fires because 'skip.defaults' is absent
         // from the sandboxed context (user properties are suppressed; system properties do not
diff --git a/impl/maven-impl/src/test/resources/poms/factory/resolved-model-with-profiles.xml b/impl/maven-impl/src/test/resources/poms/factory/resolved-model-with-profiles.xml
index e3c53aefed..fd30698c24 100644
--- a/impl/maven-impl/src/test/resources/poms/factory/resolved-model-with-profiles.xml
+++ b/impl/maven-impl/src/test/resources/poms/factory/resolved-model-with-profiles.xml
@@ -52,6 +52,18 @@
                 <profile.condition>activated</profile.condition>
             </properties>
         </profile>
+        <!-- missing()-condition profile: must NOT fire in CONSUMER_DEPENDENCY builds -->
+        <!-- missing() = !exists() and in the sandbox exists() always returns false,  -->
+        <!-- so missing() would always return true — this profile must be pre-filtered. -->
+        <profile>
+            <id>missing-condition</id>
+            <activation>
+                <condition>missing('/publisher/private/file')</condition>
+            </activation>
+            <properties>
+                <profile.missing.condition>activated</profile.missing.condition>
+            </properties>
+        </profile>
         <!-- default-on profile: fires when 'skip.defaults' is absent; suppressed when set as system property -->
         <profile>
             <id>negated-property-default-on</id>