[agent] Found by the scheduled Maven bug-hunt routine (ledger #318).
Summary
Since Maven 3.9.0, a .mvn/maven.config line that starts with # is a comment, and Maven 4 keeps that rule. The reactor planner's cli_properties doesn't skip comments. It splits the whole file on whitespace and keeps every token that starts with -D and contains =:
|
/// `-Dk=v` tokens of a `maven.config`. |
|
fn cli_properties(config: &str) -> BTreeMap<String, String> { |
|
config |
|
.split_whitespace() |
|
.filter_map(|t| t.strip_prefix("-D")?.split_once('=')) |
|
.map(|(k, v)| (k.to_string(), v.to_string())) |
|
.collect() |
|
} |
So # -Dct.version=1.10.0 (an old pin someone commented out) still becomes the user property ct.version=1.10.0. Because lookup lets user properties beat model properties (maven_reactor.rs:939), that phantom value overrides the pom's real <ct.version>.
This is the reverse of #535. There, the parser misses properties that Maven applies. Here, it invents properties that Maven ignores. The cause sits in the same function, but the inputs and the regression test are different.
Impact
- Silent downgrade (the main case). A pom with
<ct.version>1.11.0</ct.version> and a commented-out # -Dct.version=1.10.0 line builds 1.11.0. After vendor, ${ct.version} in the module is replaced by the literal 1.10.0-socket.<hex>. vendor exits 0 with applied: 1, and the only warnings are the generic maven_f_outside_root / maven_mirror_of_all. vendor --check reports vendor_check_ok, and vex attests not_affected. Every fix in 1.11.0 outside the patch is lost, and a later bump of the property no longer takes effect.
- Spurious refusal (the reverse case). A pom with
<ct.version>1.10.0</ct.version> and a commented-out # -Dct.version=1.11.0 line gets conflicting_literal_version, so nothing is pinned and the build keeps Central's unpatched 1.10.0. This direction fails closed (warned, and vendor --check fails), but the patch is refused when it should apply.
Repro
This is the e2e_vendor_jvm_build::maven_reactor fixture (aggregator, corp-parent/, modules a and b), with:
mkdir -p .mvn && printf '# -Dct.version=1.10.0\n' > .mvn/maven.config
mvn -B package org.apache.maven.plugins:maven-dependency-plugin:3.5.0:build-classpath -Dmdep.outputFile=target/cp.txt
# a/target/cp.txt → commons-text-1.11.0.jar (Maven ignores the comment)
# stage the commons-text 1.10.0 patch manifest (as the capstone does), then
socket-patch vendor --json --offline
# applied: 1, warnings only maven_f_outside_root / maven_mirror_of_all
# a/pom.xml: <version>${ct.version}</version> → <version>1.10.0-socket.1d3c1fd2</version>
mvn -B package ...build-classpath...
# a/ and b/ cp.txt → .socket/vendor/maven2/.../commons-text-1.10.0-socket.1d3c1fd2.jar ← downgraded
socket-patch vendor --check --offline --json # verified: 1, vendor_check_ok
socket-patch vex --offline --output vex.json # not_affected / inline_mitigations_already_exist
# -Dct.version=1.10.0 old pin (extra spaces and trailing words) behaves the same way.
Controls:
- The same project without
.mvn/maven.config gets conflicting_literal_version, no pin, and the build stays on 1.11.0.
# just a note with the pom at 1.10.0 is patched correctly.
Expected vs actual
- Expected: the planner models Maven's own user properties (
lookup: "User properties win over model properties, as in Maven"), so lines that Maven treats as comments define nothing. docs/design/maven-vendoring.md says that conflicting explicit versions "produce specific warnings; the backend does not silently claim those unsupported declarations are patched." With the comment ignored, the main case is a conflicting_literal_version (1.11.0 ≠ 1.10.0) with no pin, exactly like the no-config control.
- Actual: the commented-out value is used. The resolved 1.11.0 is rewritten to
1.10.0-socket.*, and vendor --check and VEX both confirm it.
Matrix (Linux, JDK 21, main 61cfb9b)
| Maven |
pom 1.11.0 + # -Dct.version=1.10.0 |
pom 1.10.0 + # -Dct.version=1.11.0 |
no config / # just a note (controls) |
| 3.6.3 |
n/a: Maven rejects # lines itself |
n/a |
— |
| 3.8.8 |
n/a: Maven rejects # lines itself |
n/a |
— |
| 3.9.11 |
fail: 1.11.0 → 1.10.0-socket.* (reproduced 3×) |
fail: refused, unpatched |
pass |
| 3.9.16 |
fail (2×, one with the old pin variant) |
fail |
pass |
| 4.0.0-rc-7 |
fail (2×) |
fail |
pass |
macOS and Windows weren't probed. The parsing is OS-independent.
First bad commit
2463257 (#277, the v5 consolidation that added the reactor planner and cli_properties). It isn't in any release yet (latest release 4.0.0).
Suspect code: crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs:1250 (cli_properties, which drops no # lines before tokenising). A fix there would naturally handle #535 too: parse per line, skip # lines, and accept --define= / -D k=v.
[agent] Found by the scheduled Maven bug-hunt routine (ledger #318).
Summary
Since Maven 3.9.0, a
.mvn/maven.configline that starts with#is a comment, and Maven 4 keeps that rule. The reactor planner'scli_propertiesdoesn't skip comments. It splits the whole file on whitespace and keeps every token that starts with-Dand contains=:socket-patch/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs
Lines 1249 to 1256 in 61cfb9b
So
# -Dct.version=1.10.0(an old pin someone commented out) still becomes the user propertyct.version=1.10.0. Becauselookuplets user properties beat model properties (maven_reactor.rs:939), that phantom value overrides the pom's real<ct.version>.This is the reverse of #535. There, the parser misses properties that Maven applies. Here, it invents properties that Maven ignores. The cause sits in the same function, but the inputs and the regression test are different.
Impact
<ct.version>1.11.0</ct.version>and a commented-out# -Dct.version=1.10.0line builds 1.11.0. Aftervendor,${ct.version}in the module is replaced by the literal1.10.0-socket.<hex>.vendorexits 0 withapplied: 1, and the only warnings are the genericmaven_f_outside_root/maven_mirror_of_all.vendor --checkreportsvendor_check_ok, andvexattestsnot_affected. Every fix in 1.11.0 outside the patch is lost, and a later bump of the property no longer takes effect.<ct.version>1.10.0</ct.version>and a commented-out# -Dct.version=1.11.0line getsconflicting_literal_version, so nothing is pinned and the build keeps Central's unpatched 1.10.0. This direction fails closed (warned, andvendor --checkfails), but the patch is refused when it should apply.Repro
This is the
e2e_vendor_jvm_build::maven_reactorfixture (aggregator,corp-parent/, modulesaandb), with:corp-parent/pom.xml<properties>holding<ct.version>1.11.0</ct.version>.a/pom.xmldeclaring commons-text at<version>${ct.version}</version>.b/pom.xmlwithrelativePath../corp-parent/pom.xml, to stay clear of vex and vendor --check refuse a correctly vendored Maven reactor whose <parent><relativePath> names a directory (../corp-parent or ..), with "is not a regular file" / "unsafe path" #534 for thevexstep.# -Dct.version=1.10.0 old pin(extra spaces and trailing words) behaves the same way.Controls:
.mvn/maven.configgetsconflicting_literal_version, no pin, and the build stays on 1.11.0.# just a notewith the pom at 1.10.0 is patched correctly.Expected vs actual
lookup: "User properties win over model properties, as in Maven"), so lines that Maven treats as comments define nothing.docs/design/maven-vendoring.mdsays that conflicting explicit versions "produce specific warnings; the backend does not silently claim those unsupported declarations are patched." With the comment ignored, the main case is aconflicting_literal_version(1.11.0 ≠ 1.10.0) with no pin, exactly like the no-config control.1.10.0-socket.*, andvendor --checkand VEX both confirm it.Matrix (Linux, JDK 21, main
61cfb9b)# -Dct.version=1.10.0# -Dct.version=1.11.0# just a note(controls)#lines itself#lines itselfold pinvariant)macOS and Windows weren't probed. The parsing is OS-independent.
First bad commit
2463257(#277, the v5 consolidation that added the reactor planner andcli_properties). It isn't in any release yet (latest release 4.0.0).Suspect code:
crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs:1250(cli_properties, which drops no#lines before tokenising). A fix there would naturally handle #535 too: parse per line, skip#lines, and accept--define=/-D k=v.