Elliotte, cool beans! Could you please create PRs for these issues? maven-executor code is currently run in Maven 4.0 and 4.1 ITs and they most probably do not step onto these execution paths, but we do want to get rid of these....
Thanks T On Fri, 12 Jun 2026 at 16:39, Elliotte Rusty Harold <[email protected]> wrote: > > I ran the code through an LLM and told it find bugs. I haven't gone > through these in detail yet but I'm willing to bet some of its > findings are both accurate and serious. Given the nature of > maven-executor it seems like a probable target for various attacks, > and it is extremely likely that any attacker will start by doing > exactly what I've done here, possibly with a more sophisticated LLM > than I used, so more than usual caution is warranted. > > > Bug Report: maven-executor > ========================== > > HIGH SEVERITY > ------------- > > 1. toBuilder().argument(String) throws UnsupportedOperationException > ExecutorRequest.java:150-165, 249-255 > toBuilder() passes arguments() (an unmodifiable list from Impl) > into the Builder constructor, > which stores the reference directly. When argument(String) is > called, it invokes > this.arguments.add(...) on the unmodifiable list, crashing at runtime. > > 2. MAVEN_ARGS leaks to child process in ForkedMavenExecutor > ForkedMavenExecutor.java:109, 143, 149-153 > Line 109 reads System.getenv("MAVEN_ARGS") from the parent process. > Line 143 does > env.remove("MAVEN_ARGS"), but env is a local HashMap containing > only request-specified > variables, not the parent process environment. Line 152 calls > pb.environment().putAll(env) > but MAVEN_ARGS from the parent is never removed from the inherited > environment, so the > child process also inherits it. > > 3. extractMTPSingleArgument throws StringIndexOutOfBoundsException > MavenToolProvider.java (java9):146 > When an argument like -MEX-mavenHome is passed without = and a value, > argument.substring(key.length() + 1) throws > StringIndexOutOfBoundsException. > > 4. Null values from extractMTPMapArgument produce string "null" in output > MavenToolProvider.java (java9):165-169 -> > ForkedMavenExecutor.java:127 -> DockerExeExecutor.java:91 > When -MEX-env=MY_VAR is passed without a =value, the map stores > null. This propagates to > "MY_VAR=null" in DockerExeExecutor and "-Dkey=null" in ForkedMavenExecutor. > > 5. ToolboxExecutorTool.doExecute() uses request.stdOut() (an > OutputStream reference) instead > of result.stdOutString() in error messages > ToolboxExecutorTool.java:144-146 > The error message prints request.stdOut().orElse(null) which > returns the OutputStream object > (e.g., java.io.ByteArrayOutputStream@123456), not the captured > output. The actual stdout/stderr > content is in result.stdOutString()/result.stdErrString() but is never > read. > > MEDIUM SEVERITY > --------------- > > 6. stdIn InputStream is never closed (resource leak) > ProcessBuilderExecutorSupport.java:149-158 > In the stdinPump thread, the stdIn InputStream is read from but > never closed. The > try-with-resources only closes in (the process's OutputStream). The > Javadoc says > "The stream is closed once tool execution is finished" but this > does not happen. > > 7. fs4WithDollar test uses MAVEN3_HOME instead of MAVEN4_HOME (copy-paste > error) > MavenExecutorTestSupport.java:329 > The @Disabled test fs4WithDollar (intended for Maven 4) references > Environment.MAVEN3_HOME > at line 329. > > 8. System.setProperties(null) creates thread-safety window > EmbeddedMavenExecutor.java:383 > prepareProperties() calls System.setProperties(null) followed by > properties.putAll(System.getProperties()). Between these two calls, > any concurrent thread > accessing System.getProperties() may get null or empty properties. > > 9. ExecutorHelperImpl.close() does not catch RuntimeException from > executor close() > ExecutorHelperImpl.java:88-109 > Only ExecutorException is caught when closing executors. If an > executor's close() throws > a RuntimeException (e.g., NullPointerException), it propagates > immediately and prevents > subsequent executors from being closed. > > 10. UncheckedIOException not wrapped as ExecutorException in > TestContainersExecutor > TestContainersExecutor.java:161-166 > detectUid() wraps IOException in UncheckedIOException (a > RuntimeException). The execute() > method's catch blocks only catch IOException. The > UncheckedIOException propagates uncaught. > > 11. NPE if resource files are missing in TestProjects.createSimpleProject > TestProjects.java:37, 40 > TestProjects.class.getResourceAsStream(...) can return null, and > Files.copy(InputStream, Path) > will throw NullPointerException instead of a clear "resource not > found" message. > > LOW SEVERITY > ------------ > > 12. extractMTPSingleArgument removes ALL matching arguments, not just the > first > MavenToolProvider.java (java9):145 > allArguments.removeIf(s -> s.equals(argument)) removes every > occurrence, silently > discarding duplicates. > > 13. Builder.stdOut()/stdErr() don't clear the other stream > ExecutorRequest.java:324-334 > Setting stdOut(...) sets grabOutputAsString = false but does not > clear this.stdErr, > and vice versa. > > 14. Temp directory deletion may fail in ForkedMavenExecutor.mavenVersion() > ForkedMavenExecutor.java:74, 93 > Files.createTempDirectory(...) is deleted in finally via > Files.deleteIfExists(cwd), but > if the Maven process creates files inside this directory, the > delete fails with > DirectoryNotEmptyException. > > 15. nullInputStream.read(byte[], int, int) violates InputStream contract > IOTools.java:83-89 > The read(byte[], int, int) contract requires throwing > IndexOutOfBoundsException if off is > negative, len is negative, or off + len > b.length. No bounds > validation is performed. > > 16. Deprecated ToolboxExecutorTool(Executor) hardcodes 0.15.8 vs current > 0.15.14 > ToolboxExecutorTool.java:53-56 > The deprecated constructor uses an old cemented version while the > POM defines > version.toolbox as 0.15.14. > > 17. @DisabledOnOs comment vs annotation mismatch in Docker provider tests > TestContainersExecutorTest.java:38, DockerExeExecutorTest.java:38 > Comment says "not supported on Windows" but annotation also > disables on OS.MAC. > > > > On Fri, Jun 12, 2026 at 10:41 AM <[email protected]> wrote: > > > > Hello Maven, > > > > I'd like to call a vote on releasing the following artifacts as > > Apache Maven Executor 1.0.0. This vote is being conducted using an > > Alpha version of the Apache Trusted Releases (ATR) platform. > > Please report any bugs or issues to the ASF Tooling team. > > > > The release candidate page, including downloads, can be found at: > > > > https://release-test.apache.org/vote/maven-executor/1.0.0 > > > > The release artifacts are signed with one or more OpenPGP keys from: > > > > https://dist.apache.org/repos/dist/atr/maven/KEYS > > > > Maven staging repository: > > > > https://repository.apache.org/content/repositories/maven-2437/ > > > > Release notes: > > > > https://gist.github.com/cstamas/50b019a71d2821d497cc1af2bb5bc3e8 > > > > Staging site: > > > > https://maven.apache.org/executor-archives/executor-LATEST/ > > > > Please review the release candidate and vote accordingly. > > > > [ ] +1 Release this package > > [ ] +0 Abstain > > [ ] -1 Do not release this package (please provide specific comments) > > > > You can vote on ATR at the URL above, or manually by replying to this email. > > > > The vote is open for 72 hours. > > > > > > Thanks, > > Tamas Cservenak (cstamas) > > > > This email was sent by [email protected] on the Apache Trusted Releases > > platform > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > > -- > Elliotte Rusty Harold > [email protected] > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
