Tarun4201 commented on PR #13277:
URL: https://github.com/apache/maven/pull/13277#issuecomment-5855457241
Thank you for the thorough review! All three issues have been resolved in
commit `b3925954`. Here is a summary of the fixes:
### Fix 1: Removed double class loading from `m2.conf` (Critical - Option A)
The `optionally ${maven.home}/lib/clapp/${maven.clapp.name}/*.jar` line has
been **removed** from `m2.conf`. The `URLClassLoader` inside
`MavenClappCling.launchClapp()` is now the **sole mechanism** for loading
CLAPP-private jars, providing true classpath isolation without touching the
shared `plexus.core` realm.
### Fix 2: Fixed `URLClassLoader` resource leak
Changed the manual `try/finally` to **try-with-resources**, ensuring the
`URLClassLoader` is always closed after the CLAPP invocation completes - even
on exception paths.
```java
// Before (leaked):
URLClassLoader clappLoader = new URLClassLoader(...);
try { ... } finally { restoreContextClassLoader(); }
// After (fixed):
try (URLClassLoader clappLoader = new URLClassLoader(...)) { ... }
finally { restoreContextClassLoader(); }
```
### Fix 3: Prevented path traversal vulnerability
Added tool name validation with an allowlist regex `^[a-zA-Z0-9_-]+$` and an
explicit `startsWith()` check on the resolved path:
```java
if (!clappName.matches("^[a-zA-Z0-9_-]+$")) {
throw new IOException("Invalid CLAPP tool name: '" + clappName + "'");
}
Path baseDir =
Paths.get(mavenHome).resolve(CLAPP_LIB_RELATIVE_PATH).normalize();
Path clappLibDir = baseDir.resolve(clappName).normalize();
if (!clappLibDir.startsWith(baseDir)) {
throw new IOException("Invalid CLAPP lib directory path traversal
attempt for: " + clappName);
}
```
A new unit test `launchClapp_throwsIOException_whenToolNameInvalid()` covers
this case with `"../badtool"`.
---
If this looks good to you, please approve and merge. Happy to make further
changes if needed!
--
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]