Fix Maven reactor VEX over a base-version module (#584)

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

@@ -1237,6 +1237,62 @@ fn maven_reactor_uncertain_property_version_withholds_vex() {
     }
 }
 
+/// #584: a module added after vendoring that declares the base literally
+/// builds the unpatched jar. `vendor --check` flags the drift and `vex`
+/// withholds attestation until a re-vendor rewrites the module.
+#[test]
+fn maven_reactor_module_added_at_base_after_vendoring_withholds_vex() {
+    let tmp = tempfile::tempdir().unwrap();
+    let root = tmp.path();
+    fixture(root, Shape::Reactor, &[("foo", FOO_UUID)]);
+    let pristine = std::fs::read_to_string(root.join("proj/a/pom.xml")).unwrap();
+    ok(root, &["vendor"]);
+    let vex = root.join("vex.json");
+    let vex_args = [
+        "vex",
+        "-O",
+        vex.to_str().unwrap(),
+        "--product",
+        "pkg:generic/x@1",
+    ];
+    ok(root, &vex_args);
+    std::fs::remove_file(&vex).unwrap();
+
+    let agg = std::fs::read_to_string(root.join("proj/pom.xml")).unwrap();
+    let agg = agg.replace(
+        "<module>a</module>",
+        "<module>a</module>\n    <module>b</module>",
+    );
+    write(root, "proj/pom.xml", agg.as_bytes());
+    std::fs::create_dir_all(root.join("proj/b")).unwrap();
+    let b = pristine.replace("<artifactId>a</artifactId>", "<artifactId>b</artifactId>");
+    assert!(b.contains("<version>1.0</version></dependency>"), "{b}");
+    write(root, "proj/b/pom.xml", b.as_bytes());
+
+    let (code, env) = socket(root, &["vendor", "--check"]);
+    assert_ne!(code, Some(0), "{env}");
+    let (_, env) = socket(root, &vex_args);
+    assert_eq!(
+        events(&env),
+        [(
+            purl("foo"),
+            "skipped".to_string(),
+            "vendor_unwired".to_string()
+        )],
+        "{env}"
+    );
+    assert!(!vex.exists(), "{env}");
+
+    // Re-vendoring wires `b` too, and the reactor attests again.
+    ok(root, &["vendor"]);
+    assert!(!std::fs::read_to_string(root.join("proj/b/pom.xml"))
+        .unwrap()
+        .contains("<version>1.0</version></dependency>"));
+    ok(root, &vex_args);
+    let doc: serde_json::Value = serde_json::from_slice(&std::fs::read(&vex).unwrap()).unwrap();
+    assert_eq!(doc["statements"][0]["status"], "not_affected", "{doc}");
+}
+
 /// #429 repair: a deleted owned `.gitattributes` (and nothing else) is
 /// rewritten by `repair` (no download), so the next autocrlf clone keeps the
 /// script and index LF.

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

@@ -479,9 +479,11 @@ pub fn wired_checked(
 }
 
 /// [`wired_checked`], and no declaration of `c`'s g:a is left at a version
-/// that may still read the base: a `${...}` the checkout does not define
-/// or that an inheriting pom or a profile moves (#513, #459). Such a
-/// module builds the unpatched jar, so `vex` must not attest.
+/// that may still read the base: the base itself, as a module or
+/// dependency added since vendoring declares it (#584), or a `${...}` the
+/// checkout does not define or that an inheriting pom or a profile moves
+/// (#513, #459). Such a module builds the unpatched jar, so `vex` must
+/// not attest.
 pub fn attestable_checked(
     read: ReadFn<'_>,
     list: ListFn<'_>,
@@ -507,20 +509,21 @@ pub fn attestable_checked(
         extra_artifacts: &[],
         patched_members: &[],
     };
+    let mut edits = BTreeMap::new();
     let mut uncertain = BTreeSet::new();
     for rel in &reactor.scope {
         reactor.rewrite_declarations(
             rel,
             &patch,
             &sv,
-            &mut BTreeMap::new(),
+            &mut edits,
             &mut Vec::new(),
             &mut BTreeSet::new(),
             &mut BTreeSet::new(),
             &mut uncertain,
         );
     }
-    Ok(uncertain.is_empty())
+    Ok(edits.is_empty() && uncertain.is_empty())
 }
 
 /// `<!-- socket-patch <uuid>:` — the start of `c`'s pin comment.
@@ -5746,6 +5749,47 @@ mod tests {
         assert_eq!(attestable(&after), (true, true));
     }
 
+    /// #584: a module added after vendoring, or a dependency added to an
+    /// existing one, that declares the base literally beats the root's
+    /// pin and builds the unpatched jar: not attestable until re-vendored.
+    #[test]
+    fn a_base_declaration_added_after_vendoring_is_not_attestable() {
+        let files = fs(&[
+            ("pom.xml", ROOT),
+            ("a/pom.xml", &module("a", &dep("1.10.0"))),
+            ("b/pom.xml", &module("b", "")),
+        ]);
+        let (_, after) = vendor(&files);
+        assert_eq!(attestable(&after), (true, true));
+
+        let mut added_module = after.clone();
+        added_module.insert(
+            "pom.xml".into(),
+            text(&after, "pom.xml")
+                .replace(
+                    "<module>b</module>",
+                    "<module>b</module>\n    <module>c</module>",
+                )
+                .into_bytes(),
+        );
+        added_module.insert("c/pom.xml".into(), module("c", &dep("1.10.0")).into_bytes());
+        let mut added_dep = after.clone();
+        added_dep.insert("b/pom.xml".into(), module("b", &dep("1.10.0")).into_bytes());
+        // A module still on an earlier patch's suffix builds that patch's
+        // jar, not this one.
+        let mut earlier_patch = after.clone();
+        earlier_patch.insert(
+            "b/pom.xml".into(),
+            module("b", &dep("1.10.0-socket.deadbeef")).into_bytes(),
+        );
+        for drifted in [added_module, added_dep, earlier_patch] {
+            assert_eq!(attestable(&drifted), (true, false));
+            // Re-vendoring rewrites it and the reactor attests again.
+            let (_, again) = vendor(&drifted);
+            assert_eq!(attestable(&again), (true, true));
+        }
+    }
+
     /// #513: a possibly-matching declaration named through an undefined
     /// `${...}` groupId with its own version beats any pin.
     #[test]