From 77a4d2d1af23f99cd667e0d7c7779d5ba2061d55 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:56:34 -0600 Subject: [PATCH 1/3] Stop attesting a Cargo pin a workspace member resolves from crates.io Cargo resolves each source on its own. When one declaration of a crate pins it to `socket-patch-` but another (the root's or a workspace member's) names no Socket registry (crates.io, another registry, a path or git) with a requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. Discovery still attested the Cargo.lock ref because "some declaration pins the patch" was enough. The hosted rewriter pins every declaration, so this mixed state is a partial revert or a hand edit. The ref now stays (rollback, remove and the ledgers' liveness still see the pin) but is marked unattested with the new `UnattestedKind::CargoUnpinnedDeclaration`, naming the unpinned declaration's manifest. When Cargo.lock already locks a same-version copy from another source beside the Socket entry, the detail names that lock entry as direct evidence. A declaration whose requirement excludes the patched version, and a `[patch]` registry entry (which redirects declarations rather than adding one), do not withhold the statement. The CLI omits the ref as `vex_cargo_unpinned_declaration`; documented in CLI_CONTRACT.md. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 3 +- .../src/commands/vex_sources.rs | 56 +++ .../src/vex/discover/cargo.rs | 335 +++++++++++++++++- .../socket-patch-core/src/vex/discover/mod.rs | 6 + 4 files changed, 390 insertions(+), 10 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index a3ed909d6..8a612fb66 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Four cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Unpinned Cargo declarations** (`vex_cargo_unpinned_declaration`): cargo resolves each source on its own, so when one declaration of a crate pins it to `socket-patch-` but another — the root `Cargo.toml`'s or a workspace member's, read as hosted scan reads them — names no Socket registry (crates.io, another registry, a path or git) with a version requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. The `Cargo.lock` reference is omitted, naming the unpinned declaration's manifest, and the warning also names a same-version lock entry from another source when the lock already holds one. Hosted scan pins every declaration, so this is a partial revert or a hand edit; re-running `scan` pins a crates.io declaration again. A declaration whose requirement excludes the patched version (cargo resolves it to another version) does not withhold the statement. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2678,6 +2678,7 @@ match it, withholds the statement: | `vex_deno_lock_copy` | run warning and `failed[].reason` | An npm pin is wired, but `deno.lock` locks the same `name@version`, which `deno install` installs without the wiring; no statement while it does. `vendor --check` and `scan` still treat the wiring as live. | | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted Maven pin's GA also has a live executable `` copy (not `sources` / `javadoc`) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_cargo_unpinned_declaration` | run warning and `failed[].reason` | A hosted Cargo pin is wired, but another declaration of the crate (the root's or a workspace member's) names no Socket registry and accepts the patched version, so cargo builds a separate unpatched copy for it; no statement while it does. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | A vendored Gradle entry attests only while its wiring is live (apply line, index rows, diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index 25e49273e..431dbd27d 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -232,6 +232,10 @@ const NOTE_MAVEN_REACTOR_ROOT: &str = "vex_maven_reactor_root"; /// classifier copy at another version, which the pin does not reach /// (`vex::Unattested`). const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched"; +/// Omission tag and note: a hosted Cargo pin beside another declaration of +/// the crate that names no Socket registry, which cargo resolves as a +/// separate, unpatched package (`vex::Unattested`). +const NOTE_CARGO_UNPINNED_DECLARATION: &str = "vex_cargo_unpinned_declaration"; /// The omission tag of an [`Unattested`](socket_patch_core::vex::Unattested) /// ref, and the remedy its note ends with. @@ -268,6 +272,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { "not attested while that classifier copy resolves an unpatched version; the \ wiring itself is intact, so re-running `scan` does not change this", ), + UnattestedKind::CargoUnpinnedDeclaration => ( + NOTE_CARGO_UNPINNED_DECLARATION, + "not attested while that declaration names no Socket registry (re-run \ + `socket-patch scan` to pin a crates.io declaration too; one from another \ + registry, a path or git cannot be pinned, so roll the patch back)", + ), } } @@ -1762,6 +1772,52 @@ mod tests { assert!(live.hosted.contains_key(PURL)); } + /// A hosted Cargo pin beside an unpinned declaration of the crate is + /// omitted as `vex_cargo_unpinned_declaration`, the note naming the + /// declaration and the remedy. + #[tokio::test] + async fn a_cargo_pin_beside_an_unpinned_declaration_is_gated() { + const PURL: &str = "pkg:cargo/serde@1.0.1"; + let tmp = tempfile::tempdir().unwrap(); + let mut redirect = RedirectState::new(); + redirect.records.insert(PURL.into(), record(U1)); + let mut found = discovery(vec![hosted_ref(PURL, U1, true)]); + found.unattested.push(socket_patch_core::vex::Unattested { + purl: PURL.into(), + uuid: U1.into(), + file: "crates/a/Cargo.toml".into(), + detail: "crates/a/Cargo.toml declares serde \"1\" from crates.io".into(), + kind: UnattestedKind::CargoUnpinnedDeclaration, + }); + let gated = plan( + &common(tmp.path()), + Sources { + manifest: PatchManifest::new(), + vendor: VendorState::new(), + redirect: Some(redirect), + discovery: found, + }, + &[], + &RunApiClient::new(), + ) + .await; + assert!(gated.view.patches.is_empty() && gated.hosted.is_empty()); + assert_eq!( + gated.gated, + vec![failed(PURL, NOTE_CARGO_UNPINNED_DECLARATION)] + ); + assert!( + gated + .notes + .iter() + .any(|n| n.code == NOTE_CARGO_UNPINNED_DECLARATION + && n.detail.contains("crates/a/Cargo.toml declares serde") + && n.detail.contains("socket-patch scan")), + "{:?}", + gated.notes + ); + } + /// Ledger-only records with no wiring anywhere are gated, except the /// manifest-owned one, which falls back to agent-mode verification; an /// in-run confirmed purl is live by construction. diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 3139998ec..93d2f172c 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -40,7 +40,15 @@ //! //! * not declared in any of them (a transitive-only crate; no / unparseable //! root manifest) → ref; -//! * some declaration pins `socket-patch-` (the same uuid) → ref; +//! * some declaration pins `socket-patch-` (the same uuid) → ref; but +//! when another declaration names no Socket registry (crates.io, another +//! registry, a path or git) with a requirement that accepts the locked +//! version, cargo resolves it as a separate, unpatched package — maybe of +//! this very version (the lock may already hold it beside the Socket +//! entry) — so the ref is marked +//! [`super::UnattestedKind::CargoUnpinnedDeclaration`], naming that +//! declaration's file (the hosted rewriter pins every declaration, so this +//! is a partial revert or a hand edit); //! * declarations pin only OTHER `socket-patch-` registries → the lock //! ref AND a `Cargo.toml` ref for `` at the locked version (no pin): //! the files disagree and the CLI gates the package as `wiring_conflict`; @@ -121,7 +129,7 @@ use toml_edit::{DocumentMut, Item, TableLike}; use super::{ names_vendor_dir, simple_purl, socket_patch_name_uuid, toml_or_diag, vendor_ref, DiscoverCtx, - Discovery, PatchedRef, TomlDiag, UnlockedPin, VendorRef, DIAG_REF_INVALID, + Discovery, PatchedRef, TomlDiag, UnattestedKind, UnlockedPin, VendorRef, DIAG_REF_INVALID, DIAG_REF_UNATTRIBUTABLE, }; use crate::formats::cargo::{is_registry_source, CargoLock, CopyClaim, LockedPackage}; @@ -436,8 +444,10 @@ async fn read_config( // ── manifest / config shapes ───────────────────────────────────────────── -/// One declaration of a crate: the root-relative file holding it and its -/// `registry` value (`None` = no registry key). +/// One declaration of a crate: the root-relative file holding it, its +/// `registry` value (`None` = no registry key) and, for a dependency +/// declaration, its version requirement and the source cargo fetches it +/// from. #[derive(Debug, Clone)] struct Decl { file: String, @@ -445,14 +455,20 @@ struct Decl { /// The `version` requirement (`None` for a `[patch]` registry pin, or /// a declaration without one). version: Option, + /// Where cargo resolves a dependency declaration from, for messages + /// ([`DepEntry::origin`]); `None` for a `[patch]` entry's registry pin, + /// which redirects declarations rather than adding one. + origin: Option, } impl Decl { + /// A `[patch]` entry's registry pin (no dependency of its own). 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), + origin: None, } } @@ -468,6 +484,25 @@ impl Decl { .as_deref() .is_some_and(|r| r.trim().starts_with(PIN_NAME_PREFIX)) } + + /// Whether this is a dependency declaration cargo resolves outside + /// every Socket patch registry: no `registry` key (crates.io, or a + /// path / git source), or a registry not named `socket-patch-*`. + fn unpinned(&self) -> bool { + self.origin.is_some() && !self.socket_shaped() + } + + /// Whether this declaration's version requirement may select + /// `version`. A missing or unreadable requirement (a path / git + /// declaration, a spelling semver does not read) may select any + /// version, so it does. + fn accepts(&self, version: &str) -> bool { + let (Some(req), Ok(version)) = (self.version.as_deref(), semver::Version::parse(version)) + else { + return true; + }; + semver::VersionReq::parse(req.trim()).map_or(true, |req| req.matches(&version)) + } } /// Crate name → its declarations ([`Decl`]). @@ -481,11 +516,13 @@ fn manifest_declarations(manifests: &[(String, DocumentMut)]) -> Decls { let mut decls = Decls::new(); for (file, doc) in manifests { for dep in dependency_entries(doc) { - decls.entry(dep.name).or_default().push(Decl::new( - file, - dep.registry.as_deref(), - dep.version.as_deref(), - )); + let decl = Decl { + file: file.clone(), + origin: Some(dep.origin()), + registry: dep.registry, + version: dep.version, + }; + decls.entry(dep.name).or_default().push(decl); } } decls @@ -519,6 +556,20 @@ struct DepEntry { registry: Option, /// The `version` requirement (a bare string entry is one). version: Option, + /// The entry's `git` / `path` key, when it has one. + local: Option<&'static str>, +} + +impl DepEntry { + /// Where cargo resolves this declaration from, for messages. + fn origin(&self) -> String { + match (self.local, &self.registry) { + (Some("git"), _) => "a git repository".to_string(), + (Some(_), _) => "a local path".to_string(), + (None, Some(reg)) => format!("registry `{reg}`"), + (None, None) => "crates.io".to_string(), + } + } } fn dependency_entries(doc: &DocumentMut) -> Vec { @@ -548,6 +599,7 @@ fn dependency_entries(doc: &DocumentMut) -> Vec { name: key.to_string(), registry: None, version: Some(version.to_string()), + local: None, } } else if let Some(entry) = item.as_table_like() { if entry.get("workspace").and_then(Item::as_bool) == Some(true) { @@ -558,6 +610,7 @@ fn dependency_entries(doc: &DocumentMut) -> Vec { name: field("package").unwrap_or_else(|| key.to_string()), registry: field("registry"), version: field("version"), + local: ["git", "path"].into_iter().find(|k| entry.contains_key(k)), } } else { continue; @@ -642,6 +695,7 @@ fn hosted_from_lock( .iter() .any(|d| d.registry.as_deref() == Some(reg.as_str())) { + unpinned_beside_pin(ctx, lock, pkg, &purl, &uuid, regs, out); out.push(lock_ref); continue; } @@ -688,6 +742,79 @@ fn hosted_from_lock( } } +/// Mark the Socket-locked `pkg` [`UnattestedKind::CargoUnpinnedDeclaration`] +/// when, beside a declaration pinned to its patch, another declaration of +/// the crate (the root's or a workspace member's) is not pinned to any +/// Socket registry and its version requirement accepts the locked version. +/// Cargo resolves each source on its own, so that declaration builds a +/// separate, unpatched copy (from crates.io, its own registry, a path or a +/// git checkout), which may be this very version: the lock may already +/// hold it beside the Socket entry, or the next build adds it. The hosted +/// rewriter pins every declaration, so this is a partial revert or a hand +/// edit. The ref stays (rollback, remove and the ledgers still see the +/// pin), but nothing attests it while that declaration stands. +fn unpinned_beside_pin( + ctx: &DiscoverCtx<'_>, + lock: &Lock, + pkg: &LockedPackage, + purl: &str, + uuid: &str, + regs: &[Decl], + out: &mut Discovery, +) { + let unpinned: Vec<&Decl> = regs + .iter() + .filter(|d| d.unpinned() && d.accepts(&pkg.version)) + .collect(); + let Some(first) = unpinned.first() else { + return; + }; + let declared = unpinned + .iter() + .map(|d| { + let req = d + .version + .as_deref() + .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); + let from = d.origin.as_deref().unwrap_or_default(); + format!("{} declares {} {req} from {from}", d.file, pkg.name) + }) + .collect::>() + .join(", and "); + // Direct evidence: the lock already holds that unpinned copy at the + // locked version (another source beside the Socket one). + let twin = lock + .packages() + .iter() + .filter(|p| p.name == pkg.name && p.version == pkg.version) + .find_map(|p| { + p.source + .as_deref() + .filter(|s| source_uuid(ctx, s).is_none()) + }); + let evidence = match twin { + Some(source) => format!( + "; {CARGO_LOCK} already locks that copy beside the Socket one ({} {} from {source})", + pkg.name, pkg.version + ), + None => String::new(), + }; + out.unattested( + purl, + uuid, + &first.file, + format!( + "{declared}, not registry \"{}\": cargo resolves that declaration as a \ + separate package, so the crate declaring it builds an unpatched {} that the \ + requirement allows to be {}{evidence}", + hosted_pin_name(uuid), + pkg.name, + pkg.version + ), + UnattestedKind::CargoUnpinnedDeclaration, + ); +} + /// Record every lock block that resolves a Socket-hosted crate's /// `name@version` from another source ([`Discovery::unpatched_copy`]). /// Cargo never unifies packages of different sources, so a dependency that @@ -1666,6 +1793,196 @@ mod tests { &out, &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], ); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + } + + /// A workspace whose root and member `crates/a` declare serde with the + /// given dependency lines, locked to Socket patch A at 1.0.1 (plus + /// `extra` lock entries). + fn mixed_workspace(root_dep: &str, member_dep: &str, extra: &[(&str, &str)]) -> Project { + 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{root_dep}\n" + ), + ); + p.write( + "crates/a/Cargo.toml", + format!( + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n[dependencies]\n{member_dep}\n" + ), + ); + let socket = index(UUID_A); + let mut pkgs = vec![("serde", "1.0.1", Some(socket.as_str()), Some(CKSUM))]; + pkgs.extend( + extra + .iter() + .map(|(v, s)| ("serde", *v, Some(*s), Some(CKSUM))), + ); + p.write("Cargo.lock", lock(&pkgs)); + p.write(".cargo/config.toml", registry_block(UUID_A)); + p + } + + /// The one unattested mark of `out`, asserted to be the hosted serde + /// ref's [`UnattestedKind::CargoUnpinnedDeclaration`] naming `file`. + fn unpinned_mark<'a>(out: &'a Discovery, file: &str) -> &'a Unattested { + assert_eq!(out.unattested.len(), 1, "{:#?}", out.unattested); + let u = &out.unattested[0]; + assert_eq!(u.kind, UnattestedKind::CargoUnpinnedDeclaration); + assert_eq!( + (u.purl.as_str(), u.uuid.as_str()), + ("pkg:cargo/serde@1.0.1", UUID_A) + ); + assert_eq!(u.file, std::path::PathBuf::from(file)); + u + } + + /// REGRESSION: the root pins serde to Socket, but a member declares it + /// from crates.io at a requirement the patched 1.0.1 satisfies. Cargo + /// resolves that declaration as a separate crates.io package, so the + /// member builds unpatched bytes: the ref stays (rollback / remove find + /// it, the ledger claim lives) but is not attested, naming the member. + #[tokio::test] + async fn a_root_pin_beside_an_unpinned_member_is_unattested() { + for member in [ + "serde = \"1\"", + "serde = \"=1.0.1\"", + "serde = { version = \"1\", registry = \"my-corp\" }", + "serde = { git = \"https://github.com/serde-rs/serde\" }", + "legacy = { package = \"serde\", version = \"1.0\" }", + ] { + let p = mixed_workspace(&pinned("serde", "1", UUID_A), member, &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + assert!( + out.diagnostics.is_empty(), + "{member}: {:#?}", + out.diagnostics + ); + assert_eq!( + out.hosted_claim("pkg:cargo/serde@1.0.1", UUID_A), + Some(true), + "{member}" + ); + let u = unpinned_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail.contains("crates/a/Cargo.toml declares serde") + && u.detail + .contains(&format!("not registry \"socket-patch-{UUID_A}\"")) + && !u.detail.contains("already locks"), + "{member}: {}", + u.detail + ); + } + let p = mixed_workspace(&pinned("serde", "1", UUID_A), "serde = \"1\"", &[]); + let u = &run(&p).await.unattested[0]; + assert!( + u.detail.contains("serde \"1\" from crates.io"), + "{}", + u.detail + ); + } + + /// The mirror image: a member pins serde, the root declares it from + /// crates.io. The root's declaration builds unpatched bytes. + #[tokio::test] + async fn a_member_pin_beside_an_unpinned_root_is_unattested() { + let p = mixed_workspace("serde = \"1.0\"", &pinned("serde", "1", UUID_A), &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + let u = unpinned_mark(&out, "Cargo.toml"); + assert!( + u.detail + .contains("Cargo.toml declares serde \"1.0\" from crates.io"), + "{}", + u.detail + ); + } + + /// What the hosted rewriter writes — every declaration pinned — is + /// attested; so is a pin beside an unpinned declaration that the + /// patched version cannot satisfy (cargo resolves it to another + /// version, never this one). + #[tokio::test] + async fn every_declaration_pinned_is_attested() { + for (root, member) in [ + (pinned("serde", "1", UUID_A), pinned("serde", "1.0", UUID_A)), + (pinned("serde", "1", UUID_A), "serde = \"0.9\"".to_string()), + ( + pinned("serde", "1", UUID_A), + "serde = \">=1.0.2\"".to_string(), + ), + // A `[patch]` registry entry redirects declarations; it declares + // no dependency of its own. + ( + format!( + "{}\n\n[patch.crates-io]\nserde = {{ version = \"1\", registry = \"my-corp\" }}", + pinned("serde", "1", UUID_A) + ), + pinned("serde", "1", UUID_A), + ), + ] { + let p = mixed_workspace(&root, &member, &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + assert!(out.unattested.is_empty(), "{member}: {:#?}", out.unattested); + assert!( + out.diagnostics.is_empty(), + "{member}: {:#?}", + out.diagnostics + ); + } + } + + /// The lock already holds the member's crates.io copy at the patched + /// version beside the Socket entry: the mark names that lock entry as + /// direct evidence (the crates.io-twin rule also withholds the ref). + /// A crates.io copy at another version is no evidence of this version, + /// but the declaration still may select it on the next resolve. + #[tokio::test] + async fn the_locked_crates_io_copy_is_named_as_evidence() { + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + "serde = \"1\"", + &[("1.0.1", CRATES_IO)], + ); + let out = run(&p).await; + assert_refs(&out, &[]); + assert_eq!(out.shadowed.len(), 1, "{:#?}", out.shadowed); + let u = unpinned_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail.contains(&format!( + "Cargo.lock already locks that copy beside the Socket one (serde 1.0.1 from \ + {CRATES_IO})" + )), + "{}", + u.detail + ); + + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + "serde = \"1\"", + &[("1.0.200", CRATES_IO)], + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + let u = unpinned_mark(&out, "crates/a/Cargo.toml"); + assert!(!u.detail.contains("already locks"), "{}", u.detail); } /// A manifest under `.socket/` (a path dependency into Socket's own diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 9b128f8b1..1e4165109 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -619,6 +619,12 @@ pub enum UnattestedKind { /// version: the classifier-less pin does not retarget it and the grant /// serves only the main jar, so the build runs the public classifier jar. MavenClassifierUnpatched, + /// A hosted Cargo pin beside another declaration of the crate (the + /// root's or a workspace member's) that names no Socket registry and + /// whose requirement accepts the pinned version: cargo resolves that + /// declaration from its own source as a separate package, so the crate + /// declaring it builds an unpatched copy, possibly of this version. + CargoUnpinnedDeclaration, } /// A ref discovery emits (so rollback, remove and list find the wiring, From 7837fb49a87ccd7583fc79322cee0bbba33a3365 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 22:56:31 -0600 Subject: [PATCH 2/3] Classify each Cargo declaration before judging a hosted pin Review fixes for the unpinned-declaration mark. Each declaration of a Socket-locked crate is now classified once (`route`): routed to a Socket patch (its own `socket-patch-` registry, or a Socket `[patch.]` registry entry that redirects it), unpinned (cargo builds its own copy), or saying nothing about the locked version. - A crates.io (or registry) declaration that a Socket `[patch.]` registry entry in the root manifest or project config redirects is no longer counted as unpinned. - A declaration naming `registry = "socket-patch-"` beside `path`, `git` or `registry-index` is not a pin (the hosted rewriter's own pinned check): cargo builds that other source, so it now marks the ref. - Requirements go through `patch::redirect::cargo_req_selects` with the lock's other versions of the crate, and the detail says whether a re-scan can pin the declaration: only a crates.io declaration whose requirement matches no other locked version; otherwise the rewriter refuses (`redirect_cargo_toml_dep_unrewritable`), so the remedy is to change the declaration or roll back. The CLI note's remedy is now generic and the per-case remedy lives in the detail. - `registry-index` declarations are reported as such, not as crates.io. - A `[workspace.dependencies]` template counts only when some manifest inherits it (`workspace = true`). - When the lock already holds a same-version copy from another source, no mark is added: the twin rule (#679) withholds the ref and names it. - Root pinned to patch A with a member pinned to patch B (for the locked version, and B not resolved by the lock) now emits B's Cargo.toml ref beside A's, so the CLI's wiring-conflict gate runs. - `Decl` carries a `DeclSource` enum and a `live` flag; the origin text is formatted only for messages, and `[patch]` pins are built with `Decl::patch` (no dead version parameter). Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../src/commands/vex_sources.rs | 13 +- .../src/vex/discover/cargo.rs | 656 ++++++++++++++---- .../socket-patch-core/src/vex/discover/mod.rs | 6 +- 4 files changed, 543 insertions(+), 136 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 8a612fb66..e09c2f1a1 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Unpinned Cargo declarations** (`vex_cargo_unpinned_declaration`): cargo resolves each source on its own, so when one declaration of a crate pins it to `socket-patch-` but another — the root `Cargo.toml`'s or a workspace member's, read as hosted scan reads them — names no Socket registry (crates.io, another registry, a path or git) with a version requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. The `Cargo.lock` reference is omitted, naming the unpinned declaration's manifest, and the warning also names a same-version lock entry from another source when the lock already holds one. Hosted scan pins every declaration, so this is a partial revert or a hand edit; re-running `scan` pins a crates.io declaration again. A declaration whose requirement excludes the patched version (cargo resolves it to another version) does not withhold the statement. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Unpinned Cargo declarations** (`vex_cargo_unpinned_declaration`): cargo resolves each source on its own, so when one declaration of a crate pins it to `socket-patch-` but another — the root `Cargo.toml`'s or a workspace member's, read as hosted scan reads them — is not pinned, with a version requirement that may select the locked patched version, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. Not pinned means crates.io or another registry that no Socket `[patch.]` registry entry redirects, a `registry-index`, a path or git — the last three even beside `registry = "socket-patch-"`, which they override (hosted scan's own pinned check). The `Cargo.lock` reference is omitted, naming the unpinned declaration's manifest. Hosted scan pins every declaration, so this is a partial revert or a hand edit; the warning says whether re-running `scan` pins it again (a crates.io declaration whose requirement matches no other locked version of the crate) or cannot (`redirect_cargo_toml_dep_unrewritable`: roll the patch back or change the declaration). A declaration whose requirement excludes the patched version (cargo resolves it to another version), and a `[workspace.dependencies]` entry no manifest inherits, do not withhold the statement. When the lock already holds a same-version copy from another source, the reference is withheld as an unpatched lock copy instead. A declaration pinned to ANOTHER Socket patch, which the lock does not resolve the crate from, adds that patch's `Cargo.toml` reference, so the package is gated as `wiring_conflict`. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -2678,7 +2678,7 @@ match it, withholds the statement: | `vex_deno_lock_copy` | run warning and `failed[].reason` | An npm pin is wired, but `deno.lock` locks the same `name@version`, which `deno install` installs without the wiring; no statement while it does. `vendor --check` and `scan` still treat the wiring as live. | | `vex_maven_reactor_root` | run warning and `failed[].reason` | A hosted Maven pin sits in a root `pom.xml` that declares `` / `` (a profile's included), where a module's own literal `` overrides it; only the root pom is read, so no statement (#261). `list`, `rollback` and `remove` still find the pin. | | `vex_maven_classifier_unpatched` | run warning and `failed[].reason` | A hosted Maven pin's GA also has a live executable `` copy (not `sources` / `javadoc`) at a version other than the pin, which the classifier-less pin does not retarget and the grant does not serve; no statement while it does. `list`, `rollback` and `remove` still find the pin. | -| `vex_cargo_unpinned_declaration` | run warning and `failed[].reason` | A hosted Cargo pin is wired, but another declaration of the crate (the root's or a workspace member's) names no Socket registry and accepts the patched version, so cargo builds a separate unpatched copy for it; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_cargo_unpinned_declaration` | run warning and `failed[].reason` | A hosted Cargo pin is wired, but another declaration of the crate (the root's or a workspace member's) is not pinned (crates.io or another registry no Socket `[patch]` entry redirects, a `registry-index`, a path or git) and may select the patched version, so cargo builds a separate unpatched copy for it; no statement while it does. The warning says whether re-running `scan` can pin it. `list`, `rollback` and `remove` still find the pin. | | `vex_gradle_derived_cache_unchecked` | run warning | The derived-cache walk was cut short (a very large transforms cache); the statement is still emitted, the copies the walk did reach were checked. | A vendored Gradle entry attests only while its wiring is live (apply line, index rows, diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index 431dbd27d..84c72843c 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -274,9 +274,7 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { ), UnattestedKind::CargoUnpinnedDeclaration => ( NOTE_CARGO_UNPINNED_DECLARATION, - "not attested while that declaration names no Socket registry (re-run \ - `socket-patch scan` to pin a crates.io declaration too; one from another \ - registry, a path or git cannot be pinned, so roll the patch back)", + "not attested while that declaration stands", ), } } @@ -1786,7 +1784,9 @@ mod tests { purl: PURL.into(), uuid: U1.into(), file: "crates/a/Cargo.toml".into(), - detail: "crates/a/Cargo.toml declares serde \"1\" from crates.io".into(), + detail: "crates/a/Cargo.toml declares serde \"1\" from crates.io, not registry \ + \"socket-patch-…\"; re-run `socket-patch scan` to pin it too" + .into(), kind: UnattestedKind::CargoUnpinnedDeclaration, }); let gated = plan( @@ -1812,7 +1812,10 @@ mod tests { .iter() .any(|n| n.code == NOTE_CARGO_UNPINNED_DECLARATION && n.detail.contains("crates/a/Cargo.toml declares serde") - && n.detail.contains("socket-patch scan")), + && n.detail.ends_with( + "re-run `socket-patch scan` to pin it too; not attested while that \ + declaration stands" + )), "{:?}", gated.notes ); diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 93d2f172c..76e025bbc 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -40,15 +40,23 @@ //! //! * not declared in any of them (a transitive-only crate; no / unparseable //! root manifest) → ref; -//! * some declaration pins `socket-patch-` (the same uuid) → ref; but -//! when another declaration names no Socket registry (crates.io, another -//! registry, a path or git) with a requirement that accepts the locked -//! version, cargo resolves it as a separate, unpatched package — maybe of -//! this very version (the lock may already hold it beside the Socket -//! entry) — so the ref is marked -//! [`super::UnattestedKind::CargoUnpinnedDeclaration`], naming that -//! declaration's file (the hosted rewriter pins every declaration, so this -//! is a partial revert or a hand edit); +//! * some declaration names `socket-patch-` (the same uuid) → ref; +//! the other declarations whose requirement may select the locked +//! version then decide whether it is attested. One routed to ANOTHER +//! Socket patch (that the lock does not resolve the crate from) adds a +//! `Cargo.toml` ref for it — the conflict pair below. One cargo builds +//! its own copy for — crates.io or another registry not redirected by a +//! Socket `[patch.]` entry, a `registry-index`, a path or git +//! (even beside a `socket-patch-` registry, which those override: +//! the hosted rewriter's own pinned check) — marks the ref +//! [`super::UnattestedKind::CargoUnpinnedDeclaration`], naming its file +//! and whether a re-scan can pin it: cargo resolves each source on its +//! own, so that copy is unpatched and may be this very version. The +//! hosted rewriter pins every declaration, so this is a partial revert or +//! a hand edit. A `[workspace.dependencies]` template counts only when a +//! manifest inherits it. When the lock already holds a same-version copy +//! from another source, the twin rule (`unpatched_twins`, #679) withholds +//! the ref instead; //! * declarations pin only OTHER `socket-patch-` registries → the lock //! ref AND a `Cargo.toml` ref for `` at the locked version (no pin): //! the files disagree and the CLI gates the package as `wiring_conflict`; @@ -134,6 +142,7 @@ use super::{ }; use crate::formats::cargo::{is_registry_source, CargoLock, CopyClaim, LockedPackage}; use crate::patch::redirect::generation::{hosted_pin_name, PIN_NAME_PREFIX}; +use crate::patch::redirect::{cargo_req_selects, CargoReqMatch}; use crate::utils::digest::is_hex64_lower; use crate::vendor::cargo_config::{ effective_config_rel_in, patch_entries, registry_definitions, CargoPatchEntry, CONFIG_LEGACY, @@ -183,7 +192,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { decls .entry(name) .or_default() - .push(Decl::new(CARGO_TOML, Some(reg), None)); + .push(Decl::patch(CARGO_TOML, patch.source, reg)); } } decls @@ -194,7 +203,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), None)); + existing.push(Decl::patch(file, patch.source, reg)); } } } @@ -444,10 +453,26 @@ async fn read_config( // ── manifest / config shapes ───────────────────────────────────────────── +/// Where cargo takes one declaration of a crate from. +#[derive(Debug, Clone, PartialEq, Eq)] +enum DeclSource { + /// A `[patch.]` entry's `registry` pin: it declares no + /// dependency, but redirects the declarations cargo resolves from + /// `
` ([`Decl::patched_by`]). + Patch(String), + /// A registry dependency: its `registry`, else crates.io. + Registry, + /// `registry-index = ""`. + RegistryIndex(String), + /// `path = "…"` (cargo builds it locally, whatever `registry` says). + Path, + /// `git = "…"`. + Git, +} + /// One declaration of a crate: the root-relative file holding it, its -/// `registry` value (`None` = no registry key) and, for a dependency -/// declaration, its version requirement and the source cargo fetches it -/// from. +/// `registry` value (`None` = no registry key), its version requirement +/// and where cargo takes it from. #[derive(Debug, Clone)] struct Decl { file: String, @@ -455,20 +480,21 @@ struct Decl { /// The `version` requirement (`None` for a `[patch]` registry pin, or /// a declaration without one). version: Option, - /// Where cargo resolves a dependency declaration from, for messages - /// ([`DepEntry::origin`]); `None` for a `[patch]` entry's registry pin, - /// which redirects declarations rather than adding one. - origin: Option, + source: DeclSource, + /// `false` for a `[workspace.dependencies]` template no manifest + /// inherits (`workspace = true`): cargo resolves it for nobody. + live: bool, } impl Decl { - /// A `[patch]` entry's registry pin (no dependency of its own). - fn new(file: &str, registry: Option<&str>, version: Option<&str>) -> Self { + /// A `[patch.
]` entry's registry pin. + fn patch(file: &str, table: &str, registry: &str) -> Self { Decl { file: file.to_string(), - registry: registry.map(str::to_string), - version: version.map(str::to_string), - origin: None, + registry: Some(registry.to_string()), + version: None, + source: DeclSource::Patch(table.to_string()), + live: true, } } @@ -485,23 +511,38 @@ impl Decl { .is_some_and(|r| r.trim().starts_with(PIN_NAME_PREFIX)) } - /// Whether this is a dependency declaration cargo resolves outside - /// every Socket patch registry: no `registry` key (crates.io, or a - /// path / git source), or a registry not named `socket-patch-*`. - fn unpinned(&self) -> bool { - self.origin.is_some() && !self.socket_shaped() + /// The Socket patch this declaration itself pins the crate to: a + /// `socket-patch-` registry on a registry dependency or a + /// `[patch]` entry. Beside `path`, `git` or `registry-index` the + /// registry key does not decide where cargo takes the crate from (the + /// hosted rewriter's own pinned check), so that is no pin. + fn pin(&self) -> Option { + match self.source { + DeclSource::Registry | DeclSource::Patch(_) => self.socket_uuid(), + _ => None, + } } - /// Whether this declaration's version requirement may select - /// `version`. A missing or unreadable requirement (a path / git - /// declaration, a spelling semver does not read) may select any - /// version, so it does. - fn accepts(&self, version: &str) -> bool { - let (Some(req), Ok(version)) = (self.version.as_deref(), semver::Version::parse(version)) - else { - return true; - }; - semver::VersionReq::parse(req.trim()).map_or(true, |req| req.matches(&version)) + /// Whether the `[patch.
]` table redirects this declaration: + /// a registry dependency of that registry (`crates-io`, or a crates.io + /// URL alias, for one with no `registry`). + fn patched_by(&self, table: &str) -> bool { + self.source == DeclSource::Registry + && match self.registry.as_deref() { + None => is_crates_io_source(table), + Some(reg) => reg.trim() == table, + } + } + + /// Where cargo takes this dependency from, for messages. + fn origin(&self) -> String { + match (&self.source, self.registry.as_deref()) { + (DeclSource::Git, _) => "a git repository".to_string(), + (DeclSource::Path, _) => "a local path".to_string(), + (DeclSource::RegistryIndex(url), _) => format!("the registry index {url}"), + (_, Some(reg)) => format!("registry `{reg}`"), + (_, None) => "crates.io".to_string(), + } } } @@ -510,20 +551,33 @@ 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. +/// key). `workspace = true` inheritors are not declarations of their own — +/// the root's `[workspace.dependencies]` template they inherit is, and it +/// is live only when some manifest inherits its key. fn manifest_declarations(manifests: &[(String, DocumentMut)]) -> Decls { + let entries: Vec<(&str, DepEntry)> = manifests + .iter() + .flat_map(|(file, doc)| { + dependency_entries(doc) + .into_iter() + .map(move |d| (file.as_str(), d)) + }) + .collect(); + let inherited: Vec<&str> = entries + .iter() + .filter(|(_, d)| d.inherits) + .map(|(_, d)| d.key.as_str()) + .collect(); let mut decls = Decls::new(); - for (file, doc) in manifests { - for dep in dependency_entries(doc) { - let decl = Decl { - file: file.clone(), - origin: Some(dep.origin()), - registry: dep.registry, - version: dep.version, - }; - decls.entry(dep.name).or_default().push(decl); - } + for (file, dep) in entries.iter().filter(|(_, d)| !d.inherits) { + let decl = Decl { + file: file.to_string(), + registry: dep.registry.clone(), + version: dep.version.clone(), + source: dep.source.clone(), + live: !dep.template || inherited.contains(&dep.key.as_str()), + }; + decls.entry(dep.name.clone()).or_default().push(decl); } decls } @@ -550,37 +604,39 @@ fn decl_files<'a>(decls: impl IntoIterator) -> String { files.join(", ") } -/// One manifest dependency declaration (see [`manifest_declarations`]). +/// One manifest dependency entry (see [`manifest_declarations`]). struct DepEntry { + /// The table key. + key: String, + /// The crate: `package = "…"` when renamed, else the key. name: String, registry: Option, /// The `version` requirement (a bare string entry is one). version: Option, - /// The entry's `git` / `path` key, when it has one. - local: Option<&'static str>, -} - -impl DepEntry { - /// Where cargo resolves this declaration from, for messages. - fn origin(&self) -> String { - match (self.local, &self.registry) { - (Some("git"), _) => "a git repository".to_string(), - (Some(_), _) => "a local path".to_string(), - (None, Some(reg)) => format!("registry `{reg}`"), - (None, None) => "crates.io".to_string(), - } - } + source: DeclSource, + /// A `[workspace.dependencies]` template. + template: bool, + /// A `workspace = true` inheritor (of the template keyed `key`). + inherits: bool, } fn dependency_entries(doc: &DocumentMut) -> Vec { let mut deps = Vec::new(); - let mut tables: Vec<&dyn TableLike> = Vec::new(); + let mut tables: Vec<(&dyn TableLike, bool)> = Vec::new(); let root = doc.as_table(); for kind in DEP_TABLES { - tables.extend(root.get(kind).and_then(Item::as_table_like)); + tables.extend( + root.get(kind) + .and_then(Item::as_table_like) + .map(|t| (t, false)), + ); } if let Some(ws) = root.get("workspace").and_then(Item::as_table_like) { - tables.extend(ws.get("dependencies").and_then(Item::as_table_like)); + tables.extend( + ws.get("dependencies") + .and_then(Item::as_table_like) + .map(|t| (t, true)), + ); } if let Some(targets) = root.get("target").and_then(Item::as_table_like) { for (_, target) in targets.iter() { @@ -588,29 +644,47 @@ fn dependency_entries(doc: &DocumentMut) -> Vec { continue; }; for kind in DEP_TABLES { - tables.extend(target.get(kind).and_then(Item::as_table_like)); + tables.extend( + target + .get(kind) + .and_then(Item::as_table_like) + .map(|t| (t, false)), + ); } } } - for table in tables { + for (table, template) in tables { for (key, item) in table.iter() { let dep = if let Some(version) = item.as_str() { DepEntry { + key: key.to_string(), name: key.to_string(), registry: None, version: Some(version.to_string()), - local: None, + source: DeclSource::Registry, + template, + inherits: false, } } else if let Some(entry) = item.as_table_like() { - if entry.get("workspace").and_then(Item::as_bool) == Some(true) { - continue; - } let field = |k: &str| entry.get(k).and_then(Item::as_str).map(str::to_string); + let source = if entry.contains_key("git") { + DeclSource::Git + } else if entry.contains_key("path") { + DeclSource::Path + } else if let Some(url) = field("registry-index") { + DeclSource::RegistryIndex(url) + } else { + DeclSource::Registry + }; DepEntry { + key: key.to_string(), name: field("package").unwrap_or_else(|| key.to_string()), registry: field("registry"), version: field("version"), - local: ["git", "path"].into_iter().find(|k| entry.contains_key(k)), + source, + template, + inherits: !template + && entry.get("workspace").and_then(Item::as_bool) == Some(true), } } else { continue; @@ -691,20 +765,21 @@ fn hosted_from_lock( out.push(lock_ref); continue; }; + // Some declaration names this patch's registry (even one a `path`, + // `git` or `registry-index` overrides): Socket's wiring is present, + // so the lock ref stands, and the other declarations decide whether + // it is attested ([`beside_the_pin`]). if regs .iter() .any(|d| d.registry.as_deref() == Some(reg.as_str())) { - unpinned_beside_pin(ctx, lock, pkg, &purl, &uuid, regs, out); out.push(lock_ref); + beside_the_pin(ctx, lock, pkg, &purl, &uuid, regs, out); continue; } let others: Vec<(&str, String)> = regs .iter() - .filter_map(|d| { - let other = socket_patch_name_uuid(d.registry.as_deref()?, false)?; - Some((d.file.as_str(), other)) - }) + .filter_map(|d| Some((d.file.as_str(), d.pin()?))) .collect(); if others.is_empty() { let files = decl_files(regs); @@ -742,18 +817,67 @@ fn hosted_from_lock( } } -/// Mark the Socket-locked `pkg` [`UnattestedKind::CargoUnpinnedDeclaration`] -/// when, beside a declaration pinned to its patch, another declaration of -/// the crate (the root's or a workspace member's) is not pinned to any -/// Socket registry and its version requirement accepts the locked version. -/// Cargo resolves each source on its own, so that declaration builds a -/// separate, unpatched copy (from crates.io, its own registry, a path or a -/// git checkout), which may be this very version: the lock may already -/// hold it beside the Socket entry, or the next build adds it. The hosted -/// rewriter pins every declaration, so this is a partial revert or a hand -/// edit. The ref stays (rollback, remove and the ledgers still see the -/// pin), but nothing attests it while that declaration stands. -fn unpinned_beside_pin( +/// What one declaration of a crate means for its Socket-locked +/// `name@version` ([`route`]). +enum Route { + /// Routes it to this Socket patch: its own `socket-patch-` + /// registry, or a `[patch]` entry's that redirects it. + Pin(String), + /// Cargo builds its own, unpatched copy for it, which may be this + /// version: [`CargoReqMatch::Ours`] when only this locked version + /// matches its requirement, [`CargoReqMatch::Ambiguous`] when another + /// locked one does too. + Unpinned(CargoReqMatch), + /// Says nothing about this version: its requirement excludes it, it is + /// a `[patch]` entry (a redirection, counted through the declarations + /// it redirects), a template nobody inherits, or a malformed + /// `socket-patch-*` name (diagnosed by [`unresolved_manifest_pins`]). + Elsewhere, +} + +/// Classify `d` for the locked `version` of its crate, `others` being the +/// other versions the lock holds of it and `patch_pins` the `[patch.
]` +/// Socket registry pins (table, uuid) cargo honors. +fn route(d: &Decl, patch_pins: &[(&str, String)], version: &str, others: &[String]) -> Route { + if matches!(d.source, DeclSource::Patch(_)) || !d.live { + return Route::Elsewhere; + } + let selects = cargo_req_selects(d.version.as_deref(), version, others); + if selects == CargoReqMatch::NotOurs { + return Route::Elsewhere; + } + if let Some(uuid) = d.pin() { + return Route::Pin(uuid); + } + if d.source == DeclSource::Registry && d.socket_shaped() { + return Route::Elsewhere; + } + match patch_pins.iter().find(|(table, _)| d.patched_by(table)) { + Some((_, uuid)) => Route::Pin(uuid.clone()), + None => Route::Unpinned(selects), + } +} + +/// The other declarations beside a lock entry pinned to Socket patch +/// `uuid` ([`route`] for each): +/// +/// * one routed to ANOTHER Socket patch that may select this version is a +/// conflict: its wiring is emitted beside the lock ref, so the CLI's +/// conflict gate refuses to pick one; +/// * one cargo builds its own copy for (crates.io, another registry or +/// registry index, a path, git — including a `socket-patch-` +/// registry a `path` / `git` / `registry-index` overrides) marks the ref +/// [`UnattestedKind::CargoUnpinnedDeclaration`]: cargo resolves each +/// source on its own, so that copy is unpatched and may be this very +/// version. The hosted rewriter pins every declaration, so this is a +/// partial revert or a hand edit. The ref stays (rollback, remove and +/// the ledgers still see the pin); the detail names the declarations and +/// whether a re-scan can pin them. +/// +/// When the lock already holds a same-version copy from another source, +/// [`unpatched_twins`] withholds the ref and names that entry (#679), so +/// nothing is added here. +fn beside_the_pin( ctx: &DiscoverCtx<'_>, lock: &Lock, pkg: &LockedPackage, @@ -762,42 +886,109 @@ fn unpinned_beside_pin( regs: &[Decl], out: &mut Discovery, ) { - let unpinned: Vec<&Decl> = regs + let same_name = || lock.packages().iter().filter(|p| p.name == pkg.name); + let twin = same_name().any(|p| { + p.version == pkg.version + && p.source + .as_deref() + .is_some_and(|s| source_uuid(ctx, s).is_none()) + }); + if twin { + return; + } + let others = distinct( + same_name() + .filter(|p| p.version != pkg.version) + .map(|p| Some(p.version.clone())), + ); + let patch_pins: Vec<(&str, String)> = regs .iter() - .filter(|d| d.unpinned() && d.accepts(&pkg.version)) + .filter_map(|d| match &d.source { + DeclSource::Patch(table) => Some((table.as_str(), d.socket_uuid()?)), + _ => None, + }) .collect(); - let Some(first) = unpinned.first() else { + let mut unpinned: Vec<(&Decl, CargoReqMatch)> = Vec::new(); + for d in regs { + match route(d, &patch_pins, &pkg.version, &others) { + // A patch the lock resolves the crate from at all is its own + // ref (that registry serves only its own version, a separate, + // patched package): no conflict. + Route::Pin(other) + if other != uuid + && !same_name().any(|p| { + p.source + .as_deref() + .and_then(|s| source_uuid(ctx, s)) + .is_some_and(|u| u == other) + }) => + { + out.push(PatchedRef::hosted( + purl.to_string(), + other, + &d.file, + None, + None, + true, + )); + } + Route::Unpinned(selects) => unpinned.push((d, selects)), + Route::Pin(_) | Route::Elsewhere => {} + } + } + let Some((first, _)) = unpinned.first() else { return; }; let declared = unpinned .iter() - .map(|d| { + .map(|(d, _)| { let req = d .version .as_deref() .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); - let from = d.origin.as_deref().unwrap_or_default(); - format!("{} declares {} {req} from {from}", d.file, pkg.name) + format!("{} declares {} {req} from {}", d.file, pkg.name, d.origin()) }) .collect::>() .join(", and "); - // Direct evidence: the lock already holds that unpinned copy at the - // locked version (another source beside the Socket one). - let twin = lock - .packages() + // What a re-scan can do: the hosted rewriter pins a crates.io + // declaration whose requirement selects only the patched version among + // the locked ones, and refuses every other kind. + let refused: Vec = unpinned .iter() - .filter(|p| p.name == pkg.name && p.version == pkg.version) - .find_map(|p| { - p.source - .as_deref() - .filter(|s| source_uuid(ctx, s).is_none()) - }); - let evidence = match twin { - Some(source) => format!( - "; {CARGO_LOCK} already locks that copy beside the Socket one ({} {} from {source})", - pkg.name, pkg.version - ), - None => String::new(), + .filter_map(|(d, selects)| { + let why = match (&d.source, d.registry.as_deref(), selects) { + (DeclSource::Registry, None, CargoReqMatch::Ours) => return None, + (DeclSource::Registry, None, _) => { + let also: Vec<&str> = others + .iter() + .filter(|v| { + cargo_req_selects(d.version.as_deref(), v, &[]) + != CargoReqMatch::NotOurs + }) + .map(String::as_str) + .collect(); + format!( + "its requirement also matches {} {} locked beside it", + pkg.name, + also.join(", ") + ) + } + (DeclSource::Registry | DeclSource::RegistryIndex(_), ..) => { + "it names another registry".to_string() + } + _ => "it builds a path or git source".to_string(), + }; + Some(format!("{}'s declaration ({why})", d.file)) + }) + .collect(); + let remedy = if refused.is_empty() { + "re-run `socket-patch scan` to pin it too".to_string() + } else { + format!( + "`socket-patch scan` cannot pin {}, so change that declaration or roll the \ + patch back", + refused.join(", or ") + ) }; out.unattested( purl, @@ -806,7 +997,7 @@ fn unpinned_beside_pin( format!( "{declared}, not registry \"{}\": cargo resolves that declaration as a \ separate package, so the crate declaring it builds an unpatched {} that the \ - requirement allows to be {}{evidence}", + requirement allows to be {}; {remedy}", hosted_pin_name(uuid), pkg.name, pkg.version @@ -1947,12 +2138,11 @@ mod tests { } /// The lock already holds the member's crates.io copy at the patched - /// version beside the Socket entry: the mark names that lock entry as - /// direct evidence (the crates.io-twin rule also withholds the ref). - /// A crates.io copy at another version is no evidence of this version, - /// but the declaration still may select it on the next resolve. + /// version beside the Socket entry: the crates.io-twin rule (#679) + /// withholds the ref (shadowed, its diagnostic naming that lock entry) + /// and no unattested mark is added for a ref that no longer stands. #[tokio::test] - async fn the_locked_crates_io_copy_is_named_as_evidence() { + async fn a_locked_same_version_twin_is_left_to_the_twin_rule() { let p = mixed_workspace( &pinned("serde", "1", UUID_A), "serde = \"1\"", @@ -1961,18 +2151,35 @@ mod tests { let out = run(&p).await; assert_refs(&out, &[]); assert_eq!(out.shadowed.len(), 1, "{:#?}", out.shadowed); - let u = unpinned_mark(&out, "crates/a/Cargo.toml"); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.detail.contains(&format!("serde 1.0.1 ({CRATES_IO})"))), + "{:#?}", + out.diagnostics + ); + } + + /// The remedy follows what a re-scan can do: the hosted rewriter pins a + /// crates.io declaration whose requirement selects only the patched + /// version, but refuses one that also matches another locked version + /// (`redirect_cargo_toml_dep_unrewritable`), one from another registry + /// or `registry-index`, and a path / git one. + #[tokio::test] + async fn the_remedy_says_whether_a_rescan_can_pin_it() { + let p = mixed_workspace(&pinned("serde", "=1.0.1", UUID_A), "serde = \"1\"", &[]); + let u = run(&p).await.unattested.remove(0); assert!( - u.detail.contains(&format!( - "Cargo.lock already locks that copy beside the Socket one (serde 1.0.1 from \ - {CRATES_IO})" - )), + u.detail.contains("re-run `socket-patch scan`"), "{}", u.detail ); + // `"1"` also matches the crates.io 1.0.200 locked beside it. let p = mixed_workspace( - &pinned("serde", "1", UUID_A), + &pinned("serde", "=1.0.1", UUID_A), "serde = \"1\"", &[("1.0.200", CRATES_IO)], ); @@ -1982,7 +2189,176 @@ mod tests { &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], ); let u = unpinned_mark(&out, "crates/a/Cargo.toml"); - assert!(!u.detail.contains("already locks"), "{}", u.detail); + assert!( + u.detail.contains("also matches serde 1.0.200") + && u.detail.contains("roll the patch back") + && !u.detail.contains("re-run `socket-patch scan`"), + "{}", + u.detail + ); + + for (member, from) in [ + ( + "serde = { version = \"1\", registry = \"my-corp\" }", + "from registry `my-corp`", + ), + ( + "serde = { version = \"1\", registry-index = \"https://corp.example/index\" }", + "from the registry index https://corp.example/index", + ), + ("serde = { path = \"../serde\" }", "from a local path"), + ] { + let p = mixed_workspace(&pinned("serde", "1", UUID_A), member, &[]); + let out = run(&p).await; + let u = unpinned_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail.contains(from) + && u.detail.contains("roll the patch back") + && !u.detail.contains("re-run `socket-patch scan`"), + "{member}: {}", + u.detail + ); + } + } + + /// A declaration that names the Socket registry but also a `path`, + /// `git` or `registry-index` source is not a pin (the hosted rewriter's + /// own check): cargo builds that source. Beside a real pin it withholds + /// the statement; alone it still keeps the ref (Socket's wiring is + /// present) but never attests it. + #[tokio::test] + async fn a_socket_registry_beside_another_source_is_not_a_pin() { + let reg = format!("registry = \"socket-patch-{UUID_A}\""); + for extra in [ + "path = \"../serde\"", + "git = \"https://github.com/serde-rs/serde\"", + "registry-index = \"https://corp.example/index\"", + ] { + let member = format!("serde = {{ version = \"1\", {reg}, {extra} }}"); + let p = mixed_workspace(&pinned("serde", "1", UUID_A), &member, &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + unpinned_mark(&out, "crates/a/Cargo.toml"); + + let p = Project::new(); + p.write("Cargo.toml", manifest(&member)); + 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, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + unpinned_mark(&out, "Cargo.toml"); + } + } + + /// A `[workspace.dependencies]` entry is a template: cargo resolves it + /// only for a manifest that inherits it (`workspace = true`). An + /// unpinned template nobody inherits builds nothing; an inherited one + /// does. + #[tokio::test] + async fn an_unpinned_workspace_template_counts_only_when_inherited() { + for (member_dep, inherited) in [ + ("other = \"1\"", false), + ("serde = { workspace = true }", true), + ] { + let p = Project::new(); + p.write( + "Cargo.toml", + format!( + "[workspace]\nmembers = [\"crates/*\"]\n\n[workspace.dependencies]\n\ + serde = \"1\"\n\n[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\n{}\n", + pinned("serde", "1", UUID_A) + ), + ); + p.write( + "crates/a/Cargo.toml", + format!( + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n[dependencies]\n\ + {member_dep}\n" + ), + ); + 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, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + if inherited { + unpinned_mark(&out, "Cargo.toml"); + } else { + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + } + } + } + + /// The root pins the lock's patch A, a member pins ANOTHER patch B for + /// the same version: the files disagree, so both wirings are emitted + /// and the CLI's conflict gate refuses to pick one. A member pin whose + /// requirement excludes the locked version is about another version. + #[tokio::test] + async fn a_member_pinned_to_another_patch_is_a_conflict_pair() { + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "1", UUID_B), + &[], + ); + 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 b = out.refs.iter().find(|r| r.uuid == UUID_B).unwrap(); + assert_eq!( + b.source_file, + std::path::PathBuf::from("crates/a/Cargo.toml") + ); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "0.9", UUID_B), + &[], + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + + // The lock resolves the member's B pin too, at another version: B's + // registry serves only that version, a separate patched package + // with its own ref, not a conflict over 1.0.1. + let b_index = index(UUID_B); + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "1", UUID_B), + &[("1.0.5", b_index.as_str())], + ); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted), + ("pkg:cargo/serde@1.0.5", UUID_B, WiringMode::Hosted), + ], + ); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); } /// A manifest under `.socket/` (a path dependency into Socket's own @@ -2309,6 +2685,31 @@ mod tests { &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], ); assert!(out.diagnostics.is_empty(), "{:#?}", out.diagnostics); + // The crates.io declaration is what the entry redirects: not unpinned. + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + + // A declaration from ANOTHER registry is not redirected by + // `[patch.crates-io]`: cargo builds my-corp's serde for it. + let p = Project::new(); + p.write( + "Cargo.toml", + format!( + "{}\n[patch.crates-io]\n{}\n", + manifest("serde = \"1\"\nalt = { package = \"serde\", version = \"1\", registry = \"my-corp\" }"), + 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_eq!(out.unattested.len(), 1, "{:#?}", out.unattested); + assert!( + out.unattested[0].detail.contains("from registry `my-corp`"), + "{:#?}", + out.unattested + ); } /// A registry pin in the effective config's `[patch]` counts as a @@ -2331,6 +2732,7 @@ mod tests { &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], ); assert!(out.diagnostics.is_empty(), "{:#?}", out.diagnostics); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); } #[tokio::test] diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 1e4165109..495aa261f 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -620,8 +620,10 @@ pub enum UnattestedKind { /// serves only the main jar, so the build runs the public classifier jar. MavenClassifierUnpatched, /// A hosted Cargo pin beside another declaration of the crate (the - /// root's or a workspace member's) that names no Socket registry and - /// whose requirement accepts the pinned version: cargo resolves that + /// root's or a workspace member's) that is not pinned (no Socket + /// registry or `[patch]` redirect reaches it, or a path / git / + /// `registry-index` overrides its registry) and whose requirement may + /// select the pinned version: cargo resolves that /// declaration from its own source as a separate package, so the crate /// declaring it builds an unpatched copy, possibly of this version. CargoUnpinnedDeclaration, From ebe84e199f9c6e654f42dc2e67d505bb8335bca1 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:33:49 +0000 Subject: [PATCH 3/3] Count registry = "crates-io" as re-pinnable in the Cargo remedy Second review round. The unpinned-declaration remedy sent every declaration with an explicit `registry` key to "it names another registry ... roll the patch back", including `registry = "crates-io"`, which the hosted rewriter re-pins (#386); it now says a re-scan pins it. Co-Authored-By: Claude Opus 5.5 --- .../src/vex/discover/cargo.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 216ecddbb..6c62b9878 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -947,7 +947,10 @@ fn beside_the_pin( let refused: Vec = unpinned .iter() .filter_map(|(d, selects)| { - let why = match (&d.source, d.registry.as_deref(), selects) { + // `registry = "crates-io"` is crates.io itself, which the + // rewriter re-pins like a declaration with no `registry` (#386). + let registry = d.registry.as_deref().filter(|r| r.trim() != "crates-io"); + let why = match (&d.source, registry, selects) { (DeclSource::Registry, None, CargoReqMatch::Ours) => return None, (DeclSource::Registry, None, _) => { let also: Vec<&str> = others @@ -2167,6 +2170,19 @@ mod tests { "{}", u.detail ); + // Review on #2019 (round 2): `registry = "crates-io"` is crates.io, + // which a re-scan re-pins (#386). + let p = mixed_workspace( + &pinned("serde", "=1.0.1", UUID_A), + "serde = { version = \"1\", registry = \"crates-io\" }", + &[], + ); + let u = run(&p).await.unattested.remove(0); + assert!( + u.detail.contains("re-run `socket-patch scan`"), + "{}", + u.detail + ); // `"1"` also matches the crates.io 1.0.200 locked beside it. let p = mixed_workspace(