diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index dc2ed2974..168c75e75 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -981,8 +981,10 @@ fn overlap_from_states( /// them to right now — the truth source for takeover direction. /// /// Both directions are proved by lockfile discovery with the same liveness -/// rules `vex` gates attestations on (core `Discovery::redirect_record_live` -/// / `Discovery::vendor_entry_live`). `redirect` holds the overlap PURLs the +/// rules `vex` gates attestations on (core `Discovery::redirect_record_verdict` +/// / `Discovery::vendor_entry_live`); a hosted record on a patch server this +/// run was not given (`RecordLiveness::Undecided`) counts as wired, since the +/// wiring is still there. `redirect` holds the overlap PURLs the /// lock routes to the hosted patch server (the vendored ledger entry is /// stale); `vendored` holds those it routes to a committed /// `.socket/vendor//` artifact. @@ -1067,7 +1069,7 @@ pub(super) async fn classify_overlap_takeover_with( let mut liveness = LedgerLiveness::new(cwd, discovery, None); for purl in overlap { let hosted_live = match redirect_uuid_by_purl.get(&PurlKey::new(&purl)) { - Some(uuid) => liveness.redirect_record(&purl, uuid).await, + Some(uuid) => liveness.redirect_record_retained(&purl, uuid).await, None => discovery.wires_package(&purl, WiringMode::Hosted), }; let vendored_live = match vendor_by_purl.get(&PurlKey::new(&purl)) { @@ -1391,8 +1393,9 @@ async fn gradle_scan( /// pin names the purl (`redirect_state`, the lockfiles' hosted state — see /// [`crate::commands::hosted_state_from_pins`]) AND lockfile discovery /// proves the current lockfile still routes it to that hosted patch — core -/// `Discovery::redirect_record_live`, the same liveness rule `vex` gates -/// hosted attestations on. +/// `Discovery::redirect_record_verdict`, the same liveness rule `vex` gates +/// hosted attestations on, keeping a record on a patch server this run was +/// not given (`RecordLiveness::Undecided`): its wiring is still retained. /// /// Deliberately NOT routed through [`classify_overlap_takeover`]: that /// classifier keys on purls present in BOTH ledgers, so hosted-only wiring @@ -1435,7 +1438,7 @@ pub(super) async fn hosted_wiring_retained_purls( let mut liveness = LedgerLiveness::new(cwd, &discovery, None); let mut out = Vec::new(); for (purl, uuid) in candidates { - if liveness.redirect_record(&purl, uuid).await { + if liveness.redirect_record_retained(&purl, uuid).await { out.push(purl); } } diff --git a/crates/socket-patch-core/src/hosted/engine.rs b/crates/socket-patch-core/src/hosted/engine.rs index 09901663b..b6baa2aef 100644 --- a/crates/socket-patch-core/src/hosted/engine.rs +++ b/crates/socket-patch-core/src/hosted/engine.rs @@ -751,7 +751,7 @@ pub async fn read_candidate_files( // unsatisfiable. Keyed `/Cargo.toml` for the cargo rewriter. if out.files.contains_key("Cargo.toml") && candidates.iter().any(|c| c.dep.ecosystem == "cargo") { - for rel in crate::utils::cargo_workspace::member_manifests_in(view) { + for rel in crate::utils::cargo_workspace::member_manifests_in(view).await { out.read(view, unreadable, &rel).await; } } diff --git a/crates/socket-patch-core/src/hosted/memory/mod.rs b/crates/socket-patch-core/src/hosted/memory/mod.rs index 4c40d8845..8553adf4d 100644 --- a/crates/socket-patch-core/src/hosted/memory/mod.rs +++ b/crates/socket-patch-core/src/hosted/memory/mod.rs @@ -53,7 +53,7 @@ use std::time::Instant; use crate::api::client::PatchApi; use crate::api::types::{PatchResponse, PatchSearchResult}; use crate::crawlers::Ecosystem; -use crate::utils::cargo_workspace::member_manifests_in; +use crate::utils::cargo_workspace::member_manifests_in_memory; use crate::vendor::lock_inventory::{ inventory_project_diagnosed_in, MemoryEntry, MemoryProject, ProjectView, }; @@ -279,7 +279,7 @@ fn demote_cargo_members(states: &mut [RootState], warnings: &mut Vec Option { /// A workspace-member manifest key the caller supplied: `/Cargo.toml`, /// a plain repo-relative path (never absolute, never `..`, never under the /// ledger's `.socket/` or a build `target/`). -fn is_cargo_member_manifest_key(key: &str) -> bool { +pub(crate) fn is_cargo_member_manifest_key(key: &str) -> bool { let Some(dir) = key.strip_suffix("/Cargo.toml") else { return false; }; @@ -7648,8 +7648,11 @@ pub fn hosted_patch_uuid(url: &str, extra_origins: &[String]) -> Option /// exact acceptance rules, without electing one segment. `vex` discovery /// uses it to RECOGNIZE every Socket identity a lockfile mentions, including /// the malformed or rejected shapes whose "last uuid" is not a patch. -pub fn hosted_patch_url_uuids(url: &str, extra_origins: &[String]) -> Option> { - use crate::patch::path_safety::is_canonical_uuid; +/// `url` normalized the way [`hosted_patch_url_uuids`] reads it: `\/` +/// unescaped, a percent-encoded `http(s)://` url decoded, and a Cargo +/// `sparse+` / `registry+` source prefix stripped. `None` without a +/// `://`. +pub(crate) fn normalized_hosted_url(url: &str) -> Option { use crate::utils::purl::percent_decode_purl_component; let unescaped = url.trim().replace("\\/", "/"); @@ -7661,13 +7664,20 @@ pub fn hosted_patch_url_uuids(url: &str, extra_origins: &[String]) -> Option { - &decoded[kind.len() + 1..] + decoded[kind.len() + 1..].to_string() } - _ => decoded.as_str(), - }; - let parsed = reqwest::Url::parse(text).ok()?; + _ => decoded, + }) +} + +pub fn hosted_patch_url_uuids(url: &str, extra_origins: &[String]) -> Option> { + use crate::patch::path_safety::is_canonical_uuid; + use crate::utils::purl::percent_decode_purl_component; + + let text = normalized_hosted_url(url)?; + let parsed = reqwest::Url::parse(&text).ok()?; if !matches!(parsed.scheme(), "https" | "http") || !parsed.username().is_empty() || parsed.password().is_some() diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index db99948a0..6aa8e0388 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -11,10 +11,11 @@ //! would follow it). Cargo still reads those manifests; the rewriter's //! Cargo.lock dependents check refuses a crate one of them depends on. -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; use std::sync::Arc; +use futures_util::StreamExt; use toml_edit::{DocumentMut, Item, Table}; use crate::utils::workspace_globs::segment_glob_matches; @@ -32,18 +33,179 @@ pub fn member_manifests(root: &Path) -> Vec { member_manifests_with(&DiskTree(root)) } -/// [`member_manifests`] over a [`ProjectView`]: the in-memory variant -/// reads the manifests the host supplied and expands member globs against -/// the supplied paths (a directory exists when some supplied path lives -/// under it; symbolic links are never directories). -pub fn member_manifests_in(view: &ProjectView<'_>) -> Vec { - match view { - ProjectView::Disk(_) | ProjectView::Snapshot(_) => { - let root = view.disk_root().expect("a disk view has a root"); - member_manifests(root) +/// [`member_manifests`] of an in-memory project: the manifests the host +/// supplied, member globs expanded against the supplied paths (a directory +/// exists when some supplied path lives under it; symbolic links are never +/// directories). +pub fn member_manifests_in_memory(project: &MemoryProject) -> Vec { + member_manifests_with(&MemoryTree(project)) +} + +/// [`member_manifests`] over a [`ProjectView`], every read, probe and +/// listing going through the view: a snapshot's overlays and content cache +/// apply and its recording notes the walk; in memory it reads the supplied +/// files. The manifests are read concurrently (bounded). +pub async fn member_manifests_in(view: &ProjectView<'_>) -> Vec { + member_manifest_texts_in(view) + .await + .into_iter() + .map(|(rel, _)| rel) + .collect() +} + +/// In-flight manifest reads of one [`member_manifest_texts_in`] level. +const MEMBER_READ_CONCURRENCY: usize = 16; + +/// [`member_manifests_in`], each key with the text the walk read through +/// the view (so a caller need not read it again). +pub(crate) async fn member_manifest_texts_in(view: &ProjectView<'_>) -> Vec<(String, String)> { + let Some((facts, _)) = view_manifest(view, "Cargo.toml").await else { + return Vec::new(); + }; + let mut frontier: Vec = Vec::new(); + if let Some((members, exclude)) = &facts.workspace { + let mut excluded: BTreeSet = BTreeSet::new(); + for pattern in exclude { + excluded.extend(expand_glob_in(view, pattern, CARGO_SKIP).await); + } + for pattern in members { + for dir in expand_glob_in(view, pattern, CARGO_SKIP).await { + if !excluded.contains(&dir) { + frontier.push(dir); + } + } + } + } + frontier.extend( + facts + .path_deps + .iter() + .filter_map(|dep| normalize_rel("", dep)), + ); + let mut found: BTreeMap = BTreeMap::new(); + let mut tried: BTreeSet = BTreeSet::new(); + while !frontier.is_empty() { + let mut level: Vec = Vec::new(); + for dir in std::mem::take(&mut frontier) { + if found.len() + level.len() >= MAX_MANIFESTS { + break; + } + if !walkable(&dir) || tried.contains(&dir) { + continue; + } + tried.insert(dir.clone()); + if every_segment(&dir, |at| view_is_real_dir(view, at)) { + level.push(dir); + } + } + let read: Vec<(String, Option)> = + crate::utils::concurrent::ordered_concurrent( + level, + MEMBER_READ_CONCURRENCY, + |dir| async move { + let manifest = view_manifest(view, &format!("{dir}/Cargo.toml")).await; + (dir, manifest) + }, + ) + .collect() + .await; + for (dir, manifest) in read { + let Some((facts, text)) = manifest else { + continue; + }; + frontier.extend( + facts + .path_deps + .iter() + .filter_map(|dep| normalize_rel(&dir, dep)), + ); + found.insert(dir, text); + } + } + found + .into_iter() + .map(|(dir, text)| (format!("{dir}/Cargo.toml"), text)) + .collect() +} + +/// A manifest the view walk read: its walk facts and its text. +type ReadManifest = (Arc, String); + +/// A regular (non-symlink) manifest read through the view: its walk facts +/// and its text. +async fn view_manifest(view: &ProjectView<'_>, rel: &str) -> Option { + if view.is_symlink(rel) { + return None; + } + let text = view.read_text(rel).await.ok()?; + let facts = FACTS_MEMO + .parse(text.as_bytes(), || { + text.parse::() + .map(|doc| ManifestFacts::of(&doc)) + }) + .ok()?; + Some((facts, text)) +} + +/// [`DirTree::is_real_dir`] through the view's own mediated probes (a +/// snapshot notes each and sees its overlays). +fn view_is_real_dir(view: &ProjectView<'_>, rel: &str) -> bool { + view.is_dir(rel) && !view.is_symlink(rel) +} + +/// [`expand_glob`] over a [`ProjectView`]: the same walk, listing through +/// the view's `list_dir` (a real directory's real sub-directories, as +/// [`DirTree::child_dirs`] answers). +async fn expand_glob_in(view: &ProjectView<'_>, pattern: &str, skip: &[&str]) -> Vec { + let Some(normalized) = normalize_rel("", pattern.trim_end_matches('/')) else { + return Vec::new(); + }; + let segments: Vec<&str> = normalized.split('/').filter(|s| !s.is_empty()).collect(); + let mut out = Vec::new(); + // [`expand_from`]'s recursion as an explicit stack, children pushed in + // reverse so they are visited in the same order. + let mut stack: Vec<(String, usize)> = vec![(String::new(), 0)]; + while let Some((at, i)) = stack.pop() { + if out.len() >= MAX_MANIFESTS { + break; + } + let Some(seg) = segments.get(i) else { + out.push(at); + continue; + }; + if !segment_is_pattern(seg) { + let next = join_rel(&at, seg); + if view_is_real_dir(view, &next) { + stack.push((next, i + 1)); + } + continue; + } + let Ok(entries) = view.list_dir(&at).await else { + continue; + }; + let children = glob_children( + entries.into_iter().filter(|e| e.is_dir).map(|e| e.name), + seg, + skip, + ); + let mut next: Vec<(String, usize)> = Vec::new(); + if *seg == "**" { + next.push((at.clone(), i + 1)); + next.extend(children.iter().map(|child| (join_rel(&at, child), i))); + } else { + let pattern = cargo_segment(seg); + for child in &children { + let name: Vec = child.chars().collect(); + if segment_glob_matches(&pattern, &name) { + next.push((join_rel(&at, child), i + 1)); + } + } } - ProjectView::Memory(project) => member_manifests_with(&MemoryTree(project)), + stack.extend(next.into_iter().rev()); } + out.sort(); + out.dedup(); + out } /// The two directory questions a glob walk asks, keyed by `/`-separated @@ -258,28 +420,36 @@ fn is_real_dir(path: &Path) -> bool { /// Every component of repo-relative `dir` is a real directory under /// the root — none is a symbolic link (which may lead outside the root). pub(crate) fn is_real_dir_path(tree: &dyn DirTree, dir: &str) -> bool { + every_segment(dir, |at| tree.is_real_dir(at)) +} + +/// Whether `real` holds for every ancestor-or-self prefix of `dir`. +fn every_segment(dir: &str, real: impl Fn(&str) -> bool) -> bool { let mut at = String::new(); dir.split('/').all(|seg| { at = join_rel(&at, seg); - tree.is_real_dir(&at) + real(&at) }) } +/// Whether the walk may enter `dir` at all. `target` is cargo's build +/// directory at every level: the glob walk already skips it, and a literal +/// `members = ["target/gen"]` or a path dependency into it names a manifest +/// the next `cargo clean` deletes — pinning it would record a hosted edit +/// whose file can vanish, blocking the rollback of every other cargo edit. +fn walkable(dir: &str) -> bool { + !dir.is_empty() && !dir.split('/').any(|seg| seg == "target") +} + fn enqueue( tree: &dyn Tree, dir: String, dirs: &mut BTreeSet, queue: &mut Vec<(String, Arc)>, ) { - // `target` is cargo's build directory at every level: the glob walk - // already skips it, and a literal `members = ["target/gen"]` or a path - // dependency into it names a manifest the next `cargo clean` deletes — - // pinning it would record a hosted edit whose file can vanish, blocking - // the rollback of every other cargo edit. - if dir.is_empty() + if !walkable(&dir) || dirs.len() >= MAX_MANIFESTS || dirs.contains(&dir) - || dir.split('/').any(|seg| seg == "target") || !is_real_dir_path(tree, &dir) { return; @@ -404,6 +574,23 @@ fn cargo_segment(seg: &str) -> Vec { seg.replace("[^", "[\0^").chars().collect() } +/// The sub-directories a wildcard segment `seg` may match, sorted: never a +/// dot-directory (unless `seg` itself starts with `.`) or a name in `skip`. +fn glob_children( + entries: impl IntoIterator, + seg: &str, + skip: &[&str], +) -> Vec { + let mut children: Vec = entries + .into_iter() + .filter(|name| { + (!name.starts_with('.') || seg.starts_with('.')) && !skip.contains(&name.as_str()) + }) + .collect(); + children.sort(); + children +} + fn expand_from( tree: &dyn DirTree, at: String, @@ -428,13 +615,7 @@ fn expand_from( let Some(entries) = tree.child_dirs(&at) else { return; }; - let mut children: Vec = entries - .into_iter() - .filter(|name| { - (!name.starts_with('.') || seg.starts_with('.')) && !skip.contains(&name.as_str()) - }) - .collect(); - children.sort(); + let children = glob_children(entries, seg, skip); if *seg == "**" { expand_from(tree, at.clone(), tail, skip, out); for child in children { @@ -478,6 +659,124 @@ mod tests { format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\n") } + /// Over a tracked snapshot the walk reads through the view: its reads + /// fill the content cache, so files written just now (racy by stat + /// alone) — including a `.socket/` copy's manifest the walk follows + /// through `[patch]` — still leave a read set that holds; a member + /// added under a glob afterwards breaks it (the listing was recorded). + #[tokio::test] + async fn snapshot_walk_records_what_it_reads() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write( + root, + "Cargo.toml", + "[workspace]\nmembers = [\"crates/*\"]\n\n\ + [patch.crates-io]\nserde = { path = \".socket/vendor/cargo/u1/serde-1.0.0\" }\n", + ); + write(root, "crates/a/Cargo.toml", &pkg("a")); + write( + root, + ".socket/vendor/cargo/u1/serde-1.0.0/Cargo.toml", + &pkg("serde"), + ); + let snap = crate::vendor::lock_inventory::DiskSnapshot::tracked(root); + snap.begin_recording(); + let walked = member_manifests_in(&ProjectView::Snapshot(&snap)).await; + assert_eq!(walked, member_manifests(root)); + assert!(walked.contains(&".socket/vendor/cargo/u1/serde-1.0.0/Cargo.toml".to_string())); + let reads = snap.end_recording().expect("the walk keeps the recording"); + assert!(reads.unchanged(), "fresh files read through the view hold"); + write(root, "crates/b/Cargo.toml", &pkg("b")); + assert!(!reads.unchanged(), "a new member dir changes the listing"); + } + + /// A snapshot's overlays are part of the walk: an overlaid member + /// manifest (a pending write) is a member before it reaches the disk. + #[tokio::test] + async fn snapshot_walk_sees_overlays() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write( + root, + "Cargo.toml", + "[workspace]\nmembers = [\"crates/*\"]\n", + ); + write(root, "crates/a/Cargo.toml", &pkg("a")); + let snap = crate::vendor::lock_inventory::DiskSnapshot::new(root); + snap.overlay("crates/c/Cargo.toml", pkg("c").as_bytes()); + assert_eq!( + member_manifests_in(&ProjectView::Snapshot(&snap)).await, + vec!["crates/a/Cargo.toml", "crates/c/Cargo.toml"] + ); + assert_eq!(member_manifests(root), vec!["crates/a/Cargo.toml"]); + } + + /// The view walk answers exactly as the disk walk over every shape the + /// memo test covers (globs, `**`, excludes, nested path dependencies, + /// `[patch]` copies, unparseable manifests) plus a symlinked member + /// directory, and returns the text it read for each. + #[tokio::test] + async fn view_walk_matches_the_disk_walk() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write( + root, + "Cargo.toml", + "[workspace]\nmembers = [\"crates/*\", \"tools/**\", \"lit\"]\n\ + exclude = [\"crates/x\"]\n\ + [patch.crates-io]\nserde = { path = \".socket/vendor/cargo/u1/serde-1.0.0\" }\n\ + [workspace.dependencies]\nshared = { path = \"libs/shared\" }\n", + ); + write( + root, + "crates/a/Cargo.toml", + &format!("{}[dependencies.b]\npath = \"../b\"\n", pkg("a")), + ); + write(root, "crates/b/Cargo.toml", &pkg("b")); + write(root, "crates/x/Cargo.toml", &pkg("x")); + write(root, "crates/.hidden/Cargo.toml", &pkg("hidden")); + write(root, "crates/target/Cargo.toml", &pkg("target")); + write(root, "crates/bad/Cargo.toml", "not = [toml"); + write(root, "tools/deep/t/Cargo.toml", &pkg("t")); + write(root, "lit/Cargo.toml", &pkg("lit")); + write(root, "libs/shared/Cargo.toml", &pkg("shared")); + write( + root, + ".socket/vendor/cargo/u1/serde-1.0.0/Cargo.toml", + &format!( + "{}[dependencies]\nderive = {{ path = \"../derive\" }}\n", + pkg("serde") + ), + ); + write( + root, + ".socket/vendor/cargo/u1/derive/Cargo.toml", + &pkg("derive"), + ); + #[cfg(unix)] + { + write(root, "elsewhere/l/Cargo.toml", &pkg("l")); + std::os::unix::fs::symlink(root.join("elsewhere"), root.join("crates/link")).unwrap(); + } + let disk = member_manifests(root); + assert!(disk.contains(&"tools/deep/t/Cargo.toml".to_string())); + assert!(!disk.iter().any(|k| k.starts_with("crates/link"))); + assert_eq!(member_manifests_in(&ProjectView::Disk(root)).await, disk); + let snap = crate::vendor::lock_inventory::DiskSnapshot::new(root); + assert_eq!( + member_manifests_in(&ProjectView::Snapshot(&snap)).await, + disk + ); + for (rel, text) in member_manifest_texts_in(&ProjectView::Disk(root)).await { + assert_eq!( + text, + std::fs::read_to_string(root.join(&rel)).unwrap(), + "{rel}" + ); + } + } + #[test] fn members_globs_excludes_and_path_dependencies() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 40e6ab008..3139998ec 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -29,12 +29,17 @@ //! (cargo < 1.41) files it in `[metadata]` as `"checksum //! ()"` instead — read as the same pin. //! -//! `Cargo.toml` decides whether cargo still USES that entry (rule 10): cargo +//! The manifests decide whether cargo still USES that entry (rule 10): cargo //! re-resolves a locked package whose source no longer matches the -//! manifest's declaration. So for the lock entry's crate: +//! manifest's declaration. The manifests are the root `Cargo.toml` and every +//! workspace member's (`/Cargo.toml`, enumerated as the hosted rewriter +//! enumerates the manifests it pins: +//! [`crate::utils::cargo_workspace::member_manifests_in`], minus `.socket/` +//! and `target/`), whose declarations count alike. So for the lock entry's +//! crate: //! -//! * not declared in the root `Cargo.toml` at all (a workspace member's dep -//! the root lock describes; no / unparseable manifest) → ref; +//! * not declared in any of them (a transitive-only crate; no / unparseable +//! root manifest) → ref; //! * some declaration pins `socket-patch-` (the same uuid) → ref; //! * declarations pin only OTHER `socket-patch-` registries → the lock //! ref AND a `Cargo.toml` ref for `` at the locked version (no pin): @@ -145,29 +150,43 @@ const DEP_TABLES: [&str; 5] = [ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { let lock = load_lock(ctx, out).await; - let manifest = read_toml(ctx, CARGO_TOML, out).await; + let root = read_toml(ctx, CARGO_TOML, out).await; let config = read_config(ctx, out).await; - - // Crate name → the `registry` value of each root-manifest declaration - // (`None` = declared with no registry key). Absent map = no usable - // manifest. - let decls = manifest.as_ref().map(|doc| { - let mut decls = manifest_declarations(doc); + // The root manifest, then every workspace member's — the manifests the + // hosted rewriter pins a crate in (each that declares it). + let mut manifests: Vec<(String, DocumentMut)> = Vec::new(); + if let Some(doc) = root { + let root_pins = manifest_has_socket_pin(&doc); + manifests.push((CARGO_TOML.to_string(), doc)); + manifests.extend(read_members(ctx, &lock, root_pins, out).await); + } + let manifest: Option<&DocumentMut> = manifests.first().map(|(_, doc)| doc); + + // Crate name → each declaration of it in the root or a member manifest + // (its file, `registry` value — `None` = declared with no registry key — + // and version requirement). Absent map = no usable root manifest. + let decls = manifest.map(|doc| { + let mut decls = manifest_declarations(&manifests); + // `[patch]` is read from the root manifest only: cargo ignores (and + // warns about) a member's. for patch in patch_entries(doc) { if let Some(reg) = patch.registry { let name = patch.name.to_string(); - decls.entry(name).or_default().push(Some(reg.to_string())); + decls + .entry(name) + .or_default() + .push(Decl::new(CARGO_TOML, Some(reg), None)); } } decls }); // The config's `[patch]` registry pins are also declarations cargo - // honors (only matters for crates the manifest also declares). + // honors (only matters for crates the manifests also declare). let decls = decls.map(|mut decls| { - if let Some((_, doc)) = &config { + if let Some((file, doc)) = &config { for patch in patch_entries(doc) { if let (Some(reg), Some(existing)) = (patch.registry, decls.get_mut(patch.name)) { - existing.push(Some(reg.to_string())); + existing.push(Decl::new(file, Some(reg), None)); } } } @@ -180,10 +199,10 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { hosted_from_lock(ctx, &lock, decls.as_ref(), &definitions, out); unpatched_twins(ctx, &lock, out); - if let (Some(decls), Some(doc)) = (&decls, &manifest) { - unresolved_manifest_pins(ctx, &lock, decls, doc, &definitions, out); + if let Some(decls) = &decls { + unresolved_manifest_pins(ctx, &lock, decls, &definitions, out); } - if let Some(doc) = &manifest { + if let Some(doc) = manifest { // Cargo drops a manifest crates.io `[patch]` item that the project // config redefines under the same key (any version), and a // `[patch.""]` table replaces `[patch.crates-io]` @@ -223,7 +242,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { vendored_from_patches(ctx, file, doc, &lock, &|_| None, out).await; } let wired: Vec = manifest - .iter() + .into_iter() .chain(config.iter().map(|(_, doc)| doc)) .flat_map(socket_copies) .collect(); @@ -311,6 +330,68 @@ async fn read_toml(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discovery) -> Op parse_toml(file, &text, out) } +/// Every workspace member manifest (`/Cargo.toml`) of the root one, +/// parsed — the set the hosted rewriter pins in +/// ([`crate::utils::cargo_workspace::member_manifests_in`], keeping the +/// keys it accepts: never under `.socket/` — a vendored copy is Socket's, +/// not a declaration — or a build `target/`). Unparseable ones are skipped +/// (diagnosed). +/// +/// Members only ever decide about Socket wiring, so the walk is skipped +/// when there is none they could decide about: a parsed lock with no +/// Socket-sourced package, or an unparseable one (no ref either way). With +/// no lock, a member matters only when some manifest carries a +/// `socket-patch-` pin (a lockless pin, [`UnlockedPin`]); the walk has the +/// texts, so the parse is skipped otherwise. +async fn read_members( + ctx: &DiscoverCtx<'_>, + lock: &Lock, + root_pins: bool, + out: &mut Discovery, +) -> Vec<(String, DocumentMut)> { + let lockless = match lock { + Lock::Parsed(_) if has_socket_sourced(ctx, lock) => false, + Lock::Parsed(_) | Lock::Unparseable => return Vec::new(), + Lock::Absent => true, + }; + let texts: Vec<(String, String)> = + crate::utils::cargo_workspace::member_manifest_texts_in(&ctx.view) + .await + .into_iter() + .filter(|(rel, _)| crate::patch::redirect::is_cargo_member_manifest_key(rel)) + .collect(); + if lockless && !root_pins && !texts.iter().any(|(_, t)| t.contains(PIN_NAME_PREFIX)) { + return Vec::new(); + } + let mut members = Vec::new(); + for (rel, text) in texts { + ctx.adopt_read(&rel, &text); + if let Some(doc) = parse_toml(&rel, &text, out) { + members.push((rel, doc)); + } + } + members +} + +/// Whether some dependency declaration of `doc` names a `socket-patch-` +/// registry. +fn manifest_has_socket_pin(doc: &DocumentMut) -> bool { + dependency_entries(doc).iter().any(|d| { + d.registry + .as_deref() + .is_some_and(|r| r.trim().starts_with(PIN_NAME_PREFIX)) + }) +} + +/// Whether the lock resolves some package from a Socket patch server. +fn has_socket_sourced(ctx: &DiscoverCtx<'_>, lock: &Lock) -> bool { + lock.packages().iter().any(|p| { + p.source + .as_deref() + .is_some_and(|s| source_uuid(ctx, s).is_some()) + }) +} + fn parse_toml(file: &str, text: &str, out: &mut Discovery) -> Option { toml_or_diag(file, text, TomlDiag::TrimEnd, out).map(toml_edit::Document::into_mut) } @@ -355,19 +436,83 @@ async fn read_config( // ── manifest / config shapes ───────────────────────────────────────────── -/// Every dependency declaration in a manifest, keyed by the crate it names -/// (`package = "…"` when renamed, else the key): the `registry` value of -/// each (`None` = no registry key). `workspace = true` inheritors are -/// skipped — the `[workspace.dependencies]` entry they inherit is itself -/// collected. -fn manifest_declarations(doc: &DocumentMut) -> BTreeMap>> { - let mut decls: BTreeMap>> = BTreeMap::new(); - for dep in dependency_entries(doc) { - decls.entry(dep.name).or_default().push(dep.registry); +/// One declaration of a crate: the root-relative file holding it and its +/// `registry` value (`None` = no registry key). +#[derive(Debug, Clone)] +struct Decl { + file: String, + registry: Option, + /// The `version` requirement (`None` for a `[patch]` registry pin, or + /// a declaration without one). + version: Option, +} + +impl Decl { + fn new(file: &str, registry: Option<&str>, version: Option<&str>) -> Self { + Decl { + file: file.to_string(), + registry: registry.map(str::to_string), + version: version.map(str::to_string), + } + } + + /// The Socket patch uuid a well-formed `socket-patch-` registry + /// names. + fn socket_uuid(&self) -> Option { + socket_patch_name_uuid(self.registry.as_deref()?, false) + } + + /// Whether the registry is `socket-patch-…`-shaped (well-formed or not). + fn socket_shaped(&self) -> bool { + self.registry + .as_deref() + .is_some_and(|r| r.trim().starts_with(PIN_NAME_PREFIX)) + } +} + +/// Crate name → its declarations ([`Decl`]). +type Decls = BTreeMap>; + +/// Every dependency declaration in `manifests` (root first, then members), +/// keyed by the crate it names (`package = "…"` when renamed, else the +/// key). `workspace = true` inheritors are skipped — the root's +/// `[workspace.dependencies]` entry they inherit is itself collected. +fn manifest_declarations(manifests: &[(String, DocumentMut)]) -> Decls { + let mut decls = Decls::new(); + for (file, doc) in manifests { + for dep in dependency_entries(doc) { + decls.entry(dep.name).or_default().push(Decl::new( + file, + dep.registry.as_deref(), + dep.version.as_deref(), + )); + } } decls } +/// The distinct present values, in first-seen order. +fn distinct(values: impl IntoIterator>) -> Vec { + let mut out: Vec = Vec::new(); + for value in values.into_iter().flatten() { + if !out.contains(&value) { + out.push(value); + } + } + out +} + +/// The distinct files among `decls`, in order, joined for a message. +fn decl_files<'a>(decls: impl IntoIterator) -> String { + let mut files: Vec<&str> = Vec::new(); + for d in decls { + if !files.contains(&d.file.as_str()) { + files.push(&d.file); + } + } + files.join(", ") +} + /// One manifest dependency declaration (see [`manifest_declarations`]). struct DepEntry { name: String, @@ -437,7 +582,7 @@ fn source_uuid(ctx: &DiscoverCtx<'_>, source: &str) -> Option { fn hosted_from_lock( ctx: &DiscoverCtx<'_>, lock: &Lock, - decls: Option<&BTreeMap>>>, + decls: Option<&Decls>, definitions: &BTreeMap, out: &mut Discovery, ) { @@ -488,43 +633,53 @@ fn hosted_from_lock( true, ); let Some(regs) = decls.and_then(|d| d.get(&pkg.name)) else { - // Not declared by the root manifest (a member's dependency the - // root lock describes), or no usable manifest: the lock is the - // only word. + // Declared by no manifest (a transitive-only crate), or no + // usable root manifest: the lock is the only word. out.push(lock_ref); continue; }; - if regs.iter().any(|r| r.as_deref() == Some(reg.as_str())) { + if regs + .iter() + .any(|d| d.registry.as_deref() == Some(reg.as_str())) + { out.push(lock_ref); continue; } - let others: Vec = regs + let others: Vec<(&str, String)> = regs .iter() - .flatten() - .filter_map(|r| socket_patch_name_uuid(r, false)) + .filter_map(|d| { + let other = socket_patch_name_uuid(d.registry.as_deref()?, false)?; + Some((d.file.as_str(), other)) + }) .collect(); if others.is_empty() { + let files = decl_files(regs); + let verb = if files.contains(", ") { + "declare" + } else { + "declares" + }; out.diag( DIAG_REF_INVALID, CARGO_LOCK, format!( - "{CARGO_LOCK}: {purl} resolves from Socket patch {uuid}, but {CARGO_TOML} \ - declares {} without registry = \"{reg}\" — cargo re-resolves it from the \ + "{CARGO_LOCK}: {purl} resolves from Socket patch {uuid}, but {files} \ + {verb} {} without registry = \"{reg}\" — cargo re-resolves it from the \ declared source; not counted", pkg.name ), ); continue; } - // The manifest routes the crate to a DIFFERENT Socket patch than the + // A manifest routes the crate to a DIFFERENT Socket patch than the // lock pins: emit both wirings and let the CLI's conflict gate refuse // to pick one. out.push(lock_ref); - for other in others { + for (file, other) in others { out.push(PatchedRef::hosted( purl.clone(), other, - CARGO_TOML, + file, None, None, true, @@ -584,74 +739,111 @@ fn unpatched_twins(ctx: &DiscoverCtx<'_>, lock: &Lock, out: &mut Discovery) { } /// Diagnose `Cargo.toml` Socket-registry pins no lock entry turns into a ref -/// (no exact version to attest), and malformed `socket-patch-*` names. +/// (no exact version to attest), and malformed `socket-patch-*` names. One +/// diagnostic per crate and patch, naming every manifest that pins it. /// /// With NO `Cargo.lock` at all (a library that gitignores it; `rewrite_cargo` /// supports that state — `LockCommit::Absent` — and writes only the pin and -/// the registry definition) a pin is still live wiring for the crate: every -/// declaration of it routes to `socket-patch-`, the project config defines -/// that registry on the Socket host for the same `U`, and that registry -/// serves only the patched version. It is recorded as an [`UnlockedPin`] -/// carrying the manifest's version requirements, so a redirect-ledger record -/// whose exact version satisfies them stays live -/// ([`Discovery::hosted_claim`]); it is still not a ref (no version to -/// attest on its own). +/// the registry definition) a pin is still live wiring for the crate: no +/// declaration routes it to another Socket registry, the project config +/// defines that registry on the Socket host for the same `U`, and that +/// registry serves only the patched version. It is recorded as ONE +/// [`UnlockedPin`] per crate and patch, carrying the pinning declarations' +/// version requirements (from the root and every member) and, as +/// `excluded_reqs`, those of the declarations without a pin: the rewriter +/// leaves a declaration alone only when its requirement excludes the +/// patched version, so a redirect-ledger record stays live +/// ([`Discovery::hosted_claim`]) when its exact version satisfies every +/// pinned requirement and no unpinned one. An unpinned declaration with no +/// version requirement (git, path) may resolve any version: no pin. It is +/// still not a ref (no version to attest on its own). fn unresolved_manifest_pins( ctx: &DiscoverCtx<'_>, lock: &Lock, - decls: &BTreeMap>>, - manifest: &DocumentMut, + decls: &Decls, definitions: &BTreeMap, out: &mut Discovery, ) { - let deps = dependency_entries(manifest); for (name, regs) in decls { - for reg in regs.iter().flatten() { - if !reg.trim().starts_with(PIN_NAME_PREFIX) { - continue; - } - let Some(uuid) = socket_patch_name_uuid(reg, false) else { - out.diag( - DIAG_REF_INVALID, - CARGO_TOML, - format!( - "{CARGO_TOML}: {name} is pinned to registry {reg:?}, which is not a \ - socket-patch- name" - ), - ); - continue; - }; - let socket_sourced = lock.packages().iter().any(|p| { - p.name == *name - && p.source - .as_deref() - .and_then(|s| source_uuid(ctx, s)) - .is_some() - }); - if socket_sourced { - continue; // a ref (or the conflict pair) came from the lock + let mut malformed: Vec<(&str, &str)> = Vec::new(); + let mut uuids: Vec = Vec::new(); + for decl in regs.iter().filter(|d| d.socket_shaped()) { + let reg = decl.registry.as_deref().unwrap_or_default(); + match decl.socket_uuid() { + Some(uuid) if !uuids.contains(&uuid) => uuids.push(uuid), + Some(_) => {} + None if !malformed.contains(&(decl.file.as_str(), reg)) => { + malformed.push((decl.file.as_str(), reg)); + } + None => {} } + } + for (file, reg) in malformed { + out.diag( + DIAG_REF_INVALID, + file, + format!( + "{file}: {name} is pinned to registry {reg:?}, which is not a \ + socket-patch- name" + ), + ); + } + if uuids.is_empty() { + continue; + } + let socket_sourced = lock.packages().iter().any(|p| { + p.name == *name + && p.source + .as_deref() + .and_then(|s| source_uuid(ctx, s)) + .is_some() + }); + if socket_sourced { + continue; // a ref (or the conflict pair) came from the lock + } + for uuid in &uuids { + let pinning: Vec<&Decl> = regs + .iter() + .filter(|d| d.socket_uuid().as_deref() == Some(uuid.as_str())) + .collect(); + let reg = pinning[0].registry.as_deref().unwrap_or_default(); + let file = pinning[0].file.as_str(); let why = match lock { Lock::Unparseable => continue, // already diagnosed Lock::Absent => { - let only_this_registry = regs.iter().all(|r| r.as_deref() == Some(reg)); + // Another Socket registry (or a malformed one) for the + // same crate is a conflict, never a pin. + let only_this_patch = regs + .iter() + .filter(|d| d.socket_shaped()) + .all(|d| d.socket_uuid().as_deref() == Some(uuid.as_str())); let defined_here = definitions - .get(reg.as_str()) + .get(reg) .and_then(|index| source_uuid(ctx, index)) - .is_some_and(|u| u == uuid); - let version_reqs: Vec = deps + .is_some_and(|u| u == *uuid); + let version_reqs = distinct(pinning.iter().map(|d| d.version.clone())); + // `None` when some unpinned declaration carries no + // requirement (git, path): it may resolve any version. + let excluded_reqs: Option> = regs .iter() - .filter(|d| d.name == *name && d.registry.as_deref() == Some(reg)) - .filter_map(|d| d.version.clone()) + .filter(|d| !d.socket_shaped()) + .map(|d| d.version.clone()) .collect(); - if only_this_registry && defined_here && !version_reqs.is_empty() { + if let (true, true, false, Some(excluded_reqs)) = ( + only_this_patch, + defined_here, + version_reqs.is_empty(), + excluded_reqs, + ) { + let excluded_reqs = distinct(excluded_reqs.into_iter().map(Some)); out.unlocked_pin(UnlockedPin { ecosystem: "cargo".to_string(), name: name.clone(), uuid: uuid.clone(), - file: CARGO_TOML.into(), + file: file.into(), version_reqs, - index_url: definitions.get(reg.as_str()).cloned(), + excluded_reqs, + index_url: definitions.get(reg).cloned(), }); } format!("there is no {CARGO_LOCK} to fix its version") @@ -662,10 +854,11 @@ fn unresolved_manifest_pins( --patch-server-url)" ), }; + let files = decl_files(pinning.iter().copied()); out.diag( DIAG_REF_UNATTRIBUTABLE, - CARGO_TOML, - format!("{CARGO_TOML}: {name} is pinned to Socket patch {uuid}, but {why}"), + file, + format!("{files}: {name} is pinned to Socket patch {uuid}, but {why}"), ); } } @@ -1362,6 +1555,296 @@ mod tests { } } + /// A workspace root that declares nothing itself, with one member. + fn workspace_with_member(p: &Project, member_deps: &str) { + p.write("Cargo.toml", "[workspace]\nmembers = [\"crates/*\"]\n"); + p.write( + "crates/a/Cargo.toml", + format!( + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n[dependencies]\n{member_deps}\n" + ), + ); + } + + /// REGRESSION (C2): the hosted rewriter pins the crate in every member + /// manifest that declares it, so a member's declaration decides like the + /// root's. A member reverted to crates.io (the root declares nothing) + /// makes cargo re-resolve from crates.io: no ref, diagnosed against the + /// member, and the stale lock entry's patch is recognized (a ledger + /// record for it is dead). + #[tokio::test] + async fn reverted_member_manifest_pin_drops_the_stale_lock_entry() { + for decl in [ + "serde = \"1.0.1\"".to_string(), + "serde = { version = \"1\", registry = \"my-corp\" }".to_string(), + ] { + let p = Project::new(); + workspace_with_member(&p, &decl); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_refs(&out, &[]); + assert_eq!(diag_codes(&out), vec![DIAG_REF_INVALID], "{decl}"); + assert!( + out.diagnostics[0].detail.contains("crates/a/Cargo.toml"), + "{:#?}", + out.diagnostics + ); + assert_eq!( + out.hosted_claim("pkg:cargo/serde@1.0.1", UUID_A), + Some(false), + "{decl}" + ); + } + } + + /// The member's own pin keeps the lock entry a ref; a member pin for + /// ANOTHER patch emits the conflict pair, attributed to the member. + #[tokio::test] + async fn member_manifest_pins_count_like_the_roots() { + let p = Project::new(); + workspace_with_member(&p, &pinned("serde", "1", UUID_A)); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + assert!(out.diagnostics.is_empty(), "{:#?}", out.diagnostics); + + let p = Project::new(); + workspace_with_member(&p, &pinned("serde", "1", UUID_B)); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted), + ("pkg:cargo/serde@1.0.1", UUID_B, WiringMode::Hosted), + ], + ); + let toml_ref = out.refs.iter().find(|r| r.uuid == UUID_B).unwrap(); + assert_eq!( + toml_ref.source_file, + std::path::PathBuf::from("crates/a/Cargo.toml") + ); + } + + /// The root's pin still decides when a member declares the crate under + /// another version requirement the pin does not reach (the rewriter + /// leaves such a declaration alone): some declaration pins the patch. + #[tokio::test] + async fn a_root_pin_with_an_unpinned_member_declaration_is_a_ref() { + let p = Project::new(); + p.write( + "Cargo.toml", + format!( + "[workspace]\nmembers = [\"crates/*\"]\n\n[package]\nname = \"app\"\n\ + version = \"0.1.0\"\n\n[dependencies]\n{}\n", + pinned("serde", "1", UUID_A) + ), + ); + p.write( + "crates/a/Cargo.toml", + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n[dependencies]\nserde = \"0.9\"\n", + ); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + } + + /// A manifest under `.socket/` (a path dependency into Socket's own + /// tree, as a vendored copy is) is not a declaration the hosted + /// rewriter pins, so it never decides: the lock stays the only word. + #[tokio::test] + async fn manifests_under_dot_socket_are_not_declarations() { + let p = Project::new(); + p.write( + "Cargo.toml", + manifest("local = { path = \".socket/local\" }"), + ); + p.write( + ".socket/local/Cargo.toml", + "[package]\nname = \"local\"\nversion = \"0.1.0\"\n\n[dependencies]\nserde = \"1\"\n", + ); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + } + + /// With no lock, a member's pin is an unlocked pin carrying the member's + /// requirement, attributed to the member manifest. + #[tokio::test] + async fn lockless_member_pin_is_an_unlocked_pin() { + let p = Project::new(); + workspace_with_member(&p, &pinned("serde", "1", UUID_A)); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_refs(&out, &[]); + assert_eq!(out.unlocked_pins.len(), 1, "{:#?}", out.unlocked_pins); + let pin = &out.unlocked_pins[0]; + assert_eq!(pin.file, std::path::PathBuf::from("crates/a/Cargo.toml")); + assert_eq!(pin.version_reqs, vec!["1".to_string()]); + assert_eq!(diag_codes(&out), vec![DIAG_REF_UNATTRIBUTABLE]); + assert_eq!( + out.diagnostics[0].file, + std::path::PathBuf::from("crates/a/Cargo.toml") + ); + } + + /// A workspace whose root pins `serde` and declares `crates/*` members. + fn pinned_root_workspace(p: &Project) { + p.write( + "Cargo.toml", + format!( + "[workspace]\nmembers = [\"crates/*\"]\n\n[package]\nname = \"app\"\n\ + version = \"0.1.0\"\n\n[dependencies]\n{}\n", + pinned("serde", "1", UUID_A) + ), + ); + } + + fn member(p: &Project, dir: &str, deps: &str) { + p.write( + &format!("crates/{dir}/Cargo.toml"), + format!("[package]\nname = \"{dir}\"\nversion = \"0.1.0\"\n\n[dependencies]\n{deps}\n"), + ); + } + + /// REVIEW (lockless members): a member's unpinned declaration at a + /// version the pin does not reach (`serde = "0.9"` beside the root's + /// pinned `serde = "1"`, which the rewriter leaves alone) keeps the + /// lockless pin; it only excludes the versions it accepts. One that + /// accepts the record's version, or that has no requirement at all (a + /// git dependency), leaves the record unrouted. + #[tokio::test] + async fn lockless_pin_survives_a_member_declaring_another_version() { + let record = "pkg:cargo/serde@1.0.1"; + for (member_decl, live) in [ + ("serde = \"0.9\"", true), + ("serde = \"1.0\"", false), + ("serde = { git = \"https://example.com/serde\" }", false), + ] { + let p = Project::new(); + pinned_root_workspace(&p); + member(&p, "a", member_decl); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_refs(&out, &[]); + assert_eq!( + out.hosted_claim(record, UUID_A), + Some(live), + "{member_decl}: {:#?}", + out.unlocked_pins + ); + } + let p = Project::new(); + pinned_root_workspace(&p); + member(&p, "a", "serde = \"0.9\""); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_eq!(out.unlocked_pins.len(), 1); + assert_eq!(out.unlocked_pins[0].excluded_reqs, vec!["0.9".to_string()]); + assert_eq!( + out.hosted_claim("pkg:cargo/serde@0.9.5", UUID_A), + Some(false), + "the member resolves 0.9 from crates.io" + ); + } + + /// REVIEW (one pin per crate and patch): the root and two members pin the + /// same patch — one lockless pin carrying every requirement, one + /// diagnostic listing each manifest once. + #[tokio::test] + async fn one_lockless_pin_and_diagnostic_per_crate_and_patch() { + let p = Project::new(); + pinned_root_workspace(&p); + member(&p, "a", &pinned("serde", "1.0", UUID_A)); + member( + &p, + "b", + &format!( + "{}\n\n[dev-dependencies]\n{}", + pinned("serde", "1", UUID_A), + pinned("serde", "1", UUID_A) + ), + ); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_eq!(out.unlocked_pins.len(), 1, "{:#?}", out.unlocked_pins); + let pin = &out.unlocked_pins[0]; + assert_eq!(pin.file, std::path::PathBuf::from("Cargo.toml")); + assert_eq!(pin.version_reqs, vec!["1".to_string(), "1.0".to_string()]); + assert!(pin.excluded_reqs.is_empty()); + assert_eq!(diag_codes(&out), vec![DIAG_REF_UNATTRIBUTABLE]); + assert!( + out.diagnostics[0] + .detail + .starts_with("Cargo.toml, crates/a/Cargo.toml, crates/b/Cargo.toml: serde"), + "{}", + out.diagnostics[0].detail + ); + } + + /// REVIEW (perf): members only matter to Socket wiring, so a project + /// with a lock that resolves nothing from a Socket patch server — or no + /// lock and no `socket-patch-` pin anywhere — reads no member manifest + /// as discovery's own (none in [`Discovery::read`], which the full run + /// collects). + #[tokio::test] + async fn members_are_read_only_when_socket_wiring_could_need_them() { + let member_read = |out: &Discovery| { + out.read + .iter() + .any(|r| r.file == std::path::Path::new("crates/a/Cargo.toml")) + }; + // A plain lock: no walk. + let p = Project::new(); + workspace_with_member(&p, "serde = \"1\""); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(CRATES_IO), Some(CKSUM))]), + ); + assert!(!member_read(&p.discover().await)); + // No lock, no pin anywhere: walked, not adopted. + let p = Project::new(); + workspace_with_member(&p, "serde = \"1\""); + assert!(!member_read(&p.discover().await)); + // A Socket-sourced lock entry: read. + let p = Project::new(); + workspace_with_member(&p, "serde = \"1\""); + p.write( + "Cargo.lock", + lock(&[("serde", "1.0.1", Some(&index(UUID_A)), Some(CKSUM))]), + ); + assert!(member_read(&p.discover().await)); + // No lock, a member pin: read. + let p = Project::new(); + workspace_with_member(&p, &pinned("serde", "1", UUID_A)); + assert!(member_read(&p.discover().await)); + } + /// A rotated grant token in the definition is the same patch. #[tokio::test] async fn rotated_token_in_the_definition_still_agrees() { diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 815d4644d..9ed147621 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -415,6 +415,13 @@ pub struct UnlockedPin { /// `version = "…"`); a ledger version must satisfy every one. Empty = /// none to check (nuget: the csproj `Version` is only a minimum). pub version_reqs: Vec, + /// Requirements of the package's OTHER declarations, which carry no + /// pin (cargo: a member's `serde = "0.9"` beside the root's pinned + /// `serde = "1"`). The hosted rewriter pins every declaration whose + /// requirement accepts the patched version, so one that accepts a + /// ledger version means the pin does not reach it: a ledger version must + /// satisfy none of these. + pub excluded_reqs: Vec, /// The Socket index url the pin routes to (the nuget source value, the /// cargo registry's `index`): its grant-token segment is part of this /// pin, not other hosted wiring. @@ -438,16 +445,22 @@ impl UnlockedPin { }; let pinned = canonical_base_purl(&format!("pkg:{}/{}@{version}", self.ecosystem, self.name)); + let parsed = semver::Version::parse(version); + // An unreadable requirement or version answers `None`: it matches + // no pinned requirement and is never proven excluded. + let matches = |req: &str| match (semver::VersionReq::parse(req), &parsed) { + (Ok(req), Ok(version)) => Some(req.matches(version)), + _ => None, + }; pinned == key - && self.version_reqs.iter().all(|req| { - match ( - semver::VersionReq::parse(req), - semver::Version::parse(version), - ) { - (Ok(req), Ok(version)) => req.matches(&version), - _ => false, - } - }) + && self + .version_reqs + .iter() + .all(|req| matches(req) == Some(true)) + && self + .excluded_reqs + .iter() + .all(|req| matches(req) == Some(false)) } } @@ -760,6 +773,12 @@ pub struct Discovery { /// (#1445) ([`StaleSelector`]). Not refs and not contested wiring: the /// management commands retire them. Sorted, deduped. pub stale_selectors: Vec, + /// The operator's patch-server origins ([`DiscoverOptions::patch_server_origins`]) + /// this discovery classified urls with. Ledger liveness + /// ([`Discovery::redirect_record_live`]) classifies the lock inventory's + /// urls with the same origins, so a record discovery does not claim is + /// judged by the one origin rule ([`crate::patch::redirect::hosted_patch_uuid`]). + pub patch_server_origins: Vec, } /// A Socket-hosted yarn berry `resolutions` selector whose lock entry is @@ -1544,6 +1563,7 @@ async fn discover_with_ctx(mut ctx: DiscoverCtx<'_>) -> Discovery { out.unattest_unwired_copies(); out.recognized.extend(ctx.take_recognized()); out.read.extend(ctx.take_read()); + out.patch_server_origins = ctx.patch_server_origins.to_vec(); out.finalize(); out } @@ -1647,6 +1667,14 @@ impl<'a> DiscoverCtx<'a> { } /// Log a guarded read of `rel` ([`Discovery::read`]). + /// Account for `text`, which a walk through [`Self::view`] read from + /// `rel` on this ctx's behalf, exactly as [`Self::read_text`] accounts + /// for its own reads (rule 11 recognition and the read log). + pub(crate) fn adopt_read(&self, rel: &str, text: &str) { + self.log_read(rel, true); + self.recognize_text(rel, text); + } + fn log_read(&self, rel: &str, readable: bool) { self.read .lock() @@ -2755,8 +2783,15 @@ impl Discovery { /// `source "" do` block still declares the gem /// ([`gem::gemfile_source_block_pins`]): live — bundler re-resolves /// from it whatever the lock says; - /// 1. a lock inventory entry for this purl whose resolved url carries - /// the uuid: live; + /// 1. a lock inventory entry for this purl whose resolved url is a + /// Socket-hosted url for THIS uuid + /// ([`crate::patch::redirect::hosted_patch_uuid`] over Socket's host + /// and [`Discovery::patch_server_origins`]): live. Failing that, an + /// entry whose url has the hosted shape for this uuid on an origin + /// this run was not given (a patch server an earlier run was pointed + /// at with `--patch-server-url`, or a host that merely spells the + /// uuid): [`RecordLiveness::Undecided`] — never attested, never + /// judged dead; /// 2. a lock inventory entry that resolves the purl from somewhere ELSE /// (a registry url, a crates.io-checksummed Cargo.lock source): dead /// — the lock positively says the build fetches the unpatched @@ -2771,6 +2806,9 @@ impl Discovery { /// ([`inventory_project_every_lock`] — step 2 is negative evidence, so a /// collapsed view that dropped the hosted instance must not feed it), /// loaded on first need and shared across calls. + /// + /// The attestation-grade verdict: [`Self::redirect_record_verdict`] + /// is [`RecordLiveness::Live`]. pub async fn redirect_record_live( &self, root: &Path, @@ -2779,8 +2817,23 @@ impl Discovery { recorded_files: &[&str], inventory: &mut Option>, ) -> bool { + self.redirect_record_verdict(root, purl, uuid, recorded_files, inventory) + .await + .attests() + } + + /// [`Self::redirect_record_live`]'s three-way verdict (see + /// [`RecordLiveness`]). + pub async fn redirect_record_verdict( + &self, + root: &Path, + purl: &str, + uuid: &str, + recorded_files: &[&str], + inventory: &mut Option>, + ) -> RecordLiveness { if let Some(live) = self.hosted_claim(purl, uuid) { - return live; + return RecordLiveness::from_bool(live); } // Bundler resolves from the Gemfile / `gems.rb`: when a `source // "" do` block there still pins the uuid but the @@ -2805,7 +2858,7 @@ impl Discovery { continue; }; if gem::gemfile_source_block_pins(&text, uuid, Some(&gem_name)) { - return true; + return RecordLiveness::Live; } } } @@ -2823,19 +2876,73 @@ impl Discovery { .iter() .filter(|e| lookup(std::slice::from_ref(*e), purl).is_some()) .collect(); - if entries - .iter() - .any(|e| e.resolved.as_deref().is_some_and(|r| r.contains(uuid))) - { - return true; + let resolved = || entries.iter().filter_map(|e| e.resolved.as_deref()); + if resolved().any(|r| { + crate::patch::redirect::hosted_patch_uuid(r, &self.patch_server_origins).as_deref() + == Some(uuid) + }) { + return RecordLiveness::Live; + } + if resolved().any(|r| foreign_hosted_uuid(r).as_deref() == Some(uuid)) { + return RecordLiveness::Undecided; } if entries.iter().any(|e| resolves_elsewhere(e)) { - return false; + return RecordLiveness::Dead; } - hosted_wiring_in_files(root, &recorded, uuid).await + RecordLiveness::from_bool(hosted_wiring_in_files(root, &recorded, uuid).await) } } +/// A redirect-ledger record's liveness ([`Discovery::redirect_record_verdict`]). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RecordLiveness { + /// The build resolves the package from the record's patch. + Live, + /// A lock entry resolves the package from a url with the hosted shape + /// for the record's patch, on an origin this run does not accept as a + /// patch server: perhaps the operator's own server, configured on an + /// earlier run (`--patch-server-url`) but not this one; perhaps a host + /// that merely spells the uuid. Never attested, never judged dead — + /// what keeps the wiring (the hosted-wiring advisories) keeps it. + Undecided, + /// The build does not resolve the package from the record's patch. + Dead, +} + +impl RecordLiveness { + fn from_bool(live: bool) -> Self { + if live { + Self::Live + } else { + Self::Dead + } + } + + /// Whether the record may be attested. + pub fn attests(self) -> bool { + self == Self::Live + } + + /// Whether the record's wiring is to be kept (not judged gone). + pub fn retained(self) -> bool { + self != Self::Dead + } +} + +/// The patch uuid `url` names when read as a hosted url of its OWN origin +/// ([`crate::patch::redirect::hosted_patch_uuid`]'s grammar — no userinfo, +/// the last canonical-uuid path segment — with the url's origin taken as +/// configured): what a lock entry on an origin this run was not given +/// claims to resolve from. +fn foreign_hosted_uuid(url: &str) -> Option { + // Normalized the way `hosted_patch_uuid` reads it, so an escaped, + // percent-encoded or `sparse+` url still yields its origin. + let normalized = crate::patch::redirect::normalized_hosted_url(url)?; + let parsed = reqwest::Url::parse(&normalized).ok()?; + let origin = parsed.origin().ascii_serialization(); + crate::patch::redirect::hosted_patch_uuid(url, &[origin]) +} + /// Whether root-relative `file` is one of `ecosystem`'s lockfiles — what an /// install of that ecosystem resolves from: a [`ROOT`]-role row of the /// format registry by basename (`Cargo.lock`, `composer.lock`, `go.mod`, @@ -2910,10 +3017,24 @@ impl<'a> LedgerLiveness<'a> { } } - /// [`Discovery::redirect_record_live`] for this project state. + /// [`Discovery::redirect_record_live`] for this project state: whether + /// the record may be ATTESTED. pub async fn redirect_record(&mut self, purl: &str, uuid: &str) -> bool { + self.redirect_record_verdict(purl, uuid).await.attests() + } + + /// Whether the record's hosted wiring is still in place for a caller + /// that keeps or reports wiring rather than attesting it + /// ([`RecordLiveness::retained`]: an [`RecordLiveness::Undecided`] + /// record is kept). + pub async fn redirect_record_retained(&mut self, purl: &str, uuid: &str) -> bool { + self.redirect_record_verdict(purl, uuid).await.retained() + } + + /// [`Discovery::redirect_record_verdict`] for this project state. + pub async fn redirect_record_verdict(&mut self, purl: &str, uuid: &str) -> RecordLiveness { self.discovery - .redirect_record_live( + .redirect_record_verdict( self.root, purl, uuid, @@ -2929,9 +3050,10 @@ impl<'a> LedgerLiveness<'a> { } } -/// A lock inventory entry (already known NOT to carry the patch uuid in its -/// resolved url) that positively resolves from a non-patch source: an -/// explicit artifact url, or a crates.io Cargo.lock source (the registry +/// A lock inventory entry (already known NOT to resolve from the record's +/// patch: its resolved url, if any, is no Socket-hosted url for that uuid) +/// that positively resolves from somewhere else: an explicit artifact url +/// (another host, or another patch), or a crates.io Cargo.lock source (the registry /// view's [`SourceKind::CratesIo`]; Socket sparse sources carry no url in /// the inventory and so prove nothing either way). An entry with no url is /// the conventional registry for most formats but is also how the inventory @@ -3501,6 +3623,23 @@ mod tests { use super::testing::*; use super::*; + /// A foreign hosted url reads its origin after the same normalization + /// `hosted_patch_uuid` applies: `\\/` escapes, a percent-encoded url + /// and a Cargo `sparse+` / `registry+` prefix. + #[test] + fn foreign_hosted_uuid_normalizes_like_hosted_patch_uuid() { + const U: &str = "11111111-2222-4333-8444-555555555555"; + for url in [ + format!("https://mirror.example/patch/{U}/x.tgz"), + format!("https:\\/\\/mirror.example\\/patch\\/{U}\\/x.tgz"), + format!("https%3A%2F%2Fmirror.example%2Fpatch%2F{U}%2Fx.tgz"), + format!("sparse+https://mirror.example/patch/{U}/"), + format!("registry+https://mirror.example/patch/{U}/"), + ] { + assert_eq!(foreign_hosted_uuid(&url).as_deref(), Some(U), "{url}"); + } + } + /// The read-only parse [`toml_or_diag`] hands extractors reads every /// value the way the `DocumentMut` it replaced did (#1473): escaped, /// literal and multi-line strings decode, quoted and dotted keys @@ -3900,12 +4039,13 @@ mod tests { } /// REGRESSION: step 2's negative evidence reads EVERY lock's instance. - /// `uv.lock` routes six to a hosted wheel on a patch host outside the - /// discovery allowlist (no `hosted_claim`), and a PEP 723 script lock - /// that sorts first resolves the same six from PyPI. The collapsed - /// inventory keeps only the script lock's instance, which made the - /// record look resolved-elsewhere (dead) although the project lock still - /// installs the patch. + /// `uv.lock` routes six to a hosted wheel on the operator's configured + /// patch server, which a discovery that holds no claim on the uuid + /// classifies with the run's origins, and a PEP 723 script lock that + /// sorts first resolves the same six from PyPI. The collapsed inventory + /// keeps only the script lock's instance, which made the record look + /// resolved-elsewhere (dead) although the project lock still installs + /// the patch. #[tokio::test] async fn a_script_lock_registry_instance_does_not_veto_the_hosted_project_lock() { let p = Project::new(); @@ -3964,6 +4104,21 @@ mod tests { None, "host not allowlisted" ); + // Without the origin, the internal host's url is undecided: never + // attested, and — the regression — not vetoed by the script lock's + // registry instance either. + let mut inventory = None; + assert_eq!( + discovery + .redirect_record_verdict(root, purl, UUID_A, &["uv.lock"], &mut inventory) + .await, + RecordLiveness::Undecided, + "an unconfigured patch host is never attested, nor judged dead" + ); + let discovery = Discovery { + patch_server_origins: vec!["https://patches.internal.example".to_string()], + ..Discovery::default() + }; let mut inventory = None; assert!( discovery @@ -4096,6 +4251,103 @@ mod tests { assert!(!hosted_wiring_in_files(root, &["package-lock.json"], "not-a-uuid").await); } + /// REGRESSION (P6): step 1 classifies a lock entry's resolved url with + /// the one origin rule ([`crate::patch::redirect::hosted_patch_uuid`]), + /// never by substring. Only a Socket-host url or a configured origin's + /// url for the record's uuid is LIVE (attested). A url with the hosted + /// shape for the uuid on an origin this run was not given — a patch + /// server an earlier run was pointed at, or a host that merely spells + /// it — is UNDECIDED: never attested, but its wiring is retained. A url + /// that only contains the uuid (a file name, a query) and a Socket url + /// for ANOTHER patch (whose path carries the uuid as its grant token) + /// resolve the package from somewhere else: DEAD. + #[tokio::test] + async fn liveness_matches_the_resolved_url_by_origin_not_substring() { + use RecordLiveness::{Dead, Live, Undecided}; + let origin = "http://127.0.0.1:4545"; + let purl = "pkg:npm/x@1.0.0"; + let elsewhere = + format!("https://evil.example/patch/npm/x/1.0.0/{TOKEN}/{UUID_A}/x-1.0.0.tgz"); + let in_name = format!("https://evil.example/files/x-{UUID_A}.tgz"); + let in_query = format!("https://evil.example/files/x-1.0.0.tgz?id={UUID_A}"); + let other_patch = + format!("https://patch.socket.dev/patch/npm/x/1.0.0/{UUID_A}/{UUID_B}/x-1.0.0.tgz"); + let socket = hosted_url("npm", "x", "1.0.0", UUID_A, "x-1.0.0.tgz"); + let configured = format!("{origin}/patch/npm/x/1.0.0/{TOKEN}/{UUID_A}/x-1.0.0.tgz"); + let with_origin = Discovery { + patch_server_origins: vec![origin.to_string()], + ..Discovery::default() + }; + let without = Discovery::default(); + for (resolved, discovery, verdict) in [ + (&elsewhere, &with_origin, Undecided), + (&configured, &without, Undecided), + (&in_name, &with_origin, Dead), + (&in_query, &with_origin, Dead), + (&other_patch, &with_origin, Dead), + (&socket, &without, Live), + (&configured, &with_origin, Live), + ] { + let p = Project::new(); + p.write( + "package-lock.json", + format!( + r#"{{"lockfileVersion":3,"packages":{{"":{{}},"node_modules/x":{{"version":"1.0.0","resolved":"{resolved}"}}}}}}"# + ), + ); + let why = format!("{resolved} (origins {:?})", discovery.patch_server_origins); + let mut inventory = None; + assert_eq!( + discovery + .redirect_record_verdict( + p.root(), + purl, + UUID_A, + &["package-lock.json"], + &mut inventory, + ) + .await, + verdict, + "{why}" + ); + let mut inventory = None; + assert_eq!( + discovery + .redirect_record_live( + p.root(), + purl, + UUID_A, + &["package-lock.json"], + &mut inventory, + ) + .await, + verdict == Live, + "only a live record attests: {why}" + ); + let mut liveness = LedgerLiveness::new(p.root(), discovery, None); + assert_eq!( + liveness.redirect_record(purl, UUID_A).await, + verdict.attests(), + "{why}" + ); + assert_eq!( + liveness.redirect_record_retained(purl, UUID_A).await, + verdict != Dead, + "{why}" + ); + } + } + + /// Discovery records the origins it classified with, for liveness. + #[tokio::test] + async fn discovery_carries_its_patch_server_origins() { + let p = Project::new().with_origin("http://127.0.0.1:4545"); + assert_eq!( + p.discover().await.patch_server_origins, + vec!["http://127.0.0.1:4545".to_string()] + ); + } + /// [`LedgerLiveness`]: the redirect ledger's edit files are sorted and /// deduplicated, verdicts are the [`Discovery`] rule's, and the lock /// inventory loads only when a verdict first needs it and then once per diff --git a/crates/socket-patch-core/src/vex/discover/nuget.rs b/crates/socket-patch-core/src/vex/discover/nuget.rs index 584b727bb..3393fdc52 100644 --- a/crates/socket-patch-core/src/vex/discover/nuget.rs +++ b/crates/socket-patch-core/src/vex/discover/nuget.rs @@ -491,6 +491,7 @@ fn emit_hosted( uuid: src.uuid.clone(), file: cfg_rel.into(), version_reqs: Vec::new(), + excluded_reqs: Vec::new(), index_url: Some(src.value.clone()), }); } diff --git a/crates/socket-patch-core/src/vex/discover/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index 54509be0e..464167b72 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -137,6 +137,8 @@ fn render(out: &Discovery, root: &Path) -> Value { unwired_copies: _, // Already folded into `unpatched_copies` by the time a run returns. unidentified_copies: _, + // The run's input (the golden runs with none), not a finding. + patch_server_origins: _, } = out; let render_ref = |r: &PatchedRef| { let PatchedRef { @@ -195,15 +197,22 @@ fn render(out: &Discovery, root: &Path) -> Value { uuid, file, version_reqs, + excluded_reqs, index_url: _, } = p; - json!({ + let mut pin = json!({ "ecosystem": ecosystem, "name": name, "uuid": uuid, "file": path_str(file), "version_reqs": version_reqs, - }) + }); + // Rendered only when present, so goldens without any keep + // their shape. + if !excluded_reqs.is_empty() { + pin["excluded_reqs"] = json!(excluded_reqs); + } + pin }) .collect(); let elsewhere_rendered: Vec = elsewhere