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 66a5db09d..715cbb5b2 100644 --- a/crates/socket-patch-core/src/formats/maven/mod.rs +++ b/crates/socket-patch-core/src/formats/maven/mod.rs @@ -551,6 +551,47 @@ 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) +} + +/// 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/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..64ae25bb2 100644 --- a/crates/socket-patch-core/src/vendor/jvm/apply.rs +++ b/crates/socket-patch-core/src/vendor/jvm/apply.rs @@ -993,12 +993,29 @@ 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) +} + +/// [`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 @@ -1026,7 +1043,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; @@ -1100,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, @@ -1210,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)); @@ -1246,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 @@ -1437,6 +1465,47 @@ 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)); + // 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] 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..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,13 +516,31 @@ 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. -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 +/// 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<'_>, + c: &Coords<'_>, +) -> Result { + Reactor::discover(read, list).map(|reactor| reactor.wires(c)) } -pub fn wired_checked( +/// 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<'_>, @@ -599,21 +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 - .scope - .iter() - .any(|rel| reactor.poms[rel].doc.masked.contains(&sv)) - { - return Ok(false); + if !reactor.wires(c) { + return Ok(Some(format!( + "no dependency or dependencyManagement declaration Maven always reads names \ + {ga}:{sv}" + ))); } let patch = JvmPatch { group_id: c.group_id, @@ -640,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 + ) + })) } /// `