gnodet-bot commented on code in PR #13542: URL: https://github.com/apache/maven/pull/13542#discussion_r4281902611
########## impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultBuildEnvironment.java: ########## @@ -0,0 +1,170 @@ +/* + * 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.internal.build; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.execution.MavenSession; + +/** + * Internal immutable implementation of {@link BuildEnvironment}. + * + * @param goals requested goals/phases + * @param userProperties user properties with sensitive values redacted + * @param systemInfo curated system properties for reproducibility + * @param localRepository local repository path + * @param activeProfiles explicitly activated profiles + * @param selectedProjects projects selected via {@code -pl} + * @param resumeFrom resume-from selector, or {@code null} + * @param reactorFailureBehavior reactor failure behavior + * @param offline whether offline mode was active + * @param updateSnapshots whether snapshot updates were forced + * @param noTransferProgress whether transfer progress was suppressed + * @param batchMode whether batch mode was active + * @param threads degree of concurrency + */ +public record DefaultBuildEnvironment( + List<String> goals, + Map<String, String> userProperties, + Map<String, String> systemInfo, + String localRepository, + List<String> activeProfiles, + List<String> selectedProjects, + String resumeFrom, + String reactorFailureBehavior, + boolean offline, + boolean updateSnapshots, + boolean noTransferProgress, + boolean batchMode, + int threads) + implements BuildEnvironment { + + private static final Set<String> SENSITIVE_KEY_FRAGMENTS = + Set.of("password", "passwd", "secret", "token", "apikey", "api_key", "credential", "passphrase"); + + @Override + public List<String> goals() { + return List.copyOf(goals); + } + + @Override + public Map<String, String> userProperties() { + return Map.copyOf(userProperties); + } + + @Override + public Map<String, String> systemInfo() { + return Map.copyOf(systemInfo); + } + + @Override + public List<String> activeProfiles() { + return List.copyOf(activeProfiles); + } + + @Override + public List<String> selectedProjects() { + return List.copyOf(selectedProjects); + } + + /** + * Creates a {@link BuildEnvironment} snapshot from the current session state. + * + * @param session the Maven session + * @return a new immutable build environment instance + */ + public static BuildEnvironment from(MavenSession session) { + var request = session.getRequest(); + + List<String> goals = request.getGoals() != null ? List.copyOf(request.getGoals()) : List.of(); + + Map<String, String> userProps = new LinkedHashMap<>(); + if (request.getUserProperties() != null) { + request.getUserProperties().forEach((k, v) -> { + String key = String.valueOf(k); + String value = isSensitiveKey(key) ? "***" : String.valueOf(v); + userProps.put(key, value); + }); + } + + Map<String, String> sysInfo = new TreeMap<>(); + addSystemProp(sysInfo, "os.name"); + addSystemProp(sysInfo, "os.arch"); + addSystemProp(sysInfo, "os.version"); + addSystemProp(sysInfo, "java.vendor"); + addSystemProp(sysInfo, "java.vm.name"); + addSystemProp(sysInfo, "java.vm.version"); + addSystemProp(sysInfo, "maven.home"); + addSystemProp(sysInfo, "user.home"); + addSystemProp(sysInfo, "user.name"); + sysInfo.put("available.processors", String.valueOf(Runtime.getRuntime().availableProcessors())); + + String localRepo = request.getLocalRepositoryPath() != null + ? request.getLocalRepositoryPath().getAbsolutePath() + : ""; + + List<String> activeProfiles = + request.getActiveProfiles() != null ? List.copyOf(request.getActiveProfiles()) : List.of(); + + List<String> selectedProjects = + request.getSelectedProjects() != null ? List.copyOf(request.getSelectedProjects()) : List.of(); + + String resumeFrom = request.getResumeFrom(); + + String failureBehavior = + request.getReactorFailureBehavior() != null ? request.getReactorFailureBehavior() : "FAIL_FAST"; + + return new DefaultBuildEnvironment( + goals, + userProps, + sysInfo, + localRepo, + activeProfiles, + selectedProjects, + resumeFrom, + failureBehavior, + request.isOffline(), + request.isUpdateSnapshots(), + false, Review Comment: 🐛 **Bug: `noTransferProgress` hardcoded to `false`** The literal `false` here ignores the actual request flag. Should be: ```suggestion request.isNoTransferProgress(), ``` ########## impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultBuildEnvironment.java: ########## @@ -0,0 +1,170 @@ +/* + * 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.internal.build; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.execution.MavenSession; + +/** + * Internal immutable implementation of {@link BuildEnvironment}. + * + * @param goals requested goals/phases + * @param userProperties user properties with sensitive values redacted + * @param systemInfo curated system properties for reproducibility + * @param localRepository local repository path + * @param activeProfiles explicitly activated profiles + * @param selectedProjects projects selected via {@code -pl} + * @param resumeFrom resume-from selector, or {@code null} + * @param reactorFailureBehavior reactor failure behavior + * @param offline whether offline mode was active + * @param updateSnapshots whether snapshot updates were forced + * @param noTransferProgress whether transfer progress was suppressed + * @param batchMode whether batch mode was active + * @param threads degree of concurrency + */ +public record DefaultBuildEnvironment( + List<String> goals, + Map<String, String> userProperties, + Map<String, String> systemInfo, + String localRepository, + List<String> activeProfiles, + List<String> selectedProjects, + String resumeFrom, + String reactorFailureBehavior, + boolean offline, + boolean updateSnapshots, + boolean noTransferProgress, + boolean batchMode, + int threads) + implements BuildEnvironment { + + private static final Set<String> SENSITIVE_KEY_FRAGMENTS = + Set.of("password", "passwd", "secret", "token", "apikey", "api_key", "credential", "passphrase"); + + @Override + public List<String> goals() { + return List.copyOf(goals); + } Review Comment: ⚠️ **Redundant copy on every accessor call** Calling `List.copyOf(goals)` on each accessor invocation allocates a new list every time. Since `from()` already produces immutable collections, these per-call copies serve no purpose. Add a compact constructor that normalises once: ```java public DefaultBuildEnvironment { goals = List.copyOf(goals); userProperties = Map.copyOf(userProperties); systemInfo = Map.copyOf(systemInfo); activeProfiles = List.copyOf(activeProfiles); selectedProjects = List.copyOf(selectedProjects); } ``` Then remove all these overriding accessors — the generated ones return the field directly. ########## impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultBuildEnvironment.java: ########## @@ -0,0 +1,170 @@ +/* + * 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.internal.build; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.execution.MavenSession; + +/** + * Internal immutable implementation of {@link BuildEnvironment}. + * + * @param goals requested goals/phases + * @param userProperties user properties with sensitive values redacted + * @param systemInfo curated system properties for reproducibility + * @param localRepository local repository path + * @param activeProfiles explicitly activated profiles + * @param selectedProjects projects selected via {@code -pl} + * @param resumeFrom resume-from selector, or {@code null} + * @param reactorFailureBehavior reactor failure behavior + * @param offline whether offline mode was active + * @param updateSnapshots whether snapshot updates were forced + * @param noTransferProgress whether transfer progress was suppressed + * @param batchMode whether batch mode was active + * @param threads degree of concurrency + */ +public record DefaultBuildEnvironment( + List<String> goals, + Map<String, String> userProperties, + Map<String, String> systemInfo, + String localRepository, + List<String> activeProfiles, + List<String> selectedProjects, + String resumeFrom, + String reactorFailureBehavior, + boolean offline, + boolean updateSnapshots, + boolean noTransferProgress, + boolean batchMode, + int threads) + implements BuildEnvironment { + + private static final Set<String> SENSITIVE_KEY_FRAGMENTS = + Set.of("password", "passwd", "secret", "token", "apikey", "api_key", "credential", "passphrase"); + + @Override + public List<String> goals() { + return List.copyOf(goals); + } + + @Override + public Map<String, String> userProperties() { + return Map.copyOf(userProperties); + } + + @Override + public Map<String, String> systemInfo() { + return Map.copyOf(systemInfo); + } + + @Override + public List<String> activeProfiles() { + return List.copyOf(activeProfiles); + } + + @Override + public List<String> selectedProjects() { + return List.copyOf(selectedProjects); + } + + /** + * Creates a {@link BuildEnvironment} snapshot from the current session state. + * + * @param session the Maven session + * @return a new immutable build environment instance + */ + public static BuildEnvironment from(MavenSession session) { + var request = session.getRequest(); + + List<String> goals = request.getGoals() != null ? List.copyOf(request.getGoals()) : List.of(); + + Map<String, String> userProps = new LinkedHashMap<>(); + if (request.getUserProperties() != null) { + request.getUserProperties().forEach((k, v) -> { + String key = String.valueOf(k); + String value = isSensitiveKey(key) ? "***" : String.valueOf(v); + userProps.put(key, value); + }); + } + + Map<String, String> sysInfo = new TreeMap<>(); + addSystemProp(sysInfo, "os.name"); + addSystemProp(sysInfo, "os.arch"); + addSystemProp(sysInfo, "os.version"); + addSystemProp(sysInfo, "java.vendor"); + addSystemProp(sysInfo, "java.vm.name"); + addSystemProp(sysInfo, "java.vm.version"); + addSystemProp(sysInfo, "maven.home"); + addSystemProp(sysInfo, "user.home"); + addSystemProp(sysInfo, "user.name"); Review Comment: 🔒 **PII captured in `systemInfo`** `user.home` and `user.name` are personal identifiers. On developer machines these contain real usernames and home directory paths. If `BuildEnvironment` data flows into build reports or logs these would be exposed. Consider removing these two keys or documenting that callers must scrub them before persisting. ########## impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultBuildEnvironment.java: ########## @@ -0,0 +1,170 @@ +/* + * 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.internal.build; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.execution.MavenSession; + +/** + * Internal immutable implementation of {@link BuildEnvironment}. + * + * @param goals requested goals/phases + * @param userProperties user properties with sensitive values redacted + * @param systemInfo curated system properties for reproducibility + * @param localRepository local repository path + * @param activeProfiles explicitly activated profiles + * @param selectedProjects projects selected via {@code -pl} + * @param resumeFrom resume-from selector, or {@code null} + * @param reactorFailureBehavior reactor failure behavior + * @param offline whether offline mode was active + * @param updateSnapshots whether snapshot updates were forced + * @param noTransferProgress whether transfer progress was suppressed + * @param batchMode whether batch mode was active + * @param threads degree of concurrency + */ +public record DefaultBuildEnvironment( + List<String> goals, + Map<String, String> userProperties, + Map<String, String> systemInfo, + String localRepository, + List<String> activeProfiles, + List<String> selectedProjects, + String resumeFrom, + String reactorFailureBehavior, + boolean offline, + boolean updateSnapshots, + boolean noTransferProgress, + boolean batchMode, + int threads) + implements BuildEnvironment { + + private static final Set<String> SENSITIVE_KEY_FRAGMENTS = + Set.of("password", "passwd", "secret", "token", "apikey", "api_key", "credential", "passphrase"); + + @Override + public List<String> goals() { + return List.copyOf(goals); + } + + @Override + public Map<String, String> userProperties() { + return Map.copyOf(userProperties); + } + + @Override + public Map<String, String> systemInfo() { + return Map.copyOf(systemInfo); + } + + @Override + public List<String> activeProfiles() { + return List.copyOf(activeProfiles); + } + + @Override + public List<String> selectedProjects() { + return List.copyOf(selectedProjects); + } + + /** + * Creates a {@link BuildEnvironment} snapshot from the current session state. + * + * @param session the Maven session + * @return a new immutable build environment instance + */ + public static BuildEnvironment from(MavenSession session) { + var request = session.getRequest(); + + List<String> goals = request.getGoals() != null ? List.copyOf(request.getGoals()) : List.of(); + + Map<String, String> userProps = new LinkedHashMap<>(); + if (request.getUserProperties() != null) { + request.getUserProperties().forEach((k, v) -> { + String key = String.valueOf(k); + String value = isSensitiveKey(key) ? "***" : String.valueOf(v); + userProps.put(key, value); + }); + } + + Map<String, String> sysInfo = new TreeMap<>(); + addSystemProp(sysInfo, "os.name"); + addSystemProp(sysInfo, "os.arch"); + addSystemProp(sysInfo, "os.version"); + addSystemProp(sysInfo, "java.vendor"); + addSystemProp(sysInfo, "java.vm.name"); + addSystemProp(sysInfo, "java.vm.version"); + addSystemProp(sysInfo, "maven.home"); + addSystemProp(sysInfo, "user.home"); + addSystemProp(sysInfo, "user.name"); + sysInfo.put("available.processors", String.valueOf(Runtime.getRuntime().availableProcessors())); + + String localRepo = request.getLocalRepositoryPath() != null + ? request.getLocalRepositoryPath().getAbsolutePath() Review Comment: 💡 **`localRepository()` should return `Path` in a v4 API** Converting `File → String` here is the wrong direction for a new v4 API. Prefer `File → Path`: ```suggestion ? request.getLocalRepositoryPath().toPath() ``` And update `localRepository()` in `BuildEnvironment` to return `Path` rather than `String`. ########## api/maven-api-core/src/main/java/org/apache/maven/api/BuildEnvironment.java: ########## @@ -0,0 +1,203 @@ +/* + * 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; + +import java.util.List; +import java.util.Map; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Immutable; +import org.apache.maven.api.annotations.Nonnull; + +/** + * Describes the invocation context of a Maven build: the flags, properties, and + * environment settings that were active when the build started. + * + * <p>An instance is available via {@link Session#buildEnvironment()} during the build, + * and is also recorded in the structured build report for post-mortem analysis and + * reproducibility. + * + * <h2>What is captured</h2> + * <ul> + * <li>Goals and lifecycle phases requested ({@link #goals()})</li> + * <li>User properties passed via {@code -Dkey=value} ({@link #userProperties()}), + * with sensitive keys redacted — see {@link #userProperties()} for the denylist</li> + * <li>A curated subset of system properties relevant to reproducibility + * ({@link #systemInfo()}): OS name/arch/version, Java vendor and VM name/version, + * Maven home, and available processors</li> + * <li>Local repository path ({@link #localRepository()})</li> + * <li>Explicitly activated or deactivated profiles ({@link #activeProfiles()})</li> + * <li>Selected projects ({@link #selectedProjects()}) and resume-from + * ({@link #resumeFrom()})</li> + * <li>Reactor failure behavior ({@link #reactorFailureBehavior()})</li> + * <li>Offline mode ({@link #offline()}) and snapshot update policy + * ({@link #updateSnapshots()})</li> + * <li>Degree of concurrency ({@link #threads()})</li> + * <li>Transfer-progress suppression ({@link #noTransferProgress()}) and batch mode ({@link #batchMode()})</li> + * </ul> + * + * <h2>What is not yet captured</h2> + * <p>The following information is not currently available through the Maven 4 API + * and therefore cannot be recorded here. It may be added in future versions as + * the API evolves: + * <ul> + * <li><b>Raw command-line arguments</b> ({@code args[]}): the CLI bootstrap layer + * does not surface these through {@code MavenExecutionRequest} or the session</li> + * <li><b>Implicitly activated profiles</b>: profiles activated by OS, JDK, or + * property conditions rather than by explicit {@code -P} flag; computing + * these requires per-project activation and is not available at session level</li> + * </ul> + * + * @since 4.1.0 + * @see Session#buildEnvironment() + */ +@Experimental +@Immutable +public interface BuildEnvironment { + + /** + * The goals or lifecycle phases that were requested on the command line. + * + * @return the requested goals/phases, never {@code null} + */ + @Nonnull + List<String> goals(); + + /** + * User properties passed via {@code -Dkey=value} on the command line or via + * {@code --define}. + * + * <p>Sensitive keys are redacted and replaced with {@code "***"}. A key is + * considered sensitive if its lower-case form contains any of: + * {@code password}, {@code passwd}, {@code secret}, {@code token}, + * {@code apikey}, {@code api_key}, {@code credential}, {@code passphrase}. + * + * @return the user properties, never {@code null}; values of sensitive keys + * are replaced with {@code "***"} + */ + @Nonnull + Map<String, String> userProperties(); + + /** + * A curated subset of system properties capturing platform and JVM identity, + * relevant for build reproducibility analysis. + * + * <p>The following keys are included when present: + * {@code os.name}, {@code os.arch}, {@code os.version}, + * {@code java.vendor}, {@code java.vm.name}, {@code java.vm.version}, + * {@code maven.home}, {@code user.home}, {@code user.name}, + * and {@code available.processors} (from {@code Runtime.getRuntime()}). + * + * <p>The full {@code System.getProperties()} map is intentionally not captured: + * it is large, mostly irrelevant, and may contain sensitive values. + * + * @return the curated system info map, never {@code null} + */ + @Nonnull + Map<String, String> systemInfo(); + + /** + * The path to the local repository used for this build. + * + * @return the local repository path string, never {@code null} + */ + @Nonnull + String localRepository(); + + /** + * Profiles explicitly activated ({@code -P profileId}) or deactivated + * ({@code -P !profileId}) on the command line. + * + * <p>This list reflects only explicit {@code -P} selections. Profiles activated + * implicitly by OS, JDK, or property conditions are not included here — see + * the class-level javadoc for details. + * + * @return the explicitly selected profiles, never {@code null}; may be empty + */ + @Nonnull + List<String> activeProfiles(); + + /** Review Comment: 💡 **`reactorFailureBehavior()` leaks v3 internal string constants** The possible values (`"FAIL_FAST"`, `"FAIL_AT_END"`, `"FAIL_NEVER"`) are `MavenExecutionRequest` constants from the v3 layer. Callers of the v4 API will need to string-match against these undocumented values. Consider defining typed constants on `BuildEnvironment` itself (or an enum) so the API is self-documenting and not coupled to v3 internal names. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
