From aecb08c7cdcb0c0406c9e6b1dfd74efc342f9b06 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:45:25 -0600 Subject: [PATCH 1/2] Stop vendored Maven liveness attesting unpatched builds Three cases where vendored JVM (Maven reactor) liveness proved an entry "wired" and let vex attest while the build may still use the unpatched jar. Hosted mode and VEX discovery already refuse all three. J2, executable classifier copies: when the project also declares an executable classifier copy of the patched GA (tests, a native build), the reactor planner only warned `classifier_declared` and moved on, so `attestable_checked` passed. The classifier-less pin doesn't retarget the copy and the vendored tree serves only the main jar, so Maven fetches the public release and its code runs. A used (non-managed) executable classifier declaration whose version isn't the suffixed one now marks the reactor uncertain, so the entry isn't attestable. `sources` and `javadoc` still only warn. The executable-classifier rule now lives in `formats::maven::is_executable_classifier`, shared with the hosted rewriter and discovery. J8, unscoped "wired" check: `maven_reactor::wired_checked` was a substring match for the suffixed version over every reactor pom's comment-masked text. A suffixed version left only in a build plugin's dependencies, a profile, an unused property, another artifact's or a classifier copy's declaration kept the entry live. It now requires a ``/`` declaration outside `` (Doc::declarations already excludes ``/``) naming the GA's main jar at the suffixed version, literally or through a property the checkout defines. That covers `vex`/`attestable_checked`, `vendor --check` (`check_entry`) and the plan's `refuse_unwired`. The old substring check is kept as `references_checked` and used only by the `scan --prune` GC (`entry_references`): the maven.config tail also serves plugin resolution, so a tree a `` plugin or a profile still names must not be reverted out from under the build. J15, fail-open on read errors: `entry_wired` was `entry_wired_checked(..).unwrap_or(true)`, so an unreadable or unparseable pom made `Discovery::vendor_entry_live` (vendor's kept-patch rule, scan's takeover classifier and vex's ledger liveness) report a JVM entry as live. It now fails closed (`false`). The GC doesn't use it: it asks `entry_references`, which still answers "keep" on a read error, so nothing is deleted on an unknown answer. Tests (each fails on origin/main): - maven_reactor: an_executable_classifier_copy_is_not_attestable - maven_reactor: a_suffixed_version_outside_the_declarations_is_not_wired - apply: an_unreadable_pom_is_not_live_but_is_kept (also pins that entry_references / vendor_entry_in_use still keep the tree) Co-Authored-By: Claude Opus 5.5 --- .../src/formats/maven/mod.rs | 13 ++ .../src/patch/redirect/mod.rs | 2 +- .../socket-patch-core/src/vendor/jvm/apply.rs | 44 +++- .../src/vendor/jvm/maven_reactor.rs | 202 +++++++++++++++++- .../src/vex/discover/maven.rs | 7 +- 5 files changed, 244 insertions(+), 24 deletions(-) diff --git a/crates/socket-patch-core/src/formats/maven/mod.rs b/crates/socket-patch-core/src/formats/maven/mod.rs index 66a5db09d..5924a0185 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -551,6 +551,19 @@ pub(crate) fn may_shadow( && may_name(dep_artifact, artifact) } +/// The classifiers whose jars never run on a classpath: `sources` and +/// `javadoc`. Every other classifier copy of a patched GA (tests, a native +/// or JDK-specific build) carries code, so it must resolve the patched +/// release too. The hosted rewriter, VEX discovery and the vendored +/// reactor's attestation share this rule. +pub(crate) const NON_EXECUTABLE_CLASSIFIERS: &[&str] = &["sources", "javadoc"]; + +/// Whether a `` copy of a GA carries code onto a classpath +/// (see [`NON_EXECUTABLE_CLASSIFIERS`]). +pub(crate) fn is_executable_classifier(classifier: &str) -> bool { + !NON_EXECUTABLE_CLASSIFIERS.contains(&classifier) +} + fn is_property_name(name: &str) -> bool { !name.is_empty() && name diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0c778ac74..bb0e0f24e 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -8801,7 +8801,7 @@ fn rewrite_maven_pom( .is_some_and(|(base, _)| base == dep.version); if names_release { let classifier = variant.classifier.as_deref().unwrap_or_default(); - let executable = !matches!(classifier, "sources" | "javadoc"); + let executable = crate::formats::maven::is_executable_classifier(classifier); executable_variant |= executable; result.warnings.push(RewriteWarning { code: "redirect_maven_classifier_unsupported".into(), diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index 0fffc3a24..5e9733fce 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -993,12 +993,15 @@ pub fn recorded_tree_files(entry: &VendorEntry) -> BTreeSet { .collect() } -/// Whether the project still wires the JVM `entry` for `vex` (see -/// [`entry_wired_checked`]). +/// Whether the project provably still wires the JVM `entry` (see +/// [`entry_wired_checked`]): the liveness every attesting reader of the +/// vendor ledger applies. Fails closed: a file that cannot be read or +/// parsed proves nothing wired, so the answer is `false`. Callers that +/// must not delete on an unknown answer (the `scan --prune` GC) ask +/// [`entry_references`] instead, which keeps the tree then; `vex` uses the +/// checked variant to report the diagnostic. pub fn entry_wired(root: &Path, entry: &VendorEntry) -> bool { - // Failure to read does not prove that deleting a tree is safe. Attestation - // uses the checked variant and reports the diagnostic instead. - entry_wired_checked(root, entry).unwrap_or(true) + entry_wired_checked(root, entry).unwrap_or(false) } /// Whether the project still references the JVM `entry`'s tree (its @@ -1026,7 +1029,7 @@ pub fn entry_references(root: &Path, entry: &VendorEntry) -> bool { scala_cli::wired_checked(&read, &c).unwrap_or(true) } else { let (maven, gradle) = sides(&entry.wiring); - (maven && maven_reactor::wired_checked(&read, &list, &c).unwrap_or(true)) + (maven && maven_reactor::references_checked(&read, &list, &c).unwrap_or(true)) || (gradle && gradle::references_checked(&read, &c).unwrap_or(true)) }; // `read` answers `None` for an unreadable file as for a missing one; @@ -1437,6 +1440,35 @@ mod tests { } } + /// A reactor pom the reader cannot read proves nothing either way: + /// liveness for attestation (`vex`, `vendor`'s kept-patch rule, `scan`'s + /// takeover classifier) answers "not live", while the `scan --prune` + /// GC's [`entry_references`] still keeps the tree. + #[tokio::test] + async fn an_unreadable_pom_is_not_live_but_is_kept() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + std::fs::write( + root.join("pom.xml"), + "4.0.0t\ + root1pom\ + m\ + ga1-socket.1d3c1fd2\ + ", + ) + .unwrap(); + // The module's pom is a directory: unreadable, not missing. + std::fs::create_dir_all(root.join("m/pom.xml")).unwrap(); + let e = entry(vec![record(POM_FRAGMENT_KIND, "pom.xml")]); + assert!(is_jvm_entry(&e)); + assert!(entry_wired_checked(root, &e).is_err()); + assert!(!entry_wired(root, &e)); + let discovery = crate::vex::discover::Discovery::default(); + assert!(!discovery.vendor_entry_live(root, &e).await); + assert!(entry_references(root, &e)); + assert_eq!(discovery.vendor_entry_in_use(root, &e).await, Some(true)); + } + #[test] fn recorded_paths_are_whitelisted_per_kind() { let c = coords(); diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index 42a853fff..f5311bb5f 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -516,16 +516,37 @@ pub fn unplan( } } -/// Whether a pom of the reactor still declares `c`'s suffixed version -/// (outside comments): the liveness proof `vex` needs for this layout. +/// [`wired_checked`], `false` when the reactor cannot be read. pub fn wired(read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>) -> bool { wired_checked(read, list, c).unwrap_or(false) } +/// Whether a declaration Maven always reads wires `c`: a `` +/// or `` entry of a reactor pom (outside +/// ``, `` and ``) naming `c`'s main jar at its +/// suffixed version — literally or through a property the checkout defines +/// — or the root's pin. The liveness proof `vex` needs for this layout: +/// the suffixed version left only in a build plugin's dependencies, a +/// profile, an unused property, a comment or a classifier copy does not +/// put the patched jar on the project's classpath. pub fn wired_checked( read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>, +) -> Result { + Reactor::discover(read, list).map(|reactor| reactor.wires(c)) +} + +/// Whether a pom of the reactor still names `c`'s suffixed version +/// anywhere outside comments — a declaration, a build plugin's dependency, +/// a profile or a property: anything that may still resolve from the tree +/// (the maven.config tail serves plugin resolution too), so what the +/// `scan --prune` GC must not revert from under the build. Broader than +/// [`wired_checked`], which is what attests. +pub fn references_checked( + read: ReadFn<'_>, + list: ListFn<'_>, + c: &Coords<'_>, ) -> Result { let sv = c.suffixed_version(); Reactor::discover(read, list).map(|reactor| { @@ -608,11 +629,7 @@ pub fn attestable_checked( ) -> Result { let sv = c.suffixed_version(); let reactor = Reactor::discover(read, list)?; - if !reactor - .scope - .iter() - .any(|rel| reactor.poms[rel].doc.masked.contains(&sv)) - { + if !reactor.wires(c) { return Ok(false); } let patch = JvmPatch { @@ -1724,6 +1741,29 @@ impl Reactor { roots } + /// Whether a pom of the scope declares `c`'s main jar at its suffixed + /// version where Maven always reads it (see [`wired_checked`]). + fn wires(&self, c: &Coords<'_>) -> bool { + let sv = c.suffixed_version(); + self.scope.iter().any(|rel| { + let doc = &self.poms[rel].doc; + let resolve = |v: &str| self.interpolate(rel, v, 0); + let value = |dep: usize, name: &str| doc.coordinate(dep, name, &resolve); + doc.declarations().into_iter().any(|dep| { + !doc.section_of(dep).starts_with("profile:") + && value(dep, "groupId").as_deref() == Some(c.group_id) + && value(dep, "artifactId").as_deref() == Some(c.artifact_id) + && doc + .child_text(dep, "type") + .is_none_or(|t| t.is_empty() || t == "jar") + && doc + .child_text(dep, "classifier") + .is_none_or(|t| t.is_empty()) + && value(dep, "version").as_deref() == Some(sv.as_str()) + }) + }) + } + /// The g:a declarations of `rel`, with `${...}` coordinates /// interpolated in its context. fn keyed_declarations(&self, rel: &str, g: &str, a: &str) -> Vec<(usize, String)> { @@ -1881,14 +1921,31 @@ impl Reactor { continue; } let at = format!("{rel}:{}", doc.line_of(doc.nodes[dep].start)); - if doc - .child_text(dep, "classifier") - .is_some_and(|c| !c.is_empty()) - { + if let Some(classifier) = doc.child_text(dep, "classifier").filter(|c| !c.is_empty()) { warnings.push(degraded( "classifier_declared", format!("{at}: a classifier variant of {ga} bypasses the pin"), )); + // The classifier-less pin does not retarget the copy and the + // tree serves only the main jar, so Maven fetches the public + // release: an executable copy (tests, a native build) keeps + // the unpatched code on the classpath, which hosted mode and + // VEX discovery refuse to attest too. Only a used copy + // counts; a managed entry alone puts nothing on a classpath. + let resolved = || { + let v = doc.child_text(dep, "version")?; + if v.contains("${") { + self.interpolate(rel, &v, 0) + } else { + Some(v) + } + }; + if !doc.section_of(dep).ends_with("dependencyManagement") + && crate::formats::maven::is_executable_classifier(&classifier) + && resolved().as_deref() != Some(sv) + { + uncertain.insert(at); + } continue; } let root = self.local_root(rel); @@ -5908,6 +5965,129 @@ mod tests { assert_eq!(attestable(&after), (true, true)); } + /// A `` copy of the GA is its own coordinate: the + /// classifier-less pin does not retarget it and the vendored tree does + /// not serve it, so Maven fetches the public release. An executable + /// one (tests, a native build) keeps the unpatched code on the + /// classpath: not attestable, as in hosted mode and VEX discovery. + /// `sources` / `javadoc` jars never run, so they only warn. + #[test] + fn an_executable_classifier_copy_is_not_attestable() { + let classified = |classifier: &str, version: &str| { + format!( + "org.apache.commons\ + commons-text{version}\ + {classifier}" + ) + }; + for (classifier, version, attests) in [ + ("tests", "1.10.0", false), + ("linux-x86_64", "1.10.0", false), + // Not the base release, but still not the patched one. + ("tests", "1.9", false), + // Versioned by a profile's property: the base once it is active. + ("tests", "${ct.tests}", false), + ("sources", "1.10.0", true), + ("javadoc", "1.10.0", true), + ] { + let b = if version.starts_with("${") { + with_profile( + &module("b", &classified(classifier, version)), + "t", + "1.10.0", + ) + } else { + module("b", &classified(classifier, version)) + }; + let files = fs(&[ + ("pom.xml", ROOT), + ("a/pom.xml", &module("a", &dep("1.10.0"))), + ("b/pom.xml", &b), + ]); + let (plan, after) = vendor(&files); + assert_eq!(reasons(&plan), ["classifier_declared"], "{classifier}"); + assert_eq!(text(&after, "a/pom.xml"), module("a", &dep(SV))); + assert_eq!( + attestable(&after), + (true, attests), + "{classifier} {version}" + ); + } + // A classifier copy added inside a profile after vendoring. + let files = fs(&[ + ("pom.xml", ROOT), + ("a/pom.xml", &module("a", &dep("1.10.0"))), + ("b/pom.xml", &module("b", "")), + ]); + let (_, mut after) = vendor(&files); + assert_eq!(attestable(&after), (true, true)); + let b = module("b", "").replace( + " ", + &format!( + " it{}\ + \n ", + classified("tests", "1.10.0") + ), + ); + after.insert("b/pom.xml".into(), b.into_bytes()); + assert_eq!(attestable(&after), (true, false)); + } + + /// Only a declaration Maven always reads wires the patch: a + /// `` / `` entry of the GA's main + /// jar outside `` whose version resolves to the suffixed + /// one. The suffixed version left in a build plugin's dependencies, a + /// profile, an unused property or another artifact's declaration + /// builds the base for the project's own classpath. + #[test] + fn a_suffixed_version_outside_the_declarations_is_not_wired() { + let plugin = format!( + "pp\ + {}", + dep(SV) + ); + let profile = format!( + "p{}", + dep(SV) + ); + let property = format!("{SV}"); + let other = dep(SV).replace("commons-text", "commons-lang3"); + let classifier = + dep(SV).replace("", "sources"); + for extra in [plugin, profile, property, other.clone(), classifier.clone()] { + let a = if extra.starts_with("") { + module("a", &extra) + } else { + module("a", "").replace(" ", &format!(" {extra}\n ")) + }; + let files = fs(&[ + ("pom.xml", ROOT), + ("a/pom.xml", &a), + ("b/pom.xml", &module("b", "")), + ]); + assert_eq!(attestable(&files), (false, false), "{extra}"); + } + // A property the declaration reads, and the root's managed pin, do. + let via_property = module("a", &dep("${ct.version}")).replace( + " ", + &format!(" {SV}\n "), + ); + let files = fs(&[ + ("pom.xml", ROOT), + ("a/pom.xml", &via_property), + ("b/pom.xml", &module("b", "")), + ]); + assert_eq!(attestable(&files), (true, true)); + 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!(text(&after, "pom.xml").contains(SV)); + 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. diff --git a/crates/socket-patch-core/src/vex/discover/maven.rs b/crates/socket-patch-core/src/vex/discover/maven.rs index cc4c69fe8..66dd53e10 100644 --- a/crates/socket-patch-core/src/vex/discover/maven.rs +++ b/crates/socket-patch-core/src/vex/discover/maven.rs @@ -712,11 +712,6 @@ fn group_by_ga(deps: &[PomDep]) -> BTreeMap<(String, String), GaVersions> { map } -/// The classifiers whose jars never run on a classpath. Every other -/// classifier copy of a pinned GA (tests, a native or JDK-specific build) -/// executes, so it must resolve the patched release too. -const NON_EXECUTABLE_CLASSIFIERS: &[&str] = &["sources", "javadoc"]; - /// The first live executable classifier copy of `group:artifact` that does /// not resolve `pinned`, with the version it resolves (`None` when this pom /// gives it none). Its version is its own literal, else a @@ -734,7 +729,7 @@ fn unpatched_executable_classifier<'a>( .filter(|d| !d.managed) .filter_map(|d| { let classifier = d.classifier.as_deref()?; - (!NON_EXECUTABLE_CLASSIFIERS.contains(&classifier)).then_some((d, classifier)) + crate::formats::maven::is_executable_classifier(classifier).then_some((d, classifier)) }) .find_map(|(d, classifier)| { let version = d.version.as_deref().or_else(|| { From e0c351ffd84cfc39e29e527a442af25d223d55d9 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 14:56:17 -0600 Subject: [PATCH 2/2] Address review findings on vendored Maven liveness Fixes all nine /code-review high findings on #2006. 1. A typed copy of the GA (`test-jar`, which is the `tests` jar, or `ejb`, `ejb-client`, `maven-plugin`, an unknown type) was skipped with a bare `continue`, so it still attested. A new shared rule, `formats::maven::is_executable_variant(type, classifier)`, maps types to the classifier they imply and treats every type except `pom`, `java-source` and `javadoc` as executable. The reactor now warns `classifier_declared` for such a copy and marks it uncertain unless every reader resolves it to the suffixed version. 2. With `entry_wired` failing closed, `scan`'s overlap takeover classifier read an unreadable pom, or vendored wiring left only in a plugin or profile, as a dead vendored claim. It then emitted `redirect_supersedes_vendored` ("remove ") while the GC still kept the entry. New tri-state: `apply::entry_liveness`, `Discovery::vendor_entry_liveness` and `LedgerLiveness:: vendor_entry_state`. It returns `None` when the claim is undecided (read error, or still referenced without attestable wiring), and the classifier stays silent on `None`. 3. A classifier copy's version now goes through the same override check as the main jar's (`version_moved`, generalized from `version_override`). An inheriting pom or a profile that moves the property off the suffixed version makes it uncertain. 4. A version-less declaration now reads its version from the nearest local `` on its chain, interpolated as the declaring pom reads it. Anything other than the suffixed version, including no local management (a local root left unpinned), is uncertain. One pinned root no longer proves an unpinned sibling root. 5. `vendor --check` now agrees with `vex`. `check_entry` returns `Ok(Some(why))` when the entry is intact and wired but not attestable. `vendor --check` keeps the entry `verified` and adds the new run warning `vendor_jvm_unattested` naming the declarations (documented in CLI_CONTRACT.md). It is a warning rather than a failure because the contract's unattested-references rule says liveness gates keep such wiring live, and re-vendoring cannot clear it. 6. `Reactor::wires` now interpolates each declaration in the declaring pom and in every pom that inherits it, as Maven interpolates after inheritance. A parent's managed `${ct.version}` that each module defines as the pin now counts as wired again, so `refuse_unwired` no longer refuses that plan. 7. The classifier version check uses `Doc::coordinate`. 8. The main-jar predicate is `formats::maven::is_main_jar`, used through `Doc::is_main_jar` by `wires`, `rewrite_declarations`, `warn_unresolved_coordinates` and `jar_declaration`. 9. Removed the uncalled `maven_reactor::wired`. Tests (each fails with its fix neutralized): - plugin_dependencies_exclusions_and_other_types_are_untouched: updated. It pinned the silent `test-jar` pass-through; executable types now warn and don't attest, while `pom` and `java-source` still pass silently. - a_classifier_copy_a_reader_moves_back_to_the_base_is_not_attestable - a_version_less_declaration_under_an_unpinned_root_is_not_attestable - a_parent_declaration_its_modules_resolve_to_the_pin_is_wired - vendor_check_reports_an_unattestable_entry - maven_takeover_stays_silent_on_an_undecided_vendored_claim (CLI) - an_unreadable_pom_is_not_live_but_is_kept also covers the tri-state Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 10 +- .../socket-patch-cli/src/commands/scan/mod.rs | 127 ++++- .../socket-patch-cli/src/commands/vendor.rs | 13 +- .../src/formats/maven/mod.rs | 28 ++ .../socket-patch-core/src/vendor/jvm/apply.rs | 41 +- .../src/vendor/jvm/maven_reactor.rs | 436 ++++++++++++++---- .../socket-patch-core/src/vex/discover/mod.rs | 19 + 7 files changed, 579 insertions(+), 95 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index a3ed909d6..a7923e1f4 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -2280,7 +2280,14 @@ reference to `.socket/vendor///` that no ledger entry owns (the artifact-level event repair emits: `uuid` plus `details.{ecosystem,path}`, no purl), so a checkout whose ledger was ignored or dropped from the commit no longer passes with nothing to check. Offline upstream metadata is reported as the run warning -`vendor_jvm_upstream_unverified`. The check never starts an API client or writes +`vendor_jvm_upstream_unverified`. A Maven reactor entry that is intact and wired +but that `vex` does not attest — a declaration may still build the base release: +an executable classifier or type copy of the patched GA (`tests`, `test-jar`, a +native build), a version a property, a profile or an unpinned local root may +move, or a base declaration added since vendoring that the re-plan cannot take +— stays `verified` and adds the run warning `vendor_jvm_unattested`, whose +detail names the entry and the declarations. Like the other unattested +references, re-vendoring cannot clear it, so it is not drift. The check never starts an API client or writes lock/recovery files. `--check` conflicts with `--revert`. `vendor --check --local-repo ` additionally checks existing suffixed Maven @@ -2634,6 +2641,7 @@ with `reason: : `: | `vendor_jvm_degraded` | applied, VEX withheld | `gradle_unscanned_build_logic`, `unwired_build_logic` (a nonliteral included build), `settings_plugins_unwired`, `classifier_unpatched_copy` (a classifier jar carries an unpatched copy of a patched member), `verification_parent_chain_unhandled`, `conflicting_managed_version` / `management_unresolved` (Maven reactor: an imported BOM or an out-of-checkout parent manages the artifact at another version, or its POM is unavailable; that local root is not pinned) | | `vendor_jvm_note` | applied, informational | `range_declared` (a range, prefix or rich selector lists versions from the derived `maven-metadata.xml`), `ide_sources_unavailable` | | `vendor_jvm_upstream_unverified` | warning | Upstream metadata was taken offline and not authenticated against registry checksums. | +| `vendor_jvm_unattested` | warning (`vendor --check`) | A Maven reactor entry is intact and wired, but a declaration may still build the base release (an executable classifier or type copy, a version a property, profile or unpinned local root may move), so `vex` does not attest it. The entry stays `verified`; the detail names the purl and the declarations (`:`). | The Maven half of a root (single-module, reactor or mixed) also reports `vendor_jvm_degraded` with `reason: maven_f_outside_root` (Maven 3.9.2–3.9.8 diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index dc2ed2974..a34f021ad 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -1070,15 +1070,18 @@ pub(super) async fn classify_overlap_takeover_with( Some(uuid) => liveness.redirect_record(&purl, uuid).await, None => discovery.wires_package(&purl, WiringMode::Hosted), }; + // `None`: undecided (a JVM pom that cannot be read, or a tree still + // referenced without attestable wiring) — neither live nor dead. let vendored_live = match vendor_by_purl.get(&PurlKey::new(&purl)) { - Some(entry) => liveness.vendor_entry(entry).await, - None => false, + Some(entry) => liveness.vendor_entry_state(entry).await, + None => Some(false), }; match (hosted_live, vendored_live) { - (true, false) => out.redirect.push(purl), - (false, true) => out.vendored.push(purl), - // Both (a half-migrated lock naming both) or neither (no rewire / - // unreadable) does not prove a single direction — stay silent. + (true, Some(false)) => out.redirect.push(purl), + (false, Some(true)) => out.vendored.push(purl), + // Both (a half-migrated lock naming both), neither (no rewire) + // or an undecided vendored side (unreadable) does not prove a + // single direction — stay silent. _ => {} } } @@ -4671,6 +4674,118 @@ mod tests { ); } + // ---- maven: an undecided vendored claim is not a dead one ------------- + + const MAVEN_PURL: &str = "pkg:maven/org.slf4j/slf4j-api@1.7.36"; + /// The vendored patch: another uuid than the hosted pin's, so its + /// suffixed version tells the two wirings apart. + const MAVEN_VENDORED_UUID: &str = "5c0ffee0-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; + + fn maven_dep(version: &str) -> String { + format!( + "org.slf4jslf4j-api\ + {version}" + ) + } + + /// A root pom pinning slf4j-api to the hosted [`TAKEOVER_UUID`] patch, + /// with `extra` (a `` section) inside the project. + async fn write_maven_hosted_pom(root: &Path, extra: &str) { + let pom = format!( + "\n\n \ + 4.0.0\n com.example\n \ + app\n 1.0.0\n \ + \n {}\n \n \n \n \ + socket-patch-{TAKEOVER_UUID}\n \ + https://patch.socket.dev/patch-registry/maven/11111111-2222-4333-8444-555555555555/{TAKEOVER_UUID}/maven2\n \ + \n true\n \ + fail\n \n \n \ + \n {extra}\n\n", + maven_dep(&format!("1.7.36-socket.{}", &TAKEOVER_UUID[..8])), + ); + tokio::fs::write(root.join("pom.xml"), pom).await.unwrap(); + } + + /// A vendored ledger whose one entry is a Maven reactor vendoring of + /// slf4j-api at [`MAVEN_VENDORED_UUID`]. + async fn write_maven_vendor_ledger(root: &Path) { + let sv = format!("1.7.36-socket.{}", &MAVEN_VENDORED_UUID[..8]); + let state = serde_json::json!({ + "version": 1, + "entries": { + MAVEN_PURL: { + "ecosystem": "maven", + "basePurl": MAVEN_PURL, + "uuid": MAVEN_VENDORED_UUID, + "artifact": { + "path": format!( + ".socket/vendor/maven2/org/slf4j/slf4j-api/{sv}/slf4j-api-{sv}.jar" + ), + }, + "wiring": [{ + "file": "pom.xml", + "kind": "maven_pom_fragment", + "action": "added", + }], + }, + }, + }); + let dir = root.join(".socket/vendor"); + tokio::fs::create_dir_all(&dir).await.unwrap(); + tokio::fs::write( + dir.join("state.json"), + serde_json::to_string_pretty(&state).unwrap(), + ) + .await + .unwrap(); + } + + /// A vendored Maven claim the reader cannot decide — a project file it + /// cannot read, or the vendored version left only where the build may + /// still resolve it from the tree (a build plugin's dependencies) — + /// is neither live nor dead: the `scan --prune` GC keeps it, so the + /// takeover classifier must not report it superseded (and tell the + /// user to remove it). Only a claim nothing references any more is. + #[tokio::test] + async fn maven_takeover_stays_silent_on_an_undecided_vendored_claim() { + let vendored = format!("1.7.36-socket.{}", &MAVEN_VENDORED_UUID[..8]); + let plugin = format!( + "pp\ + {}", + maven_dep(&vendored) + ); + // Control: nothing references the vendored version — superseded. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_maven_vendor_ledger(root).await; + write_maven_hosted_pom(root, "").await; + let takeover = classify_overlap_takeover(&common_at(root), root).await; + assert_eq!( + takeover.redirect, + vec![MAVEN_PURL.to_string()], + "{takeover:?}" + ); + + // Left only in a build plugin's dependencies. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_maven_vendor_ledger(root).await; + write_maven_hosted_pom(root, &plugin).await; + let takeover = classify_overlap_takeover(&common_at(root), root).await; + assert_eq!(takeover, OverlapTakeover::default()); + + // A Maven config the reader cannot read (a directory). + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_maven_vendor_ledger(root).await; + write_maven_hosted_pom(root, "").await; + tokio::fs::create_dir_all(root.join(".mvn/maven.config")) + .await + .unwrap(); + let takeover = classify_overlap_takeover(&common_at(root), root).await; + assert_eq!(takeover, OverlapTakeover::default()); + } + // ---- takeover DIRECTION follows the live lock, not the command --------- /// Like [`write_vendor_ledger`] but each entry records wiring the diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 873e394fd..7a6686b12 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1334,7 +1334,18 @@ async fn run_check(args: &VendorArgs) -> i32 { None => Some("patch record missing; restore the manifest or vendor ledger".to_string()), }; if failure.is_none() && vendor::jvm::apply::is_jvm_entry(entry) { - failure = vendor::jvm::apply::check_entry(root, entry, local_repo.as_deref()).err(); + match vendor::jvm::apply::check_entry(root, entry, local_repo.as_deref()) { + Ok(None) => {} + // Intact and wired, but `vex` will not attest it: a + // declaration may still build the base. Re-vendoring cannot + // clear that, so it warns rather than fails (the unattested + // references rule). + Ok(Some(why)) => env.warnings.push(RunWarning { + code: "vendor_jvm_unattested".into(), + detail: format!("{key}: {why}; vex does not attest it"), + }), + Err(e) => failure = Some(e), + } } // The npm check names the exact unwired lock entry, so it runs // before the generic liveness rule below. diff --git a/crates/socket-patch-core/src/formats/maven/mod.rs b/crates/socket-patch-core/src/formats/maven/mod.rs index 5924a0185..715cbb5b2 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -564,6 +564,34 @@ pub(crate) fn is_executable_classifier(classifier: &str) -> bool { !NON_EXECUTABLE_CLASSIFIERS.contains(&classifier) } +/// Whether a declaration of a GA's main jar: untyped or `jar`-typed, with +/// no classifier. +pub(crate) fn is_main_jar(type_text: Option<&str>, classifier: Option<&str>) -> bool { + type_text.is_none_or(|t| t.is_empty() || t == "jar") && classifier.is_none_or(str::is_empty) +} + +/// Whether a declaration of a GA that is not its main jar +/// ([`is_main_jar`]) is a copy that carries code onto a classpath. Maven +/// derives a classifier from some types (`test-jar` is the `tests` jar, +/// `ejb-client` the `client` one, `java-source` / `javadoc` the +/// non-executable ones), so a type counts like the classifier it implies. +/// Of the rest only `pom` holds no code; every other type (`ejb`, +/// `maven-plugin`, `bundle`, an unknown or `${...}` one) is executable. +pub(crate) fn is_executable_variant(type_text: Option<&str>, classifier: Option<&str>) -> bool { + let type_text = type_text.filter(|t| !t.is_empty()).unwrap_or("jar"); + let implied = match type_text { + "test-jar" => Some("tests"), + "ejb-client" => Some("client"), + "java-source" => Some("sources"), + "javadoc" => Some("javadoc"), + _ => None, + }; + match classifier.filter(|c| !c.is_empty()).or(implied) { + Some(c) => is_executable_classifier(c), + None => type_text != "pom", + } +} + fn is_property_name(name: &str) -> bool { !name.is_empty() && name diff --git a/crates/socket-patch-core/src/vendor/jvm/apply.rs b/crates/socket-patch-core/src/vendor/jvm/apply.rs index 5e9733fce..64ae25bb2 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -1004,6 +1004,20 @@ pub fn entry_wired(root: &Path, entry: &VendorEntry) -> bool { entry_wired_checked(root, entry).unwrap_or(false) } +/// [`entry_wired`] for a reader that must tell "dead" from "unknown": +/// `Some(true)` wired ([`entry_wired_checked`]); `Some(false)` the project +/// no longer references the tree at all ([`entry_references`] is +/// `false`); `None` otherwise — a file could not be read or parsed, or the +/// tree is still referenced (a build plugin, a profile, a declaration that +/// may still build the base) without wiring that `vex` attests. +pub fn entry_liveness(root: &Path, entry: &VendorEntry) -> Option { + match entry_wired_checked(root, entry) { + Ok(true) => Some(true), + _ if entry_references(root, entry) => None, + _ => Some(false), + } +} + /// Whether the project still references the JVM `entry`'s tree (its /// suffixed version in a reactor pom; its index rows plus the root apply /// line for Gradle): what a revert must not pull from under a peer. @@ -1103,11 +1117,17 @@ fn local_external_poms( } /// Verify every recorded file plus the effective wiring, without writes or network I/O. +/// +/// `Ok(Some(why))`: the entry is intact and wired, but a Maven declaration +/// may still build the base, so `vex` does not attest it +/// ([`maven_reactor::unattestable_checked`]). Like the other unattested +/// references, re-vendoring cannot clear that, so it is not drift: +/// `vendor --check` verifies the entry and warns with `why`. pub fn check_entry( root: &Path, entry: &VendorEntry, local_repo: Option<&Path>, -) -> Result<(), String> { +) -> Result, String> { let (g, a, v) = entry_gav(entry)?; let c = Coords { group_id: &g, @@ -1213,6 +1233,11 @@ pub fn check_entry( if !referenced { return Err("vendored artifact is no longer wired into the build".into()); } + let unattested = if maven && !(sbt_entry || scala_cli_entry) { + maven_reactor::unattestable_checked(&read, &list, &c).map_err(|e| e.detail)? + } else { + None + }; let mut tree_dirs = vec![plan.tree_dir.clone()]; if maven && gradle { tree_dirs.push(gradle::tree_dir(&c)); @@ -1249,7 +1274,7 @@ pub fn check_entry( if let Some(rel) = reader.escaped() { return Err(outside_root_detail(&rel)); } - Ok(()) + Ok(unattested) } /// The vendored jar of the JVM `entry` for `uuid`, project-relative: the @@ -1467,6 +1492,18 @@ mod tests { assert!(!discovery.vendor_entry_live(root, &e).await); assert!(entry_references(root, &e)); assert_eq!(discovery.vendor_entry_in_use(root, &e).await, Some(true)); + // Undecided, not dead: `scan`'s takeover classifier stays silent. + assert_eq!(entry_liveness(root, &e), None); + assert_eq!(discovery.vendor_entry_liveness(root, &e).await, None); + // Readable and referenced nowhere: dead. + std::fs::remove_dir(root.join("m/pom.xml")).unwrap(); + std::fs::write( + root.join("pom.xml"), + "4.0.0t\ + root1", + ) + .unwrap(); + assert_eq!(entry_liveness(root, &e), Some(false)); } #[test] diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs index f5311bb5f..1e427198d 100644 --- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs +++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs @@ -516,19 +516,16 @@ pub fn unplan( } } -/// [`wired_checked`], `false` when the reactor cannot be read. -pub fn wired(read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>) -> bool { - wired_checked(read, list, c).unwrap_or(false) -} - /// Whether a declaration Maven always reads wires `c`: a `` /// or `` entry of a reactor pom (outside /// ``, `` and ``) naming `c`'s main jar at its /// suffixed version — literally or through a property the checkout defines -/// — or the root's pin. The liveness proof `vex` needs for this layout: -/// the suffixed version left only in a build plugin's dependencies, a -/// profile, an unused property, a comment or a classifier copy does not -/// put the patched jar on the project's classpath. +/// for the pom that declares it or one that inherits it, as Maven +/// interpolates after inheritance — or the root's pin. The liveness proof +/// `vex` needs for this layout: the suffixed version left only in a build +/// plugin's dependencies, a profile, an unused property, a comment or a +/// classifier copy does not put the patched jar on the project's +/// classpath. pub fn wired_checked( read: ReadFn<'_>, list: ListFn<'_>, @@ -620,17 +617,32 @@ pub fn refuse_unwired( /// 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. +/// (#513, #459), a version-less one whose local root is left unpinned, or +/// an executable classifier or type copy of the GA (`tests`, `test-jar`, a +/// native build) the pin does not retarget. Such a module builds the +/// unpatched jar, so `vex` must not attest. pub fn attestable_checked( read: ReadFn<'_>, list: ListFn<'_>, c: &Coords<'_>, ) -> Result { + Ok(unattestable_checked(read, list, c)?.is_none()) +} + +/// Why [`attestable_checked`] fails for `c`, `None` when it passes. +pub fn unattestable_checked( + read: ReadFn<'_>, + list: ListFn<'_>, + c: &Coords<'_>, +) -> Result, JvmRefusal> { let sv = c.suffixed_version(); + let ga = format!("{}:{}", c.group_id, c.artifact_id); let reactor = Reactor::discover(read, list)?; if !reactor.wires(c) { - return Ok(false); + return Ok(Some(format!( + "no dependency or dependencyManagement declaration Maven always reads names \ + {ga}:{sv}" + ))); } let patch = JvmPatch { group_id: c.group_id, @@ -657,7 +669,17 @@ pub fn attestable_checked( &mut uncertain, ); } - Ok(edits.is_empty() && uncertain.is_empty()) + let mut at: Vec = edits.into_keys().collect(); + at.extend(uncertain); + Ok((!at.is_empty()).then(|| { + format!( + "{} may still build {ga}:{} rather than {sv}: a base version declared since \ + vendoring, a version a property, profile or unpinned root may move, or an \ + executable classifier or type copy the pin does not retarget", + at.join(", "), + c.version + ) + })) } /// `