From 43fd5aa0354c8a1cd9477e5e6871ace26ac6a200 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:03:21 -0600 Subject: [PATCH 1/5] Match ledger liveness uuids by origin, not substring Redirect-ledger liveness step 1 kept a record alive when any lock inventory entry's resolved url merely CONTAINED the patch uuid, so a url on any host spelling the uuid (or a Socket url for another patch whose grant token is that uuid) kept a dead hosted record alive. Step 1 now classifies the url with the one origin classifier, `patch::redirect::hosted_patch_uuid`, over Socket's host and the run's patch-server origins, and requires the classified uuid to equal the record's. Discovery records the origins it was run with (`Discovery::patch_server_origins`, set in `discover_with_ctx`), so liveness classifies with exactly the origins discovery did; every `LedgerLiveness` caller already holds a discovery built from the run's `DiscoverOptions`. Socket-host and configured-origin urls for the record's uuid stay live; anything else falls to step 2 (resolves elsewhere: dead). The uv script-lock regression test relied on the substring match for an unconfigured internal host; it now shows that host dead without the origin and live with it. Co-Authored-By: Claude Opus 5.5 --- .../socket-patch-core/src/vex/discover/mod.rs | 121 +++++++++++++++--- .../src/vex/discover/testing/golden.rs | 2 + 2 files changed, 108 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 815d4644d..6f7398d55 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -760,6 +760,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 +1550,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 } @@ -2755,8 +2762,11 @@ 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. A url on any other + /// host that merely contains the uuid is not a patch reference; /// 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 @@ -2823,10 +2833,12 @@ 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))) - { + if entries.iter().any(|e| { + e.resolved.as_deref().is_some_and(|r| { + crate::patch::redirect::hosted_patch_uuid(r, &self.patch_server_origins).as_deref() + == Some(uuid) + }) + }) { return true; } if entries.iter().any(|e| resolves_elsewhere(e)) { @@ -2929,9 +2941,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 @@ -3900,12 +3913,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 +3978,19 @@ mod tests { None, "host not allowlisted" ); + // Without the origin, the internal host's url is no patch reference + // however much of the uuid it spells: dead. + let mut inventory = None; + assert!( + !discovery + .redirect_record_live(root, purl, UUID_A, &["uv.lock"], &mut inventory) + .await, + "a uuid on an unconfigured host keeps nothing alive" + ); + let discovery = Discovery { + patch_server_origins: vec!["https://patches.internal.example".to_string()], + ..Discovery::default() + }; let mut inventory = None; assert!( discovery @@ -4096,6 +4123,70 @@ 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. A url on another host that merely spells the + /// record's uuid — even in Socket's own path shape — resolves the + /// package from somewhere else (step 2: dead), as does a Socket url for + /// ANOTHER patch whose path also carries the uuid (as its grant + /// token). A Socket-host url and a configured origin's url for the + /// record's uuid stay live. + #[tokio::test] + async fn liveness_matches_the_resolved_url_by_origin_not_substring() { + 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 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() + }; + for (resolved, origins_discovery, live) in [ + (&elsewhere, &with_origin, false), + (&other_patch, &with_origin, false), + (&configured, &Discovery::default(), false), + (&socket, &Discovery::default(), true), + (&configured, &with_origin, true), + ] { + let p = Project::new(); + p.write( + "package-lock.json", + format!( + r#"{{"lockfileVersion":3,"packages":{{"":{{}},"node_modules/x":{{"version":"1.0.0","resolved":"{resolved}"}}}}}}"# + ), + ); + let mut inventory = None; + assert_eq!( + origins_discovery + .redirect_record_live( + p.root(), + purl, + UUID_A, + &["package-lock.json"], + &mut inventory, + ) + .await, + live, + "{resolved} (origins {:?})", + origins_discovery.patch_server_origins + ); + } + } + + /// 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/testing/golden.rs b/crates/socket-patch-core/src/vex/discover/testing/golden.rs index 54509be0e..6b14d0c4a 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 { From 1a043ece081ee467611c4f314beddfe3e64b7890 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:03:42 -0600 Subject: [PATCH 2/5] Read Cargo workspace members in VEX discovery Hosted mode pins a patched crate in the root Cargo.toml AND every workspace-member manifest that declares it, and vendored mode's live-redirect refusal reads member manifests too. Discovery read only the root manifest, so a crate the root does not declare was "the lock is the only word": a member whose declaration was reverted to crates.io (no `registry = "socket-patch-"`) left a ref attested although cargo re-resolves that dependency from crates.io. Discovery now enumerates the members the way the hosted rewriter does (`cargo_workspace::member_manifests_in`, filtered by the rewriter's own `is_cargo_member_manifest_key`, so nothing under `.socket/` or `target/`) and applies its manifest-declaration check across root + members. Each declaration carries its file: diagnostics, the conflict-pair Cargo.toml ref and lockless `UnlockedPin`s name the member manifest that holds the pin. `[patch]` stays root-only (cargo ignores a member's). The member walk over a `DiskSnapshot` now notes every manifest it reads and directory it probes or lists with the snapshot instead of asking for its raw root, so a discovery run under a read-set recording (the re-scan cache) keeps the recording usable. Co-Authored-By: Claude Opus 5.5 --- .../src/patch/redirect/mod.rs | 2 +- .../src/utils/cargo_workspace.rs | 77 +++- .../src/vendor/lock_inventory/view.rs | 6 +- .../src/vex/discover/cargo.rs | 345 +++++++++++++++--- 4 files changed, 374 insertions(+), 56 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0c778ac74..f78b4e356 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -2415,7 +2415,7 @@ fn crlf_to_lf(text: &str) -> 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; }; diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index db99948a0..a7ec90826 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -18,7 +18,7 @@ use std::sync::Arc; use toml_edit::{DocumentMut, Item, Table}; use crate::utils::workspace_globs::segment_glob_matches; -use crate::vendor::lock_inventory::view::{MemoryProject, ProjectView}; +use crate::vendor::lock_inventory::view::{DiskSnapshot, MemoryProject, ProjectView}; use crate::vendor::parse_memo::ParseMemo; /// Upper bound on discovered manifests — a runaway glob (or a hostile tree) @@ -36,16 +36,47 @@ pub fn member_manifests(root: &Path) -> Vec { /// 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). +/// +/// Over a [`DiskSnapshot`] the walk reads the disk, but notes every +/// manifest it reads and every directory it probes or lists with the +/// snapshot, so a recording read set covers the walk (asking for the +/// snapshot's raw root would end the recording). 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) - } + ProjectView::Disk(root) => member_manifests(root), + ProjectView::Snapshot(snap) => member_manifests_with(&SnapshotTree(snap)), ProjectView::Memory(project) => member_manifests_with(&MemoryTree(project)), } } +/// The disk under a [`DiskSnapshot`], each access noted with it first. +struct SnapshotTree<'a, 'b>(&'a DiskSnapshot<'b>); + +impl SnapshotTree<'_, '_> { + fn disk(&self) -> DiskTree<'_> { + DiskTree(self.0.root_reading(std::iter::empty::<&str>())) + } +} + +impl Tree for SnapshotTree<'_, '_> { + fn read_manifest(&self, rel: &str) -> Option> { + self.0.touch(rel); + self.disk().read_manifest(rel) + } +} + +impl DirTree for SnapshotTree<'_, '_> { + fn is_real_dir(&self, rel: &str) -> bool { + self.0.touch(rel); + self.disk().is_real_dir(rel) + } + + fn child_dirs(&self, rel: &str) -> Option> { + self.0.touch_listing(rel); + self.disk().child_dirs(rel) + } +} + /// The two directory questions a glob walk asks, keyed by `/`-separated /// root-relative paths (`""` is the root). Shared with the pnpm workspace /// member walk ([`crate::utils::pnpm_workspace`]). @@ -478,6 +509,42 @@ mod tests { format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\n") } + /// Over a tracked snapshot the walk answers as on disk and keeps the + /// recording usable: it notes what it reads and lists, so a member + /// added under a glob afterwards changes the read set. + #[test] + fn snapshot_walk_records_what_it_reads() { + 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")); + // Files stamped just now are racy: the walk reads around the + // snapshot's content cache, so such a read set never holds. + let old = std::time::SystemTime::now() - std::time::Duration::from_secs(3600); + for rel in ["Cargo.toml", "crates/a/Cargo.toml"] { + std::fs::File::options() + .write(true) + .open(root.join(rel)) + .unwrap() + .set_modified(old) + .unwrap(); + } + let snap = DiskSnapshot::tracked(root); + snap.begin_recording(); + assert_eq!( + member_manifests_in(&ProjectView::Snapshot(&snap)), + member_manifests(root) + ); + let reads = snap.end_recording().expect("the walk keeps the recording"); + assert!(reads.unchanged()); + write(root, "crates/b/Cargo.toml", &pkg("b")); + assert!(!reads.unchanged(), "a new member dir changes the listing"); + } + #[test] fn members_globs_excludes_and_path_dependencies() { let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/view.rs b/crates/socket-patch-core/src/vendor/lock_inventory/view.rs index c55ec4549..0383d870f 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/view.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/view.rs @@ -501,12 +501,14 @@ impl<'a> DiskSnapshot<'a> { } /// Note that `rel` is about to be read or probed (see [`Self::tracked`]). - fn touch(&self, rel: &str) { + /// Crate-visible for a walk the view does not mediate that reads only + /// what it touches (the cargo workspace member walk). + pub(crate) fn touch(&self, rel: &str) { self.touch_as(rel, Access::Probe); } /// Note that directory `rel` is about to be listed. - fn touch_listing(&self, rel: &str) { + pub(crate) fn touch_listing(&self, rel: &str) { self.touch_as(rel, Access::List); } diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 40e6ab008..81e6a1032 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): @@ -147,27 +152,42 @@ 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 config = read_config(ctx, out).await; + // The root manifest, then every workspace member's — the manifests the + // hosted rewriter pins a crate in (each that declares it). + let manifests: Vec<(String, DocumentMut)> = match &manifest { + Some(root) => { + let mut manifests = vec![(CARGO_TOML.to_string(), root.clone())]; + manifests.extend(read_members(ctx, out).await); + manifests + } + None => Vec::new(), + }; - // Crate name → the `registry` value of each root-manifest declaration - // (`None` = declared with no registry key). Absent map = no usable - // manifest. + // Crate name → each declaration of it in the root or a member manifest + // (its file and `registry` value; `None` = declared with no registry + // key). Absent map = no usable root manifest. let decls = manifest.as_ref().map(|doc| { - let mut decls = manifest_declarations(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))); } } 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))); } } } @@ -180,8 +200,8 @@ 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, &manifests, &definitions, out); } if let Some(doc) = &manifest { // Cargo drops a manifest crates.io `[patch]` item that the project @@ -311,6 +331,25 @@ 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/`). Missing / unparseable ones +/// are skipped (the latter diagnosed). +async fn read_members(ctx: &DiscoverCtx<'_>, out: &mut Discovery) -> Vec<(String, DocumentMut)> { + let mut members = Vec::new(); + for rel in crate::utils::cargo_workspace::member_manifests_in(&ctx.view) { + if !crate::patch::redirect::is_cargo_member_manifest_key(&rel) { + continue; + } + if let Some(doc) = read_toml(ctx, &rel, out).await { + members.push((rel, doc)); + } + } + members +} + 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 +394,54 @@ 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, +} + +impl Decl { + fn new(file: &str, registry: Option<&str>) -> Self { + Decl { + file: file.to_string(), + registry: registry.map(str::to_string), + } + } +} + +/// 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())); + } } decls } +/// The distinct files among `decls`, in order, joined for a message. +fn decl_files(decls: &[Decl]) -> 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 +511,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 +562,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, @@ -592,30 +676,37 @@ fn unpatched_twins(ctx: &DiscoverCtx<'_>, lock: &Lock, out: &mut Discovery) { /// 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 +/// carrying the manifests' 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). fn unresolved_manifest_pins( ctx: &DiscoverCtx<'_>, lock: &Lock, - decls: &BTreeMap>>, - manifest: &DocumentMut, + decls: &Decls, + manifests: &[(String, DocumentMut)], definitions: &BTreeMap, out: &mut Discovery, ) { - let deps = dependency_entries(manifest); + let deps: Vec = manifests + .iter() + .flat_map(|(_, doc)| dependency_entries(doc)) + .collect(); for (name, regs) in decls { - for reg in regs.iter().flatten() { + for decl in regs { + let Some(reg) = decl.registry.as_ref() else { + continue; + }; + let file = decl.file.as_str(); 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, + file, format!( - "{CARGO_TOML}: {name} is pinned to registry {reg:?}, which is not a \ + "{file}: {name} is pinned to registry {reg:?}, which is not a \ socket-patch- name" ), ); @@ -634,14 +725,14 @@ fn unresolved_manifest_pins( let why = match lock { Lock::Unparseable => continue, // already diagnosed Lock::Absent => { - let only_this_registry = regs.iter().all(|r| r.as_deref() == Some(reg)); + let only_this_registry = regs.iter().all(|d| d.registry.as_ref() == Some(reg)); let defined_here = definitions .get(reg.as_str()) .and_then(|index| source_uuid(ctx, index)) .is_some_and(|u| u == uuid); let version_reqs: Vec = deps .iter() - .filter(|d| d.name == *name && d.registry.as_deref() == Some(reg)) + .filter(|d| d.name == *name && d.registry.as_ref() == Some(reg)) .filter_map(|d| d.version.clone()) .collect(); if only_this_registry && defined_here && !version_reqs.is_empty() { @@ -649,7 +740,7 @@ fn unresolved_manifest_pins( 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(), }); @@ -664,8 +755,8 @@ fn unresolved_manifest_pins( }; out.diag( DIAG_REF_UNATTRIBUTABLE, - CARGO_TOML, - format!("{CARGO_TOML}: {name} is pinned to Socket patch {uuid}, but {why}"), + file, + format!("{file}: {name} is pinned to Socket patch {uuid}, but {why}"), ); } } @@ -1362,6 +1453,164 @@ 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 rotated grant token in the definition is the same patch. #[tokio::test] async fn rotated_token_in_the_definition_still_agrees() { From 650143ef4d0264e04a97a2168a39d2680f4ad90f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:46:05 -0600 Subject: [PATCH 3/5] Address review: view-mediated member walk, lockless pins, undecided liveness Cargo member walk (review 4, 6, 7): - `cargo_workspace::member_manifests_in` is now async over the ProjectView: manifests are read with `read_text` (concurrently, bounded, ordered), globs list with `list_dir`, and directory probes use the view's own mediated `is_dir` / `is_symlink`. A snapshot's overlays and content cache apply and its recording notes the walk, so fresh manifests (a `.socket/` copy followed through `[patch]` included) no longer leave a racy read set. `DiskSnapshot::touch` / `touch_listing` are private again; `SnapshotTree` is gone. The in-memory engine keeps the sync walk (`member_manifests_in_memory`); disk-only callers keep `member_manifests`. A test pins the view walk to the disk walk over globs, `**`, excludes, path dependencies, `[patch]` copies, dot / `target` dirs and a symlinked member. Discovery (review 5, 8): - Members are walked only when they can matter: a lock with a Socket-sourced package, or no lock (then parsed only when some manifest carries a `socket-patch-` pin). The walk's texts are adopted into the ctx's read log and rule-11 recognition instead of being read again. - The root `DocumentMut` is moved into the manifest list, not cloned. Lockless pins (review 1, 3): - `UnlockedPin` gains `excluded_reqs`: the requirements of the crate's unpinned declarations. The rewriter leaves a declaration alone only when its requirement excludes the patched version, so a member's `serde = "0.9"` beside a pinned `serde = "1"` no longer suppresses the pin; a ledger version must satisfy every pinned requirement and no excluded one. An unpinned declaration with no requirement still blocks. - One pin and one diagnostic per (crate, patch): the union of the pinning declarations' requirements, every pinning manifest listed once. Liveness (review 2): - A lock entry whose url has the hosted shape for the record's uuid on an origin this run was not given is `RecordLiveness::Undecided`: never attested (`redirect_record_live` / `LedgerLiveness::redirect_record` stay attestation-grade) and never judged dead. `scan`'s takeover classifier and hosted-wiring-retained advisory use the new `LedgerLiveness::redirect_record_retained`, so a record written against a patch server a later run is not pointed at keeps its wiring. A url that merely contains the uuid (file name, query) is still dead. Co-Authored-By: Claude Opus 5.5 --- .../socket-patch-cli/src/commands/scan/mod.rs | 15 +- crates/socket-patch-core/src/hosted/engine.rs | 2 +- .../src/hosted/memory/mod.rs | 4 +- .../src/utils/cargo_workspace.rs | 373 ++++++++++++---- .../src/vendor/lock_inventory/view.rs | 6 +- .../src/vex/discover/cargo.rs | 398 ++++++++++++++---- .../socket-patch-core/src/vex/discover/mod.rs | 231 ++++++++-- .../src/vex/discover/nuget.rs | 1 + .../src/vex/discover/testing/golden.rs | 11 +- 9 files changed, 827 insertions(+), 214 deletions(-) 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 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). -/// -/// Over a [`DiskSnapshot`] the walk reads the disk, but notes every -/// manifest it reads and every directory it probes or lists with the -/// snapshot, so a recording read set covers the walk (asking for the -/// snapshot's raw root would end the recording). -pub fn member_manifests_in(view: &ProjectView<'_>) -> Vec { - match view { - ProjectView::Disk(root) => member_manifests(root), - ProjectView::Snapshot(snap) => member_manifests_with(&SnapshotTree(snap)), - ProjectView::Memory(project) => member_manifests_with(&MemoryTree(project)), - } +/// [`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() } -/// The disk under a [`DiskSnapshot`], each access noted with it first. -struct SnapshotTree<'a, 'b>(&'a DiskSnapshot<'b>); +/// In-flight manifest reads of one [`member_manifest_texts_in`] level. +const MEMBER_READ_CONCURRENCY: usize = 16; -impl SnapshotTree<'_, '_> { - fn disk(&self) -> DiskTree<'_> { - DiskTree(self.0.root_reading(std::iter::empty::<&str>())) +/// [`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<(Arc, String)>)> = + 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() } -impl Tree for SnapshotTree<'_, '_> { - fn read_manifest(&self, rel: &str) -> Option> { - self.0.touch(rel); - self.disk().read_manifest(rel) +/// A regular (non-symlink) manifest read through the view: its walk facts +/// and its text. +async fn view_manifest(view: &ProjectView<'_>, rel: &str) -> Option<(Arc, String)> { + 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)) } -impl DirTree for SnapshotTree<'_, '_> { - fn is_real_dir(&self, rel: &str) -> bool { - self.0.touch(rel); - self.disk().is_real_dir(rel) - } +/// [`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) +} - fn child_dirs(&self, rel: &str) -> Option> { - self.0.touch_listing(rel); - self.disk().child_dirs(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)); + } + } + } + stack.extend(next.into_iter().rev()); } + out.sort(); + out.dedup(); + out } /// The two directory questions a glob walk asks, keyed by `/`-separated @@ -289,28 +417,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; @@ -435,6 +571,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, @@ -459,13 +612,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 { @@ -509,42 +656,124 @@ mod tests { format!("[package]\nname = \"{name}\"\nversion = \"0.1.0\"\n") } - /// Over a tracked snapshot the walk answers as on disk and keeps the - /// recording usable: it notes what it reads and lists, so a member - /// added under a glob afterwards changes the read set. - #[test] - fn snapshot_walk_records_what_it_reads() { + /// 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", + "[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")); - // Files stamped just now are racy: the walk reads around the - // snapshot's content cache, so such a read set never holds. - let old = std::time::SystemTime::now() - std::time::Duration::from_secs(3600); - for rel in ["Cargo.toml", "crates/a/Cargo.toml"] { - std::fs::File::options() - .write(true) - .open(root.join(rel)) - .unwrap() - .set_modified(old) - .unwrap(); - } - let snap = DiskSnapshot::tracked(root); - snap.begin_recording(); - assert_eq!( - member_manifests_in(&ProjectView::Snapshot(&snap)), - member_manifests(root) + 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()); + 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/vendor/lock_inventory/view.rs b/crates/socket-patch-core/src/vendor/lock_inventory/view.rs index 0383d870f..c55ec4549 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/view.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/view.rs @@ -501,14 +501,12 @@ impl<'a> DiskSnapshot<'a> { } /// Note that `rel` is about to be read or probed (see [`Self::tracked`]). - /// Crate-visible for a walk the view does not mediate that reads only - /// what it touches (the cargo workspace member walk). - pub(crate) fn touch(&self, rel: &str) { + fn touch(&self, rel: &str) { self.touch_as(rel, Access::Probe); } /// Note that directory `rel` is about to be listed. - pub(crate) fn touch_listing(&self, rel: &str) { + fn touch_listing(&self, rel: &str) { self.touch_as(rel, Access::List); } diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 81e6a1032..3139998ec 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -150,23 +150,22 @@ 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; // The root manifest, then every workspace member's — the manifests the // hosted rewriter pins a crate in (each that declares it). - let manifests: Vec<(String, DocumentMut)> = match &manifest { - Some(root) => { - let mut manifests = vec![(CARGO_TOML.to_string(), root.clone())]; - manifests.extend(read_members(ctx, out).await); - manifests - } - None => Vec::new(), - }; + 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 and `registry` value; `None` = declared with no registry - // key). Absent map = no usable root manifest. - let decls = manifest.as_ref().map(|doc| { + // (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. @@ -176,7 +175,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { decls .entry(name) .or_default() - .push(Decl::new(CARGO_TOML, Some(reg))); + .push(Decl::new(CARGO_TOML, Some(reg), None)); } } decls @@ -187,7 +186,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { 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(Decl::new(file, Some(reg))); + existing.push(Decl::new(file, Some(reg), None)); } } } @@ -201,9 +200,9 @@ 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) = &decls { - unresolved_manifest_pins(ctx, &lock, decls, &manifests, &definitions, out); + 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]` @@ -243,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(); @@ -335,21 +334,64 @@ async fn read_toml(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discovery) -> Op /// 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/`). Missing / unparseable ones -/// are skipped (the latter diagnosed). -async fn read_members(ctx: &DiscoverCtx<'_>, out: &mut Discovery) -> Vec<(String, DocumentMut)> { +/// 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 in crate::utils::cargo_workspace::member_manifests_in(&ctx.view) { - if !crate::patch::redirect::is_cargo_member_manifest_key(&rel) { - continue; - } - if let Some(doc) = read_toml(ctx, &rel, out).await { + 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) } @@ -400,15 +442,32 @@ async fn read_config( 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>) -> Self { + 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`]). @@ -422,17 +481,29 @@ 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())); + 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(decls: &[Decl]) -> String { +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()) { @@ -668,81 +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 manifests' 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: &Decls, - manifests: &[(String, DocumentMut)], definitions: &BTreeMap, out: &mut Discovery, ) { - let deps: Vec = manifests - .iter() - .flat_map(|(_, doc)| dependency_entries(doc)) - .collect(); for (name, regs) in decls { - for decl in regs { - let Some(reg) = decl.registry.as_ref() else { - continue; - }; - let file = decl.file.as_str(); - if !reg.trim().starts_with(PIN_NAME_PREFIX) { - continue; - } - let Some(uuid) = socket_patch_name_uuid(reg, false) else { - out.diag( - DIAG_REF_INVALID, - file, - format!( - "{file}: {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(|d| d.registry.as_ref() == 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_ref() == 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: 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") @@ -753,10 +854,11 @@ fn unresolved_manifest_pins( --patch-server-url)" ), }; + let files = decl_files(pinning.iter().copied()); out.diag( DIAG_REF_UNATTRIBUTABLE, file, - format!("{file}: {name} is pinned to Socket patch {uuid}, but {why}"), + format!("{files}: {name} is pinned to Socket patch {uuid}, but {why}"), ); } } @@ -1611,6 +1713,138 @@ mod tests { ); } + /// 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 6f7398d55..9b128f8b1 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)) } } @@ -1654,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() @@ -2765,8 +2786,12 @@ impl Discovery { /// 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. A url on any other - /// host that merely contains the uuid is not a patch reference; + /// 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 @@ -2781,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, @@ -2789,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 @@ -2815,7 +2858,7 @@ impl Discovery { continue; }; if gem::gemfile_source_block_pins(&text, uuid, Some(&gem_name)) { - return true; + return RecordLiveness::Live; } } } @@ -2833,21 +2876,70 @@ 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| { - crate::patch::redirect::hosted_patch_uuid(r, &self.patch_server_origins).as_deref() - == Some(uuid) - }) + 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 true; + 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 { + let parsed = reqwest::Url::parse(url.trim()).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`, @@ -2922,10 +3014,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, @@ -3978,14 +4084,16 @@ mod tests { None, "host not allowlisted" ); - // Without the origin, the internal host's url is no patch reference - // however much of the uuid it spells: dead. + // 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!( - !discovery - .redirect_record_live(root, purl, UUID_A, &["uv.lock"], &mut inventory) + assert_eq!( + discovery + .redirect_record_verdict(root, purl, UUID_A, &["uv.lock"], &mut inventory) .await, - "a uuid on an unconfigured host keeps nothing alive" + 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()], @@ -4125,18 +4233,23 @@ mod tests { /// 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. A url on another host that merely spells the - /// record's uuid — even in Socket's own path shape — resolves the - /// package from somewhere else (step 2: dead), as does a Socket url for - /// ANOTHER patch whose path also carries the uuid (as its grant - /// token). A Socket-host url and a configured origin's url for the - /// record's uuid stay live. + /// 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"); @@ -4145,12 +4258,15 @@ mod tests { patch_server_origins: vec![origin.to_string()], ..Discovery::default() }; - for (resolved, origins_discovery, live) in [ - (&elsewhere, &with_origin, false), - (&other_patch, &with_origin, false), - (&configured, &Discovery::default(), false), - (&socket, &Discovery::default(), true), - (&configured, &with_origin, true), + 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( @@ -4159,9 +4275,24 @@ mod tests { 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!( - origins_discovery + discovery .redirect_record_live( p.root(), purl, @@ -4170,9 +4301,19 @@ mod tests { &mut inventory, ) .await, - live, - "{resolved} (origins {:?})", - origins_discovery.patch_server_origins + 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}" ); } } 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 6b14d0c4a..464167b72 100644 --- a/crates/socket-patch-core/src/vex/discover/testing/golden.rs +++ b/crates/socket-patch-core/src/vex/discover/testing/golden.rs @@ -197,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 From b2e171b5dd0581312bc9d4500df601ccc075a6c1 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 13:51:54 -0600 Subject: [PATCH 4/5] Name the view walk's read-manifest pair (clippy type_complexity) Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-core/src/utils/cargo_workspace.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/utils/cargo_workspace.rs b/crates/socket-patch-core/src/utils/cargo_workspace.rs index 2b0afff17..6aa8e0388 100644 --- a/crates/socket-patch-core/src/utils/cargo_workspace.rs +++ b/crates/socket-patch-core/src/utils/cargo_workspace.rs @@ -98,7 +98,7 @@ pub(crate) async fn member_manifest_texts_in(view: &ProjectView<'_>) -> Vec<(Str level.push(dir); } } - let read: Vec<(String, Option<(Arc, String)>)> = + let read: Vec<(String, Option)> = crate::utils::concurrent::ordered_concurrent( level, MEMBER_READ_CONCURRENCY, @@ -128,9 +128,12 @@ pub(crate) async fn member_manifest_texts_in(view: &ProjectView<'_>) -> Vec<(Str .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<(Arc, String)> { +async fn view_manifest(view: &ProjectView<'_>, rel: &str) -> Option { if view.is_symlink(rel) { return None; } From 94af3e4f0cadeda821cef6af601fc5356eece898 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 10:24:56 +0000 Subject: [PATCH 5/5] Normalize foreign hosted urls before reading their origin foreign_hosted_uuid parsed the raw lock url, while hosted_patch_uuid first unescapes \/, percent-decodes and strips a Cargo sparse+ / registry+ prefix. A hosted url on an unconfigured origin that only parses after that normalization was never Undecided and could be marked Dead, dropping live wiring. Share the normalization. Addresses one of the two Bugbot findings on this PR. Co-Authored-By: Claude --- .../src/patch/redirect/mod.rs | 24 +++++++++++++------ .../socket-patch-core/src/vex/discover/mod.rs | 22 ++++++++++++++++- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index f78b4e356..664eaf807 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -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/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 9b128f8b1..9ed147621 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -2935,7 +2935,10 @@ impl RecordLiveness { /// configured): what a lock entry on an origin this run was not given /// claims to resolve from. fn foreign_hosted_uuid(url: &str) -> Option { - let parsed = reqwest::Url::parse(url.trim()).ok()?; + // 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]) } @@ -3620,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