Skip to content

Commit b291caa

Browse files
Fix Maven reactor VEX over a base-version module (#584) (#1802)
* Start fix for #584 Assisted-by: Claude Code:claude-opus-5-5 * Withhold Maven VEX for a module left at base After a reactor was vendored, adding a module (or a dependency) that declares the patched artifact at its plain base version made that module build Central's unpatched jar. `vendor --check` reported the drift, but `vex` still attested not_affected, because its liveness check only looked for ${property} versions it could not resolve. The check now also withholds attestation when the planner would still rewrite a declaration, so `vex` skips the package as vendor_unwired until `vendor` is re-run. Fixes #584. Assisted-by: Claude Code:claude-opus-5-5 * Test Maven VEX over an earlier patch's suffix A module still declaring an earlier patch's suffixed version builds that patch's jar, so the reactor must not attest the current one. Pin that behavior with a test. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0ba3bf1 commit b291caa

2 files changed

Lines changed: 105 additions & 5 deletions

File tree

‎crates/socket-patch-cli/tests/vendor_jvm_cli.rs‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,6 +1237,62 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
12371237
}
12381238
}
12391239

1240+
/// #584: a module added after vendoring that declares the base literally
1241+
/// builds the unpatched jar. `vendor --check` flags the drift and `vex`
1242+
/// withholds attestation until a re-vendor rewrites the module.
1243+
#[test]
1244+
fn maven_reactor_module_added_at_base_after_vendoring_withholds_vex() {
1245+
let tmp = tempfile::tempdir().unwrap();
1246+
let root = tmp.path();
1247+
fixture(root, Shape::Reactor, &[("foo", FOO_UUID)]);
1248+
let pristine = std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap();
1249+
ok(root, &["vendor"]);
1250+
let vex = root.join("vex.json");
1251+
let vex_args = [
1252+
"vex",
1253+
"-O",
1254+
vex.to_str().unwrap(),
1255+
"--product",
1256+
"pkg:generic/x@1",
1257+
];
1258+
ok(root, &vex_args);
1259+
std::fs::remove_file(&vex).unwrap();
1260+
1261+
let agg = std::fs::read_to_string(root.join("proj/pom.xml")).unwrap();
1262+
let agg = agg.replace(
1263+
"<module>a</module>",
1264+
"<module>a</module>\n <module>b</module>",
1265+
);
1266+
write(root, "proj/pom.xml", agg.as_bytes());
1267+
std::fs::create_dir_all(root.join("proj/b")).unwrap();
1268+
let b = pristine.replace("<artifactId>a</artifactId>", "<artifactId>b</artifactId>");
1269+
assert!(b.contains("<version>1.0</version></dependency>"), "{b}");
1270+
write(root, "proj/b/pom.xml", b.as_bytes());
1271+
1272+
let (code, env) = socket(root, &["vendor", "--check"]);
1273+
assert_ne!(code, Some(0), "{env}");
1274+
let (_, env) = socket(root, &vex_args);
1275+
assert_eq!(
1276+
events(&env),
1277+
[(
1278+
purl("foo"),
1279+
"skipped".to_string(),
1280+
"vendor_unwired".to_string()
1281+
)],
1282+
"{env}"
1283+
);
1284+
assert!(!vex.exists(), "{env}");
1285+
1286+
// Re-vendoring wires `b` too, and the reactor attests again.
1287+
ok(root, &["vendor"]);
1288+
assert!(!std::fs::read_to_string(root.join("proj/b/pom.xml"))
1289+
.unwrap()
1290+
.contains("<version>1.0</version></dependency>"));
1291+
ok(root, &vex_args);
1292+
let doc: serde_json::Value = serde_json::from_slice(&std::fs::read(&vex).unwrap()).unwrap();
1293+
assert_eq!(doc["statements"][0]["status"], "not_affected", "{doc}");
1294+
}
1295+
12401296
/// #429 repair: a deleted owned `.gitattributes` (and nothing else) is
12411297
/// rewritten by `repair` (no download), so the next autocrlf clone keeps the
12421298
/// script and index LF.

