Repository navigation
Conversation
only-mvnw silently rewrites the distributionUrl from .zip to .tar.gz when 'unzip' is unavailable. The pinned distributionSha256Sum is computed for the .zip only, so validation then compared the downloaded .tar.gz against the .zip's checksum and always failed with a misleading "your Maven distribution might be compromised" error (e.g. on minimal images such as eclipse-temurin that lack unzip). When a checksum is pinned and 'unzip' is missing, refuse up front with an actionable message instead of downloading the wrong artifact. The .tar.gz fallback is unchanged for the common case where no checksum is pinned. Fixes apache#425 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Any updates on this? |
I'm ready to go 🙂 @bmarwell You kindly reviewed this. Any pointers how we get this merged? Thanks. |
|
can't we just drop tar.gz support as sdkman and validate prerequisite early? not sure there is a good point to support a ton of tar flavors plus unzip (but not zip for ex), so rationalizing it to just unzip with early prerequisite validation sounds better no? |
|
Thanks for looking. I'd keep that separate from this fix, for two reasons:
This PR already validates the prerequisite early for the broken case: if It's a minimal, non-breaking fix for #425 and doesn't stand in the way of going unzip-only later if you decide to. Could we get it merged as is? |
good point but since we already depend on java (and actually a jdk) we can use that said agree your PR is not regressing, just wanted to highlight we might want to break how mvnw was designed before it reached apache and make something way simpler and more resilient to the environment |
|
Agreed, a simpler mvnw would be nice. One thing to keep in mind if Thanks for confirming this one doesn't regress anything; could you or @bmarwell (who approved earlier) merge it? |
|
I wouldn't mind, but I haven't been much involved lately. I'll ping some folks to get this into the next release. |
|
FWIW the jar flavor: #476 |
Nice, that's a cleaner end state, and it fixes #425 too. One catch in #476: it only chmods How about merging this one as the minimal fix for the next release, with #476 following as the rework? Happy to close this one instead if you'd rather go straight to #476. |
|
@simono no strong opinion, think it makes sense to go ASAP to the final solution, now I'm not yet sure everybody will converge to the jar option so will depend that I think. |
What / Why
Fixes #425.
On systems without
unzip(e.g. minimal images such aseclipse-temurin), theonly-scriptmvnwsilently rewrites thedistributionUrlfrom-bin.zipto-bin.tar.gz. ButdistributionSha256Suminmaven-wrapper.propertiesis computed for the.zip— the wrapper plugin only ever writes the zip's checksum. The script then validates the downloaded.tar.gzagainst the.zip's checksum, which always fails with:This is confusing to debug: the archive is genuine, and nothing is compromised — the checksum simply belongs to a different artifact than the one downloaded. There is no second property to hold the tar's checksum, so the fallback and checksum pinning are fundamentally incompatible.
How
When
unzipis unavailable and adistributionSha256Sumis pinned, refuse up front with an actionable message instead of downloading the.tar.gzand failing validation against the wrong checksum:The
.tar.gzfallback is unchanged for the common case where no checksum is pinned. This never validates the wrong artifact and never skips validation, so it does not weaken the integrity guarantee — it fails safe, earlier and more clearly.Only the POSIX
only-mvnwis affected;only-mvnw.cmdhandles.ziponly and has no such fallback.Following this checklist to help us incorporate your contribution quickly and easily:
unzipis absent, which the existing JUnit + Invoker IT harness cannot easily simulate; I kept this PR to the minimal fix. Happy to add a test if maintainers prefer a particular approach.mvn verifyto make sure basic checks pass.mvn -Prun-its verify).🤖 Generated with Claude Code