‎crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -479,9 +479,11 @@ pub fn wired_checked(
479479
}
480480

481481
/// [`wired_checked`], and no declaration of `c`'s g:a is left at a version
482-
/// that may still read the base: a `${...}` the checkout does not define
483-
/// or that an inheriting pom or a profile moves (#513, #459). Such a
484-
/// module builds the unpatched jar, so `vex` must not attest.
482+
/// that may still read the base: the base itself, as a module or
483+
/// dependency added since vendoring declares it (#584), or a `${...}` the
484+
/// checkout does not define or that an inheriting pom or a profile moves
485+
/// (#513, #459). Such a module builds the unpatched jar, so `vex` must
486+
/// not attest.
485487
pub fn attestable_checked(
486488
read: ReadFn<'_>,
487489
list: ListFn<'_>,
@@ -507,20 +509,21 @@ pub fn attestable_checked(
507509
extra_artifacts: &[],
508510
patched_members: &[],
509511
};
512+
let mut edits = BTreeMap::new();
510513
let mut uncertain = BTreeSet::new();
511514
for rel in &reactor.scope {
512515
reactor.rewrite_declarations(
513516
rel,
514517
&patch,
515518
&sv,
516-
&mut BTreeMap::new(),
519+
&mut edits,
517520
&mut Vec::new(),
518521
&mut BTreeSet::new(),
519522
&mut BTreeSet::new(),
520523
&mut uncertain,
521524
);
522525
}
523-
Ok(uncertain.is_empty())
526+
Ok(edits.is_empty() && uncertain.is_empty())
524527
}
525528

526529
/// `<!-- socket-patch <uuid>:` — the start of `c`'s pin comment.
@@ -5746,6 +5749,47 @@ mod tests {
57465749
assert_eq!(attestable(&after), (true, true));
57475750
}
57485751

5752+
/// #584: a module added after vendoring, or a dependency added to an
5753+
/// existing one, that declares the base literally beats the root's
5754+
/// pin and builds the unpatched jar: not attestable until re-vendored.
5755+
#[test]
5756+
fn a_base_declaration_added_after_vendoring_is_not_attestable() {
5757+
let files = fs(&[
5758+
("pom.xml", ROOT),
5759+
("a/pom.xml", &module("a", &dep("1.10.0"))),
5760+
("b/pom.xml", &module("b", "")),
5761+
]);
5762+
let (_, after) = vendor(&files);
5763+
assert_eq!(attestable(&after), (true, true));
5764+
5765+
let mut added_module = after.clone();
5766+
added_module.insert(
5767+
"pom.xml".into(),
5768+
text(&after, "pom.xml")
5769+
.replace(
5770+
"<module>b</module>",
5771+
"<module>b</module>\n <module>c</module>",
5772+
)
5773+
.into_bytes(),
5774+
);
5775+
added_module.insert("c/pom.xml".into(), module("c", &dep("1.10.0")).into_bytes());
5776+
let mut added_dep = after.clone();
5777+
added_dep.insert("b/pom.xml".into(), module("b", &dep("1.10.0")).into_bytes());
5778+
// A module still on an earlier patch's suffix builds that patch's
5779+
// jar, not this one.
5780+
let mut earlier_patch = after.clone();
5781+
earlier_patch.insert(
5782+
"b/pom.xml".into(),
5783+
module("b", &dep("1.10.0-socket.deadbeef")).into_bytes(),
5784+
);
5785+
for drifted in [added_module, added_dep, earlier_patch] {
5786+
assert_eq!(attestable(&drifted), (true, false));
5787+
// Re-vendoring rewrites it and the reactor attests again.
5788+
let (_, again) = vendor(&drifted);
5789+
assert_eq!(attestable(&again), (true, true));
5790+
}
5791+
}
5792+
57495793
/// #513: a possibly-matching declaration named through an undefined
57505794
/// `${...}` groupId with its own version beats any pin.
57515795
#[test]

0 commit comments

Comments
 (0)