From 64d73a621ddbf55e36a594956f1be2dbda4a2e51 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:27:28 -0600 Subject: [PATCH 1/9] Withhold a vendored Cargo copy a member's other source bypasses `vendor` wires a crate through the root's `[patch.crates-io]`, which cargo applies only to declarations from crates.io. A workspace member (or the root) that declares the crate from another registry, a `registry-index` or git, at a requirement that may select the patched version, resolves its own unpatched copy. The vendored ref stayed attested beside it because the lock copy claim only reads sourceless entries. The ref now stays (rollback, remove and the ledgers still see the wiring) but is marked unattested with the new `UnattestedKind::CargoPatchBypassed`, naming the declaration. A same-version Cargo.lock entry from any registry or git source beside the copy is marked the same way, naming the lock entry. The check reads the declarations through #2019's classification: live declarations only, a registry dependency counts as covered when a `[patch.]` table pointing at the same copy redirects it (`Decl::patched_by`), a path declaration (the user's own fork) keeps the existing "ignored" rule, and a `socket-patch-*` registry dependency is hosted wiring. The CLI omits the ref as `vex_cargo_patch_bypassed`; documented in CLI_CONTRACT.md. The workspace member walk ran only when the lock held a Socket-sourced package (or, with no lock, a manifest named a `socket-patch-` registry). A vendored copy's lock entry has no source and it has no such pin, so the walk now also runs whenever the root manifest or the project config wires a Socket copy through `[patch]`, whatever the lock's state. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 3 +- .../src/commands/vex_sources.rs | 59 +++ .../src/vex/discover/cargo.rs | 397 +++++++++++++++++- .../socket-patch-core/src/vex/discover/mod.rs | 7 + 4 files changed, 451 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e09c2f1a1..11d122a9f 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 — 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`. +* **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`. Six 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`. **Vendored Cargo copies a `[patch]` does not reach** (`vex_cargo_patch_bypassed`): a `[patch.]` entry replaces the crate only in declarations from ``, and `vendor` writes `[patch.crates-io]`. So a vendored reference is omitted when a live declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry, a `registry-index` or a git repository, with a version requirement that may select the patched version, unless it is a registry dependency of a registry whose `[patch.]` table points at the same copy; cargo builds that declaration's own, unpatched copy. Workspace members are read for this whenever the root manifest or the project cargo config wires a Socket copy through `[patch]`, whatever the lock says. It is also omitted when `Cargo.lock` locks the crate at exactly the patched version from any source (a registry, crates.io included, or git) beside the vendored copy; the warning names that lock entry. A path declaration (the user's own fork) does not withhold the statement. Re-running `vendor` does not change this; declare the crate from crates.io there, or roll the patch back. * **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). @@ -2679,6 +2679,7 @@ match it, withholds the statement: | `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) 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_cargo_patch_bypassed` | run warning and `failed[].reason` | A vendored Cargo copy is wired, but a declaration of the crate from another registry or git accepts the patched version, or `Cargo.lock` locks that version from a registry or git, and the copy's `[patch]` entry does not redirect it; no statement while it does. `list`, `rollback` and `remove` still find the wiring. | | `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 84c72843c..d99bae325 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -236,6 +236,10 @@ const NOTE_MAVEN_CLASSIFIER_UNPATCHED: &str = "vex_maven_classifier_unpatched"; /// 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"; +/// Omission tag and note: a vendored Cargo copy beside a copy of the crate +/// from a source its `[patch]` entry does not redirect (another registry, +/// git), which cargo builds unpatched (`vex::Unattested`). +const NOTE_CARGO_PATCH_BYPASSED: &str = "vex_cargo_patch_bypassed"; /// The omission tag of an [`Unattested`](socket_patch_core::vex::Unattested) /// ref, and the remedy its note ends with. @@ -276,6 +280,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { NOTE_CARGO_UNPINNED_DECLARATION, "not attested while that declaration stands", ), + UnattestedKind::CargoPatchBypassed => ( + NOTE_CARGO_PATCH_BYPASSED, + "not attested while that copy builds; the vendored wiring itself is intact, so \ + re-running `vendor` does not change this (declare the crate from crates.io \ + there, or roll the patch back)", + ), } } @@ -1821,6 +1831,55 @@ mod tests { ); } + /// A vendored Cargo copy beside a copy of the crate its `[patch]` + /// entry does not redirect is omitted as `vex_cargo_patch_bypassed`, + /// the note naming that copy and the remedy. + #[tokio::test] + async fn a_vendored_cargo_copy_beside_a_bypassing_source_is_gated() { + const PURL: &str = "pkg:cargo/serde@1.0.1"; + let tmp = tempfile::tempdir().unwrap(); + let mut manifest = PatchManifest::new(); + manifest.patches.insert(PURL.into(), record(U1)); + let vref = vendor_ref(&format!(".socket/vendor/cargo/{U1}/serde-1.0.1")).unwrap(); + let mut found = discovery(vec![PatchedRef::vendored( + PURL.into(), + &vref, + "Cargo.toml", + None, + )]); + 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 registry `my-corp`".into(), + kind: UnattestedKind::CargoPatchBypassed, + }); + let gated = plan( + &common(tmp.path()), + Sources { + manifest, + vendor: VendorState::new(), + redirect: None, + discovery: found, + }, + &[], + &RunApiClient::new(), + ) + .await; + assert!(gated.view.patches.is_empty() && gated.hosted.is_empty()); + assert_eq!(gated.gated, vec![failed(PURL, NOTE_CARGO_PATCH_BYPASSED)]); + assert!( + gated + .notes + .iter() + .any(|n| n.code == NOTE_CARGO_PATCH_BYPASSED + && n.detail.contains("from registry `my-corp`") + && n.detail.contains("re-running `vendor`")), + "{:?}", + 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 76e025bbc..971bb3220 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -126,6 +126,14 @@ //! config), and a `[patch.""]` table in the manifest //! replaces `[patch.crates-io]` wholesale — both leave the lock looking //! exactly like live Socket wiring. +//! A `[patch.]` entry replaces the crate only in declarations from +//! ``, so a ref whose build may also compile a copy of the version +//! the wiring does not reach — a live declaration (root or member) from +//! another registry, a `registry-index` or git that may select the version +//! and that no `[patch]` table pointing at this copy redirects, or a +//! same-version lock entry from any source — stays a ref but is marked +//! [`super::UnattestedKind::CargoPatchBypassed`] ([`patch_bypassed`]); the +//! members are read for this whenever a `[patch]` wires a Socket copy. //! A `.socket/cargo-patches/` path (the retired redirect backend) carries no //! uuid and is not a ref. Socket-shaped entries in a `.cargo/config.toml` //! that cargo ignores (because `.cargo/config` exists) are diagnosed, never @@ -174,8 +182,13 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { let mut manifests: Vec<(String, DocumentMut)> = Vec::new(); if let Some(doc) = root { let root_pins = manifest_has_socket_pin(&doc); + // A vendored copy wired in the root or the config is weighed against + // every member's declarations ([`patch_bypassed`]). + let vendored = std::iter::once(&doc) + .chain(config.iter().map(|(_, c)| c)) + .any(|d| !socket_copies(d).is_empty()); manifests.push((CARGO_TOML.to_string(), doc)); - manifests.extend(read_members(ctx, &lock, root_pins, out).await); + manifests.extend(read_members(ctx, &lock, root_pins, vendored, out).await); } let manifest: Option<&DocumentMut> = manifests.first().map(|(_, doc)| doc); @@ -219,6 +232,16 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if let Some(decls) = &decls { unresolved_manifest_pins(ctx, &lock, decls, &definitions, out); } + let wired: Vec<(String, VendorRef)> = manifest + .into_iter() + .chain(config.iter().map(|(_, doc)| doc)) + .flat_map(socket_copies) + .collect(); + let bypass = Bypass { + lock: &lock, + decls: decls.as_ref(), + wired: &wired, + }; 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 @@ -253,25 +276,22 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { format!("{file} redefines `[patch]` key `{key}`, which replaces it in cargo") }) }; - vendored_from_patches(ctx, CARGO_TOML, doc, &lock, &shadowed, out).await; + vendored_from_patches(ctx, CARGO_TOML, doc, &lock, &shadowed, &bypass, out).await; } if let Some((file, doc)) = &config { - vendored_from_patches(ctx, file, doc, &lock, &|_| None, out).await; + vendored_from_patches(ctx, file, doc, &lock, &|_| None, &bypass, out).await; } - let wired: Vec = manifest - .into_iter() - .chain(config.iter().map(|(_, doc)| doc)) - .flat_map(socket_copies) - .collect(); - unattributed_tags(&lock, &wired, out); + let copies: Vec = wired.into_iter().map(|(_, v)| v).collect(); + unattributed_tags(&lock, &copies, out); } -/// Every Socket copy a file's `[patch]` tables point at. -fn socket_copies(doc: &DocumentMut) -> Vec { +/// Every Socket copy a file's `[patch]` tables point at, with the +/// `[patch.]` table pointing at it. +fn socket_copies(doc: &DocumentMut) -> Vec<(String, VendorRef)> { patch_entries(doc) .into_iter() - .filter_map(|e| e.path.and_then(vendor_ref)) - .filter(|v| v.eco == "cargo") + .filter_map(|e| Some((e.source.to_string(), e.path.and_then(vendor_ref)?))) + .filter(|(_, v)| v.eco == "cargo") .collect() } @@ -359,14 +379,20 @@ async fn read_toml(ctx: &DiscoverCtx<'_>, file: &str, out: &mut Discovery) -> Op /// 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. +/// texts, so the parse is skipped otherwise. `vendored` (the root manifest +/// or the config wires a Socket copy through `[patch]`, whose lock entries +/// are sourceless) always reads them, whatever the lock: a member's +/// declaration from a source the `[patch]` does not redirect bypasses the +/// copy ([`patch_bypassed`]). async fn read_members( ctx: &DiscoverCtx<'_>, lock: &Lock, root_pins: bool, + vendored: bool, out: &mut Discovery, ) -> Vec<(String, DocumentMut)> { let lockless = match lock { + _ if vendored => false, Lock::Parsed(_) if has_socket_sourced(ctx, lock) => false, Lock::Parsed(_) | Lock::Unparseable => return Vec::new(), Lock::Absent => true, @@ -1223,6 +1249,124 @@ async fn copy_tag(ctx: &DiscoverCtx<'_>, artifact_rel: &str, out: &mut Discovery } } +/// What a vendored copy is weighed against for copies its `[patch]` wiring +/// does not redirect ([`patch_bypassed`]). +struct Bypass<'a> { + lock: &'a Lock, + /// Every declaration in the root and member manifests (`None`: no + /// usable root manifest). + decls: Option<&'a Decls>, + /// Every Socket copy the manifest and config `[patch]` tables point at, + /// with the `[patch.]` table pointing at it. + wired: &'a [(String, VendorRef)], +} + +/// Mark the vendored ref `purl` (the copy `vref` of `name`@`version`) +/// [`UnattestedKind::CargoPatchBypassed`] when the build may also compile +/// a copy of that version its wiring does not reach. A `[patch.]` +/// entry replaces the crate only in declarations from `` (v5 +/// `vendor` writes `[patch.crates-io]`), so either of these builds +/// unpatched bytes: +/// +/// * a live dependency declaration (the root's or a workspace member's) +/// from another registry, a `registry-index` or git, whose requirement +/// may select `version`, unless it is a registry dependency that a +/// `[patch]` table pointing at this copy redirects ([`Decl::patched_by`]): +/// cargo resolves it as a separate package, maybe at this very version; +/// * a `Cargo.lock` entry of `name` at exactly `version` with a source +/// (any registry or git; a Socket patch server's is hosted wiring, a ref +/// of its own): that copy is already locked beside the vendored one. +/// +/// A path declaration (the user's own fork) is left alone, as the lock's +/// copy claim leaves its sourceless entry alone; a `socket-patch-*` +/// registry dependency is hosted wiring, weighed there. The ref stays +/// (rollback, remove and the ledgers still see the wiring), but nothing +/// attests it. +fn patch_bypassed( + ctx: &DiscoverCtx<'_>, + bypass: &Bypass<'_>, + vref: &VendorRef, + name: &str, + version: &str, + purl: &str, + out: &mut Discovery, +) { + let redirected = |d: &Decl| { + bypass + .wired + .iter() + .any(|(table, v)| v.uuid == vref.uuid && v.leaf == vref.leaf && d.patched_by(table)) + }; + let declared: Vec<&Decl> = bypass + .decls + .and_then(|d| d.get(name)) + .into_iter() + .flatten() + .filter(|d| d.live) + .filter(|d| match &d.source { + DeclSource::Registry => !d.socket_shaped() && !redirected(d), + DeclSource::RegistryIndex(_) | DeclSource::Git => true, + DeclSource::Path | DeclSource::Patch(_) => false, + }) + .filter(|d| cargo_req_selects(d.version.as_deref(), version, &[]) != CargoReqMatch::NotOurs) + .collect(); + let locked = bypass + .lock + .packages() + .iter() + .filter(|p| p.name == name && p.version == version) + .find_map(|p| { + p.source + .as_deref() + .filter(|s| source_uuid(ctx, s).is_none()) + }); + let redirects = format!( + "which no [patch] entry pointing at {} redirects", + vref.artifact_rel + ); + let (file, detail) = match (declared.first(), locked) { + (Some(first), _) => { + let evidence = locked.map_or_else(String::new, |source| { + format!("; {CARGO_LOCK} also locks {name} {version} from {source}") + }); + let declared = declared + .iter() + .map(|d| { + let req = d + .version + .as_deref() + .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); + format!("{} declares {name} {req} from {}", d.file, d.origin()) + }) + .collect::>() + .join(", and "); + ( + first.file.as_str(), + format!( + "{declared}, {redirects}: cargo resolves that declaration as a separate \ + package, so the crate declaring it builds an unpatched {name} that the \ + requirement allows to be {version}{evidence}" + ), + ) + } + (None, Some(source)) => ( + CARGO_LOCK, + format!( + "{CARGO_LOCK} also locks {name} {version} from {source}, {redirects}: cargo \ + builds that copy unpatched beside the vendored one" + ), + ), + (None, None) => return, + }; + out.unattested( + purl, + &vref.uuid, + file, + detail, + UnattestedKind::CargoPatchBypassed, + ); +} + /// Vendored refs from one file's `[patch]` tables. `shadowed` names why /// cargo ignores an entry (a higher-precedence item replaces it), if so. async fn vendored_from_patches( @@ -1231,6 +1375,7 @@ async fn vendored_from_patches( doc: &DocumentMut, lock: &Lock, shadowed: &(dyn Fn(&CargoPatchEntry<'_>) -> Option + Sync), + bypass: &Bypass<'_>, out: &mut Discovery, ) { for entry in patch_entries(doc) { @@ -1343,6 +1488,7 @@ async fn vendored_from_patches( continue; } } + patch_bypassed(ctx, bypass, &vref, name, version, &purl, out); out.push(PatchedRef::vendored(purl, &vref, file, None)); } } @@ -3415,4 +3561,227 @@ mod tests { } } } + + // ── vendored: copies the [patch] does not redirect ─────────────── + + const MY_CORP: &str = "registry+https://my-corp.example/index"; + + /// A v5-vendored workspace: the root wires cfg-if 1.0.4 to the tagged + /// copy for patch A through `[patch.crates-io]` (plus `root_extra` + /// appended to the root manifest) and declares it from crates.io; the + /// member `crates/a` declares it with `member_dep`. The lock holds the + /// tagged copy plus `extra` entries `(version, source)`. + fn vendored_workspace(member_dep: &str, root_extra: &str, extra: &[(&str, &str)]) -> Project { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let p = Project::new(); + p.write( + "Cargo.toml", + format!( + "[workspace]\nmembers = [\"crates/*\"]\n\n{}\n[patch.crates-io]\n\ + cfg-if-socket-aaaaaaaa = {{ package = \"cfg-if\", path = \"{rel}\" }}\n\ + {root_extra}\n", + manifest("cfg-if = \"1\"") + ), + ); + p.write( + "crates/a/Cargo.toml", + format!( + "[package]\nname = \"a\"\nversion = \"0.1.0\"\n\n[dependencies]\n{member_dep}\n" + ), + ); + let tagged = crate::vendor::cargo_tag::tag_version("1.0.4", UUID_A); + let mut pkgs = vec![("cfg-if", tagged.as_str(), None, None)]; + pkgs.extend( + extra + .iter() + .map(|(v, s)| ("cfg-if", *v, Some(*s), Some(CKSUM))), + ); + p.write("Cargo.lock", lock(&pkgs)); + p + } + + /// The one unattested mark of `out`, asserted to be the vendored + /// cfg-if ref's [`UnattestedKind::CargoPatchBypassed`] naming `file`. + fn bypassed_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::CargoPatchBypassed); + assert_eq!( + (u.purl.as_str(), u.uuid.as_str()), + ("pkg:cargo/cfg-if@1.0.4", UUID_A) + ); + assert_eq!(u.file, std::path::PathBuf::from(file)); + u + } + + /// REGRESSION: `[patch.crates-io]` redirects only crates.io + /// declarations. A member that declares cfg-if from another registry or + /// from git, at a requirement the patched 1.0.4 satisfies, resolves its + /// own unpatched copy, which may be that very version. The vendored ref + /// stays (rollback / remove find it, the ledger claim lives) but is not + /// attested, naming the member. + #[tokio::test] + async fn a_member_from_another_source_bypasses_the_vendored_copy() { + for (member, origin) in [ + ( + "cfg-if = { version = \"1\", registry = \"my-corp\" }", + "from registry `my-corp`", + ), + ( + "fork = { package = \"cfg-if\", version = \"=1.0.4\", registry = \"my-corp\" }", + "from registry `my-corp`", + ), + ( + "cfg-if = { git = \"https://example.com/cfg-if\" }", + "from a git repository", + ), + ( + "cfg-if = { version = \"1\", registry-index = \"sparse+https://my-corp.example/index/\" }", + "from the registry index sparse+https://my-corp.example/index/", + ), + ] { + let p = vendored_workspace(member, "", &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + assert!( + out.diagnostics.is_empty(), + "{member}: {:#?}", + out.diagnostics + ); + assert_eq!( + out.vendored_claim( + "pkg:cargo/cfg-if@1.0.4", + UUID_A, + &vendor_path(UUID_A, "cfg-if-1.0.4") + ), + Some(true), + "{member}" + ); + let u = bypassed_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail.contains("crates/a/Cargo.toml declares cfg-if") + && u.detail.contains(origin) + && !u.detail.contains("also locks"), + "{member}: {}", + u.detail + ); + } + } + + /// The lock already holds a same-version cfg-if from another source + /// (another registry, git, even crates.io) beside the vendored copy: + /// cargo builds that copy unpatched whoever declares it, so the mark + /// names the lock entry. A copy at another version is no such evidence. + #[tokio::test] + async fn a_same_version_lock_entry_from_any_source_bypasses_the_vendored_copy() { + for source in [MY_CORP, "git+https://example.com/cfg-if#abc123", CRATES_IO] { + let p = vendored_workspace("cfg-if = \"1\"", "", &[("1.0.4", source)]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + let u = bypassed_mark(&out, "Cargo.lock"); + assert!( + u.detail + .contains(&format!("Cargo.lock also locks cfg-if 1.0.4 from {source}")), + "{source}: {}", + u.detail + ); + } + let p = vendored_workspace("cfg-if = \"1\"", "", &[("0.1.10", MY_CORP)]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + } + + /// What the `[patch]` wiring redirects stays attested: a member + /// declaring cfg-if from crates.io, under any spelling, and a + /// declaration from elsewhere whose requirement excludes the patched + /// version, or a registry dependency whose `[patch.]` table + /// also points at the same copy; and a `[workspace.dependencies]` + /// template from another registry that no manifest inherits. A user's + /// own path fork is the documented exception (see + /// `a_same_version_fork_beside_the_tagged_copy_is_ignored`). + #[tokio::test] + async fn declarations_the_patch_redirects_keep_the_vendored_copy_attested() { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let covered = format!("\n[patch.my-corp]\ncfg-if = {{ path = \"{rel}\" }}\n"); + let template = "\n[workspace.dependencies]\n\ + cfg-if = { version = \"1\", registry = \"my-corp\" }\n"; + for (member, root_extra) in [ + ("cfg-if = \"1\"", ""), + ("cfg-if = \"=1.0.4\"", ""), + ("cfg-if = { version = \"1\", registry = \"crates-io\" }", ""), + ("alias = { package = \"cfg-if\", version = \"1.0\" }", ""), + ("cfg-if = { version = \"0.1\", registry = \"my-corp\" }", ""), + ( + "cfg-if = { version = \"1\", registry = \"my-corp\" }", + covered.as_str(), + ), + ("cfg-if = { path = \"../fork\" }", ""), + ("cfg-if = \"1\"", template), + ] { + let p = vendored_workspace(member, root_extra, &[]); + let out = run(&p).await; + assert!( + out.refs.iter().any(|r| r.purl == "pkg:cargo/cfg-if@1.0.4" + && r.uuid == UUID_A + && r.mode == WiringMode::Vendored), + "{member}: {:#?}", + out.refs + ); + assert!(out.unattested.is_empty(), "{member}: {:#?}", out.unattested); + } + } + + /// The pre-v5 wiring in the project config redirects only crates.io + /// too: a member's git declaration bypasses it the same way. + #[tokio::test] + async fn a_member_from_git_bypasses_the_config_wired_copy() { + let p = Project::new(); + p.write( + ".cargo/config.toml", + patch_config(&[("cfg-if", &vendor_path(UUID_A, "cfg-if-1.0.4"))]), + ); + workspace_with_member(&p, "cfg-if = { git = \"https://example.com/cfg-if\" }"); + p.write("Cargo.lock", lock(&[("cfg-if", "1.0.4", None, None)])); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + bypassed_mark(&out, "crates/a/Cargo.toml"); + } + + /// REGRESSION (member gate): a vendored project's lock has no + /// Socket-sourced package (the copy's entry is sourceless), and a + /// lockless one has no `socket-patch-` pin, so the member walk must + /// not hinge on either: the members are read whenever a `[patch]` + /// wires a Socket copy, lock or no lock. + #[tokio::test] + async fn members_are_read_for_a_vendored_copy_without_a_socket_lock_entry() { + for with_lock in [true, false] { + let p = vendored_workspace( + "cfg-if = { version = \"1\", registry = \"my-corp\" }", + "", + &[], + ); + if !with_lock { + std::fs::remove_file(p.root().join("Cargo.lock")).unwrap(); + } + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + bypassed_mark(&out, "crates/a/Cargo.toml"); + } + } } diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index 495aa261f..3f1d39294 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -627,6 +627,13 @@ pub enum UnattestedKind { /// declaration from its own source as a separate package, so the crate /// declaring it builds an unpatched copy, possibly of this version. CargoUnpinnedDeclaration, + /// A vendored Cargo copy that a `[patch.]` entry wires (in v5, + /// the root's `[patch.crates-io]`) beside a copy of the crate from a + /// source that entry does not redirect: a declaration (the root's or a + /// workspace member's) from another registry or git whose requirement + /// accepts the patched version, or a same-version `Cargo.lock` entry + /// from any source. Cargo builds that copy unpatched. + CargoPatchBypassed, } /// A ref discovery emits (so rollback, remove and list find the wiring, From 346a313ae2b5f6293a2d08f52d77d3bba25de3d2 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:46:17 -0600 Subject: [PATCH 2/9] Classify Cargo lockless pins, git patch tables and conflict pins Three Cargo follow-ups to the declaration classification: Lockless pins read declarations like locked ones. A `socket-patch-` registry beside `path`, `git` or `registry-index` is no pin (cargo builds that source, and the rewriter's own pinned check agrees), so it no longer adds its requirement to an `UnlockedPin` or makes one on its own. It counts as an unpinned declaration instead, excluding the versions it accepts (every version, without a requirement). A `[workspace.dependencies]` template no manifest inherits is no declaration: a Socket-pinned one makes no lockless pin, and an unpinned one excludes nothing from the root's pin. A non-Socket `[patch]` registry entry no longer counts as an unpinned declaration either (it redirects declarations, it adds none). `DeclSource::Git` keeps its URL, and `Decl::patched_by` matches a `[patch.""]` table against it (a trailing `/` or `.git` ignored). So a git declaration whose repository table points at the same vendored copy no longer withholds that copy, while one from another repository still does. Hosted routing gains the same match for a Socket registry pin in such a table. When no declaration pins the lock's patch, a pin to another patch now makes the conflict pair only when it is live and its requirement may select the locked version (`cargo_req_selects`). One that excludes it is about another version; the stale lock entry is then diagnosed and dropped like any unrouted one. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 4 +- .../src/vex/discover/cargo.rs | 226 ++++++++++++++++-- 2 files changed, 202 insertions(+), 28 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 11d122a9f..8f35c613d 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,9 +547,9 @@ 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`. Six 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`. **Vendored Cargo copies a `[patch]` does not reach** (`vex_cargo_patch_bypassed`): a `[patch.]` entry replaces the crate only in declarations from ``, and `vendor` writes `[patch.crates-io]`. So a vendored reference is omitted when a live declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry, a `registry-index` or a git repository, with a version requirement that may select the patched version, unless it is a registry dependency of a registry whose `[patch.]` table points at the same copy; cargo builds that declaration's own, unpatched copy. Workspace members are read for this whenever the root manifest or the project cargo config wires a Socket copy through `[patch]`, whatever the lock says. It is also omitted when `Cargo.lock` locks the crate at exactly the patched version from any source (a registry, crates.io included, or git) beside the vendored copy; the warning names that lock entry. A path declaration (the user's own fork) does not withhold the statement. Re-running `vendor` does not change this; declare the crate from crates.io there, or roll the patch back. +* **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`. Six 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`. **Vendored Cargo copies a `[patch]` does not reach** (`vex_cargo_patch_bypassed`): a `[patch.]` entry replaces the crate only in declarations from ``, and `vendor` writes `[patch.crates-io]`. So a vendored reference is omitted when a live declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry, a `registry-index` or a git repository, with a version requirement that may select the patched version, unless a `[patch]` table for its source points at the same copy (`[patch.]` for a registry dependency, `[patch.""]` for a git one, a trailing `/` or `.git` ignored); cargo builds that declaration's own, unpatched copy. Workspace members are read for this whenever the root manifest or the project cargo config wires a Socket copy through `[patch]`, whatever the lock says. It is also omitted when `Cargo.lock` locks the crate at exactly the patched version from any source (a registry, crates.io included, or git) beside the vendored copy; the warning names that lock entry. A path declaration (the user's own fork) does not withhold the statement. Re-running `vendor` does not change this; declare the crate from crates.io there, or roll the patch back. * **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). +* **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. For Cargo, declarations are read as for a locked pin: a `socket-patch-` registry beside `path`, `git` or `registry-index` is no pin (cargo builds that source), so it admits nothing and its requirement (any version, without one) is excluded from the pin; a `[workspace.dependencies]` template no manifest inherits is no declaration at all. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). **Record resolution.** A candidate's record must carry the patch uuid the lockfile actually **wires**. It is taken from the first source that has one: the manifest (matched qualifier-insensitively), the hosted records above (this run's, then a pre-v5 ledger's), then the vendor ledger's embedded records. If none has it and the run is online, `vex` fetches the patch view by uuid from the patch API — for a v5 hosted checkout this is the normal path. The fetch uses `get`'s API client: the public proxy when no token is configured, and a one-shot 401/403 fallback to the proxy (free patches only). At most 10 fetches run concurrently. Fetched records stay in memory: `vex` never writes the manifest. A candidate still has no record under `--offline`, after a transport error or a 404, or when the patch is refused (paid without an entitled token); it is then omitted as `record_unavailable`, and the run is not aborted. A record whose uuid or package disagrees with the wiring is omitted as `record_mismatch`. The informational `socket-patch.vendor.json` marker is never a record source. When the lockfile wires a package to patch U, a manifest or ledger record for that package under another uuid is superseded, and a human-mode `Note:` says so. diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 971bb3220..6d7cc0ed7 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -492,8 +492,8 @@ enum DeclSource { RegistryIndex(String), /// `path = "…"` (cargo builds it locally, whatever `registry` says). Path, - /// `git = "…"`. - Git, + /// `git = ""` (empty when the value is not a string). + Git(String), } /// One declaration of a crate: the root-relative file holding it, its @@ -551,19 +551,29 @@ impl Decl { /// 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`). + /// URL alias, for one with no `registry`), or a git dependency of that + /// repository URL (a trailing `/` or `.git` ignored; any other + /// difference reads as another repository, so at worst a missed + /// redirect). fn patched_by(&self, table: &str) -> bool { - self.source == DeclSource::Registry - && match self.registry.as_deref() { + fn git_key(url: &str) -> &str { + let url = url.trim().trim_end_matches('/'); + url.strip_suffix(".git").unwrap_or(url) + } + match &self.source { + DeclSource::Registry => match self.registry.as_deref() { None => is_crates_io_source(table), Some(reg) => reg.trim() == table, - } + }, + DeclSource::Git(url) => !url.trim().is_empty() && git_key(url) == git_key(table), + _ => false, + } } /// 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::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}`"), @@ -694,7 +704,7 @@ fn dependency_entries(doc: &DocumentMut) -> Vec { } else if let Some(entry) = item.as_table_like() { 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 + DeclSource::Git(field("git").unwrap_or_default()) } else if entry.contains_key("path") { DeclSource::Path } else if let Some(url) = field("registry-index") { @@ -803,8 +813,17 @@ fn hosted_from_lock( beside_the_pin(ctx, lock, pkg, &purl, &uuid, regs, out); continue; } + // Pins to another Socket patch that may select this version; one + // whose requirement excludes it (or a template nobody inherits) is + // about another version, so it neither conflicts nor keeps the + // stale entry. let others: Vec<(&str, String)> = regs .iter() + .filter(|d| { + d.live + && cargo_req_selects(d.version.as_deref(), &pkg.version, &[]) + != CargoReqMatch::NotOurs + }) .filter_map(|d| Some((d.file.as_str(), d.pin()?))) .collect(); if others.is_empty() { @@ -1113,13 +1132,20 @@ fn unresolved_manifest_pins( 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 => {} + if decl.socket_uuid().is_none() && !malformed.contains(&(decl.file.as_str(), reg)) { + malformed.push((decl.file.as_str(), reg)); + } + } + // Classified like a locked pin ([`route`]): only a live registry + // dependency or `[patch]` entry naming `socket-patch-` pins + // (beside `path` / `git` / `registry-index` the registry key does + // not decide, as in the rewriter's own pinned check), and a + // `[workspace.dependencies]` template nobody inherits declares + // nothing. + let live: Vec<&Decl> = regs.iter().filter(|d| d.live).collect(); + for uuid in live.iter().filter_map(|d| d.pin()) { + if !uuids.contains(&uuid) { + uuids.push(uuid); } } for (file, reg) in malformed { @@ -1146,9 +1172,10 @@ fn unresolved_manifest_pins( continue; // a ref (or the conflict pair) came from the lock } for uuid in &uuids { - let pinning: Vec<&Decl> = regs + let pinning: Vec<&Decl> = live .iter() - .filter(|d| d.socket_uuid().as_deref() == Some(uuid.as_str())) + .copied() + .filter(|d| d.pin().as_deref() == Some(uuid.as_str())) .collect(); let reg = pinning[0].registry.as_deref().unwrap_or_default(); let file = pinning[0].file.as_str(); @@ -1157,20 +1184,26 @@ fn unresolved_manifest_pins( Lock::Absent => { // Another Socket registry (or a malformed one) for the // same crate is a conflict, never a pin. - let only_this_patch = regs + let only_this_patch = live .iter() - .filter(|d| d.socket_shaped()) - .all(|d| d.socket_uuid().as_deref() == Some(uuid.as_str())); + .filter(|d| { + d.socket_shaped() + && matches!(d.source, DeclSource::Registry | DeclSource::Patch(_)) + }) + .all(|d| d.pin().as_deref() == Some(uuid.as_str())); let defined_here = definitions .get(reg) .and_then(|index| source_uuid(ctx, index)) .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 + // Every live dependency declaration this patch does + // not pin (a `path` / `git` / `registry-index` one + // beside a `socket-patch-` registry included); `None` + // when one carries no requirement: it may resolve any + // version. + let excluded_reqs: Option> = live .iter() - .filter(|d| !d.socket_shaped()) + .filter(|d| !matches!(d.source, DeclSource::Patch(_)) && d.pin().is_none()) .map(|d| d.version.clone()) .collect(); if let (true, true, false, Some(excluded_reqs)) = ( @@ -1270,7 +1303,7 @@ struct Bypass<'a> { /// /// * a live dependency declaration (the root's or a workspace member's) /// from another registry, a `registry-index` or git, whose requirement -/// may select `version`, unless it is a registry dependency that a +/// may select `version`, unless it is a registry or git dependency that a /// `[patch]` table pointing at this copy redirects ([`Decl::patched_by`]): /// cargo resolves it as a separate package, maybe at this very version; /// * a `Cargo.lock` entry of `name` at exactly `version` with a source @@ -1305,7 +1338,8 @@ fn patch_bypassed( .filter(|d| d.live) .filter(|d| match &d.source { DeclSource::Registry => !d.socket_shaped() && !redirected(d), - DeclSource::RegistryIndex(_) | DeclSource::Git => true, + DeclSource::Git(_) => !redirected(d), + DeclSource::RegistryIndex(_) => true, DeclSource::Path | DeclSource::Patch(_) => false, }) .filter(|d| cargo_req_selects(d.version.as_deref(), version, &[]) != CargoReqMatch::NotOurs) @@ -1994,6 +2028,28 @@ mod tests { assert!(!toml_ref.lockfile_basis_ok()); } + /// No declaration pins the lock's patch A, and the one pinning B + /// cannot select the locked version: B's pin is about another version, + /// so there is no conflict pair, and nothing routes the crate to A, so + /// the stale lock entry is no ref. (B's pin that may select it is the + /// conflict pair, see `manifest_and_lock_disagreeing_on_the_patch_emit_both`.) + #[tokio::test] + async fn another_patch_pin_excluding_the_locked_version_is_no_conflict() { + let p = Project::new(); + p.write("Cargo.toml", manifest(&pinned("serde", "=1.0.2", 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, &[]); + assert!( + diag_codes(&out).contains(&DIAG_REF_INVALID), + "{:#?}", + out.diagnostics + ); + } + /// A project-config definition of the pinned registry that indexes a /// different patch (or a non-Socket host) is what cargo resolves /// against: the lock entry is stale. @@ -2612,6 +2668,95 @@ mod tests { ); } + /// A `socket-patch-` registry beside `path`, `git` or + /// `registry-index` is no pin (cargo builds the path / git / index + /// source; the rewriter's own pinned check agrees): lockless, such a + /// member declaration is an unpinned one, so it excludes the versions it + /// accepts (all of them with no requirement), and alone it makes no + /// lockless pin at all. + #[tokio::test] + async fn lockless_path_or_git_beside_a_socket_registry_is_no_pin() { + let record = "pkg:cargo/serde@1.0.1"; + let reg = format!("registry = \"socket-patch-{UUID_A}\""); + for member_decl in [ + format!("serde = {{ path = \"../serde\", version = \"1\", {reg} }}"), + format!("serde = {{ git = \"https://example.com/serde\", {reg} }}"), + format!( + "serde = {{ version = \"1\", registry-index = \"sparse+https://x.example/\", {reg} }}" + ), + ] { + 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(false), + "{member_decl}: {:#?}", + out.unlocked_pins + ); + } + // The member's overridden registry alone: no pin anywhere. + let p = Project::new(); + workspace_with_member( + &p, + &format!("serde = {{ path = \"../serde\", version = \"1\", {reg} }}"), + ); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert!(out.unlocked_pins.is_empty(), "{:#?}", out.unlocked_pins); + } + + /// A `[workspace.dependencies]` template no manifest inherits declares + /// nothing cargo resolves: lockless, a Socket-pinned one makes no pin, + /// and an unpinned one excludes no version from the root's pin. Once a + /// member inherits the pinned template, it is the pin. + #[tokio::test] + async fn lockless_uninherited_template_is_not_a_declaration() { + let template = |dep: &str| { + format!("[workspace]\nmembers = [\"crates/*\"]\n\n[workspace.dependencies]\n{dep}\n") + }; + let p = Project::new(); + p.write("Cargo.toml", template(&pinned("serde", "1", UUID_A))); + member(&p, "a", "other = \"1\""); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert!(out.unlocked_pins.is_empty(), "{:#?}", out.unlocked_pins); + + let p = Project::new(); + p.write("Cargo.toml", template(&pinned("serde", "1", UUID_A))); + member(&p, "a", "serde = { workspace = true }"); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_eq!(out.unlocked_pins.len(), 1, "{:#?}", out.unlocked_pins); + assert_eq!( + out.hosted_claim("pkg:cargo/serde@1.0.1", UUID_A), + Some(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) + ), + ); + member(&p, "a", "other = \"1\""); + p.write(".cargo/config.toml", registry_block(UUID_A)); + let out = run(&p).await; + assert_eq!(out.unlocked_pins.len(), 1, "{:#?}", out.unlocked_pins); + assert!(out.unlocked_pins[0].excluded_reqs.is_empty()); + assert_eq!( + out.hosted_claim("pkg:cargo/serde@1.0.1", UUID_A), + Some(true) + ); + } + /// 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. @@ -3741,6 +3886,35 @@ mod tests { } } + /// A `[patch.""]` table pointing at the same copy redirects a + /// declaration from that repository (a trailing `/` or `.git` aside), + /// so the copy stays attested; one from another repository still + /// bypasses it. + #[tokio::test] + async fn a_git_patch_table_for_the_copy_covers_that_repository_only() { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let table = + format!("\n[patch.\"https://example.com/cfg-if\"]\ncfg-if = {{ path = \"{rel}\" }}\n"); + for member in [ + "cfg-if = { git = \"https://example.com/cfg-if\" }", + "cfg-if = { git = \"https://example.com/cfg-if.git/\", branch = \"main\" }", + ] { + let out = run(&vendored_workspace(member, &table, &[])).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + assert!(out.unattested.is_empty(), "{member}: {:#?}", out.unattested); + } + let out = run(&vendored_workspace( + "cfg-if = { git = \"https://example.com/fork/cfg-if\" }", + &table, + &[], + )) + .await; + bypassed_mark(&out, "crates/a/Cargo.toml"); + } + /// The pre-v5 wiring in the project config redirects only crates.io /// too: a member's git declaration bypasses it the same way. #[tokio::test] From 6592bf24497d0655437c9a1d6aa5d08a48d910dc Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 6 Nov 2026 09:31:26 -0600 Subject: [PATCH 3/9] Fix review findings on Cargo vendored bypass and pin diagnostics False attestation: the vendored bypass check counted a declaration as redirected by any root or config `[patch]` entry pointing at the copy, including a manifest entry cargo ignores (the project config redefines its crates.io key, or a crates.io URL table replaces `[patch.crates-io]`). Cargo then builds whatever replaced it for those declarations, while the copy's other, honored entry was attested. Only honored entries count now: the root's `[patch]` entries are collected once with their shadow reason (a `Shadow` built from the root manifest and the config), and the bypass check sees only the unshadowed ones. `vendored` (the member gate) is derived from the same list instead of re-reading every `[patch]` table. One classifier: the bypass check runs each declaration through `route` with the copy's honored `[patch]` tables as the pins, as hosted pins do, passing the lock's other versions of the crate. A path declaration (the user's own fork) is still left alone. `patched_by` now covers a `registry-index` declaration with a `[patch.""]` table for the same index (a `registry+` prefix, host case and a trailing `/` ignored; `sparse+` is another source), and treats a `registry-index` naming the crates.io git index as crates.io. The sparse crates.io index stays a separate source. Git URLs compare as cargo's `CanonicalUrl` spells them (one trailing `/` and a `.git` suffix dropped, a GitHub URL lowercased whole, any host lowercased). The bypass detail ends with the remedy for its evidence: a declaration (re-running `vendor` does not help), a crates.io lock entry beside a `[patch.crates-io]` copy (stale: re-lock), or another lock entry (`cargo tree -i` names its declarer). The CLI note no longer claims re-vendoring never helps. Diagnostics: a Socket pin to another patch that the lock does not reflect (its requirement excludes every locked version) used to vanish once any Socket entry of the crate was locked; it is now diagnosed `patched_ref_unattributable`. A well-formed `socket-patch-` registry that pins nothing (beside `path` / `git` / `registry-index`, or on a `[workspace.dependencies]` entry no manifest inherits) is diagnosed `patched_ref_invalid`. The committed `workspace-member` redirect golden gains that diagnostic: its `expected/` tree omits the unchanged inheriting member, so the pinned template reads as uninherited there. `Decl::describe` and `same_version_foreign_entry` are shared by the hosted and vendored checks. Co-Authored-By: Claude Opus 5.5 --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/commands/vex_sources.rs | 7 +- .../src/vex/discover/cargo.rs | 708 ++++++++++++++---- .../vex-discover-golden/redirect-cargo.json | 8 +- 4 files changed, 571 insertions(+), 154 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 8f35c613d..0cde5a9bb 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`. Six 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`. **Vendored Cargo copies a `[patch]` does not reach** (`vex_cargo_patch_bypassed`): a `[patch.]` entry replaces the crate only in declarations from ``, and `vendor` writes `[patch.crates-io]`. So a vendored reference is omitted when a live declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry, a `registry-index` or a git repository, with a version requirement that may select the patched version, unless a `[patch]` table for its source points at the same copy (`[patch.]` for a registry dependency, `[patch.""]` for a git one, a trailing `/` or `.git` ignored); cargo builds that declaration's own, unpatched copy. Workspace members are read for this whenever the root manifest or the project cargo config wires a Socket copy through `[patch]`, whatever the lock says. It is also omitted when `Cargo.lock` locks the crate at exactly the patched version from any source (a registry, crates.io included, or git) beside the vendored copy; the warning names that lock entry. A path declaration (the user's own fork) does not withhold the statement. Re-running `vendor` does not change this; declare the crate from crates.io there, or roll the patch back. +* **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`. Six 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`. **Vendored Cargo copies a `[patch]` does not reach** (`vex_cargo_patch_bypassed`): a `[patch.]` entry replaces the crate only in declarations from ``, and `vendor` writes `[patch.crates-io]`. So a vendored reference is omitted when a live declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry, a `registry-index` or a git repository, with a version requirement that may select the patched version, unless a `[patch]` table for its source points at the same copy (`[patch.]` for a registry dependency, `[patch.""]` for a git one, compared as cargo canonicalizes it: one trailing `/` and a `.git` suffix ignored, a GitHub URL case-insensitive); cargo builds that declaration's own, unpatched copy. Workspace members are read for this whenever the root manifest or the project cargo config wires a Socket copy through `[patch]`, whatever the lock says. It is also omitted when `Cargo.lock` locks the crate at exactly the patched version from any source (a registry, crates.io included, or git) beside the vendored copy; the warning names that lock entry. A path declaration (the user's own fork) does not withhold the statement. A `registry-index` declaration is covered by a `[patch.""]` table for the same index (a `registry+` prefix, the host's case and a trailing `/` ignored; a `sparse+` index is another source than the git one), and one naming the crates.io git index is crates.io. Only `[patch]` entries cargo honors count: a manifest entry the project config or a crates.io URL table replaces covers nothing. The warning ends with the remedy for its evidence: for a declaration, re-running `vendor` does not help (declare the crate from crates.io there, point a `[patch]` table for its source at the copy, or roll the patch back); for a stale crates.io lock entry beside a `[patch.crates-io]` copy, re-lock (re-run `vendor`, or `cargo update -p `); for another lock entry, `cargo tree -i @` names the dependency that declares it. * **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. For Cargo, declarations are read as for a locked pin: a `socket-patch-` registry beside `path`, `git` or `registry-index` is no pin (cargo builds that source), so it admits nothing and its requirement (any version, without one) is excluded from the pin; a `[workspace.dependencies]` template no manifest inherits is no declaration at all. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs index d99bae325..01cc60a89 100644 --- a/crates/socket-patch-cli/src/commands/vex_sources.rs +++ b/crates/socket-patch-cli/src/commands/vex_sources.rs @@ -282,9 +282,8 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { ), UnattestedKind::CargoPatchBypassed => ( NOTE_CARGO_PATCH_BYPASSED, - "not attested while that copy builds; the vendored wiring itself is intact, so \ - re-running `vendor` does not change this (declare the crate from crates.io \ - there, or roll the patch back)", + "not attested while that copy can build beside the vendored one; the vendored \ + wiring itself is intact (the warning above says how to clear it)", ), } } @@ -1874,7 +1873,7 @@ mod tests { .iter() .any(|n| n.code == NOTE_CARGO_PATCH_BYPASSED && n.detail.contains("from registry `my-corp`") - && n.detail.contains("re-running `vendor`")), + && n.detail.contains("says how to clear it")), "{:?}", 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 6d7cc0ed7..0bf1be73d 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -153,10 +153,11 @@ 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, - CONFIG_TOML, + effective_config_rel_in, patch_entries, registry_definitions, CONFIG_LEGACY, CONFIG_TOML, +}; +use crate::vendor::cargo_manifest::{ + crates_io_url_alias_tables, is_crates_io_source, is_crates_io_url_alias, }; -use crate::vendor::cargo_manifest::{crates_io_url_alias_tables, is_crates_io_source}; use crate::vendor::cargo_tag; use crate::vendor::lock_inventory::LockIntegrity; @@ -179,14 +180,27 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { 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). + // Every `[patch]` entry pointing at a Socket copy, the root manifest's + // (with why cargo ignores it, if it does) and then the config's. + let mut wired_all: Vec<(WiredCopy, Option)> = Vec::new(); + let mut shadow: Option = None; + if let Some(doc) = &root { + let s = Shadow::new(doc, config.as_ref()); + wired_all.extend(socket_copies(doc).into_iter().map(|w| { + let why = s.why(&w.table, &w.key, Some(&w.path)); + (w, why) + })); + shadow = Some(s); + } + if let Some((_, doc)) = &config { + wired_all.extend(socket_copies(doc).into_iter().map(|w| (w, None))); + } let mut manifests: Vec<(String, DocumentMut)> = Vec::new(); if let Some(doc) = root { let root_pins = manifest_has_socket_pin(&doc); // A vendored copy wired in the root or the config is weighed against // every member's declarations ([`patch_bypassed`]). - let vendored = std::iter::once(&doc) - .chain(config.iter().map(|(_, c)| c)) - .any(|d| !socket_copies(d).is_empty()); + let vendored = !wired_all.is_empty(); manifests.push((CARGO_TOML.to_string(), doc)); manifests.extend(read_members(ctx, &lock, root_pins, vendored, out).await); } @@ -232,10 +246,13 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if let Some(decls) = &decls { unresolved_manifest_pins(ctx, &lock, decls, &definitions, out); } - let wired: Vec<(String, VendorRef)> = manifest - .into_iter() - .chain(config.iter().map(|(_, doc)| doc)) - .flat_map(socket_copies) + // Only the entries cargo honors redirect declarations to a copy: a + // shadowed manifest entry covers nothing (cargo builds whatever replaced + // it). + let wired: Vec = wired_all + .iter() + .filter(|(_, why)| why.is_none()) + .map(|(w, _)| w.clone()) .collect(); let bypass = Bypass { lock: &lock, @@ -243,58 +260,15 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { wired: &wired, }; 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]` - // wholesale — a Socket entry there is dead wiring, never attested. - let config_keys: Vec<(&str, String, Option)> = config - .iter() - .flat_map(|(file, doc)| { - patch_entries(doc) - .into_iter() - .filter(|e| is_crates_io_source(e.source)) - .map(move |e| (*file, e.key.to_string(), e.path.map(same_path_key))) - }) - .collect(); - let aliases = crates_io_url_alias_tables(doc); - let shadowed = |entry: &CargoPatchEntry<'_>| -> Option { - if !is_crates_io_source(entry.source) { - return None; - } - if let (true, Some(alias)) = (entry.source == "crates-io", aliases.first()) { - return Some(format!( - "the `[patch.\"{alias}\"]` table replaces `[patch.crates-io]` in cargo" - )); - } - // The same key wiring the same copy (a half-migrated project) - // is not a shadow: the config's own ref carries it. - let own = entry.path.map(same_path_key); - config_keys - .iter() - .find(|(_, key, path)| key == entry.key && *path != own) - .map(|(file, key, _)| { - format!("{file} redefines `[patch]` key `{key}`, which replaces it in cargo") - }) - }; - vendored_from_patches(ctx, CARGO_TOML, doc, &lock, &shadowed, &bypass, out).await; + vendored_from_patches(ctx, CARGO_TOML, doc, &lock, shadow.as_ref(), &bypass, out).await; } if let Some((file, doc)) = &config { - vendored_from_patches(ctx, file, doc, &lock, &|_| None, &bypass, out).await; + vendored_from_patches(ctx, file, doc, &lock, None, &bypass, out).await; } - let copies: Vec = wired.into_iter().map(|(_, v)| v).collect(); + let copies: Vec = wired_all.into_iter().map(|(w, _)| w.vref).collect(); unattributed_tags(&lock, &copies, out); } -/// Every Socket copy a file's `[patch]` tables point at, with the -/// `[patch.]` table pointing at it. -fn socket_copies(doc: &DocumentMut) -> Vec<(String, VendorRef)> { - patch_entries(doc) - .into_iter() - .filter_map(|e| Some((e.source.to_string(), e.path.and_then(vendor_ref)?))) - .filter(|(_, v)| v.eco == "cargo") - .collect() -} - /// Diagnose Socket-tagged sourceless lock entries that no visible /// `[patch]` wiring names (`.socket/vendor/cargo//-`): /// the lock records a vendored copy the files discovery reads cannot @@ -550,26 +524,42 @@ impl Decl { } /// 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`), or a git dependency of that - /// repository URL (a trailing `/` or `.git` ignored; any other - /// difference reads as another repository, so at worst a missed - /// redirect). + /// + /// * a registry dependency of that registry (`crates-io`, or a + /// crates.io URL alias, for one with no `registry`); + /// * a `registry-index` dependency of that index URL ([`index_key`]); + /// one naming the crates.io git index is crates.io itself (the same + /// cargo source), so `[patch.crates-io]` redirects it too; + /// * a git dependency of that repository ([`git_key`], cargo's + /// canonical spelling). + /// + /// Any other difference reads as another source: at worst a missed + /// redirect, so a withheld statement, never a false one. fn patched_by(&self, table: &str) -> bool { - fn git_key(url: &str) -> &str { - let url = url.trim().trim_end_matches('/'); - url.strip_suffix(".git").unwrap_or(url) - } match &self.source { DeclSource::Registry => match self.registry.as_deref() { None => is_crates_io_source(table), Some(reg) => reg.trim() == table, }, - DeclSource::Git(url) => !url.trim().is_empty() && git_key(url) == git_key(table), - _ => false, + DeclSource::RegistryIndex(url) => { + (is_crates_io_url_alias(url) && is_crates_io_source(table)) + || index_key(url).is_some_and(|k| index_key(table) == Some(k)) + } + DeclSource::Git(url) => git_key(url).is_some_and(|k| git_key(table) == Some(k)), + DeclSource::Path | DeclSource::Patch(_) => false, } } + /// "`` declares `` `` from ``", for + /// messages. + fn describe(&self, name: &str) -> String { + let req = self + .version + .as_deref() + .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); + format!("{} declares {name} {req} from {}", self.file, self.origin()) + } + /// Where cargo takes this dependency from, for messages. fn origin(&self) -> String { match (&self.source, self.registry.as_deref()) { @@ -582,6 +572,77 @@ impl Decl { } } +/// `url` with its scheme and host lowercased (both are case-insensitive), +/// trimmed, or `None` when empty. +fn lower_host(url: &str) -> Option { + let url = url.trim(); + if url.is_empty() { + return None; + } + let host_end = url.find("://").map(|s| s + 3).map_or(0, |start| { + start + url[start..].find('/').unwrap_or(url.len() - start) + }); + Some(format!( + "{}{}", + url[..host_end].to_ascii_lowercase(), + &url[host_end..] + )) +} + +/// A git repository URL as cargo's `CanonicalUrl` spells it: one trailing +/// `/` dropped, a GitHub URL lowercased whole (GitHub paths are +/// case-insensitive), a trailing `.git` dropped. `None` when empty. +fn git_key(url: &str) -> Option { + let mut url = lower_host(url)?; + if url.ends_with('/') { + url.pop(); + } + let github = url + .split_once("://") + .is_some_and(|(_, rest)| rest.split('/').next() == Some("github.com")); + if github { + url = url.to_ascii_lowercase(); + } + if let Some(stripped) = url.strip_suffix(".git") { + url = stripped.to_string(); + } + Some(url) +} + +/// A registry index URL for comparison: a `registry+` prefix (the lock's +/// spelling of a git index) dropped, a `sparse+` one kept (a sparse index +/// is another cargo source than the git index at the same URL), the host +/// lowercased and a trailing `/` dropped. `None` when empty. +fn index_key(url: &str) -> Option { + let url = url.trim(); + let url = url.strip_prefix("registry+").unwrap_or(url); + let (sparse, url) = match url.strip_prefix("sparse+") { + Some(rest) => ("sparse+", rest), + None => ("", url), + }; + let url = lower_host(url)?; + Some(format!("{sparse}{}", url.trim_end_matches('/'))) +} + +/// The source of a lock entry of `name` at exactly `version` that is not a +/// Socket patch server's: a copy cargo builds from crates.io, another +/// registry or git beside the patched one. +fn same_version_foreign_entry<'l>( + ctx: &DiscoverCtx<'_>, + lock: &'l Lock, + name: &str, + version: &str, +) -> Option<&'l str> { + lock.packages() + .iter() + .filter(|p| p.name == name && p.version == version) + .find_map(|p| { + p.source + .as_deref() + .filter(|s| source_uuid(ctx, s).is_none()) + }) +} + /// Crate name → its declarations ([`Decl`]). type Decls = BTreeMap>; @@ -932,13 +993,7 @@ fn beside_the_pin( out: &mut Discovery, ) { 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 { + if same_version_foreign_entry(ctx, lock, &pkg.name, &pkg.version).is_some() { return; } let others = distinct( @@ -986,13 +1041,7 @@ fn beside_the_pin( }; let declared = unpinned .iter() - .map(|(d, _)| { - let req = d - .version - .as_deref() - .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); - format!("{} declares {} {req} from {}", d.file, pkg.name, d.origin()) - }) + .map(|(d, _)| d.describe(&pkg.name)) .collect::>() .join(", and "); // What a re-scan can do: the hosted rewriter pins a crates.io @@ -1158,19 +1207,52 @@ fn unresolved_manifest_pins( ), ); } + // A well-formed Socket registry that pins nothing: say so, since the + // file reads like wiring. + let mut ineffective: Vec<(&str, String)> = Vec::new(); + for d in regs + .iter() + .filter(|d| d.socket_uuid().is_some() && (d.pin().is_none() || !d.live)) + { + let why = if d.live { + format!( + "cargo builds it from {}, whatever `registry` says", + d.origin() + ) + } else { + "no manifest inherits that `[workspace.dependencies]` entry".to_string() + }; + if !ineffective.contains(&(d.file.as_str(), why.clone())) { + ineffective.push((d.file.as_str(), why)); + } + } + for (file, why) in ineffective { + out.diag( + DIAG_REF_INVALID, + file, + format!("{file}: {name}'s Socket patch registry pins nothing there: {why}"), + ); + } 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 - } + // Patches the lock resolves the crate from, and those a ref + // already carries (the lock's, or a conflict pair beside it). + let locked_from: Vec = lock + .packages() + .iter() + .filter(|p| p.name == *name) + .filter_map(|p| p.source.as_deref().and_then(|s| source_uuid(ctx, s))) + .collect(); + let carried = |uuid: &str| { + locked_from.iter().any(|u| u == uuid) + || out.refs.iter().any(|r| { + r.uuid == uuid + && crate::utils::purl::purl_parts(&r.purl) + .is_some_and(|(ty, n, _)| ty == "cargo" && n == *name) + }) + }; + let uuids: Vec = uuids.into_iter().filter(|u| !carried(u)).collect(); for uuid in &uuids { let pinning: Vec<&Decl> = live .iter() @@ -1225,6 +1307,11 @@ fn unresolved_manifest_pins( } format!("there is no {CARGO_LOCK} to fix its version") } + Lock::Parsed { .. } if !locked_from.is_empty() => format!( + "{CARGO_LOCK} resolves it only from other Socket patches, at versions \ + this pin's requirement excludes (a stale lock: the next build \ + re-resolves this declaration)" + ), Lock::Parsed { .. } => format!( "{CARGO_LOCK} does not resolve it from a Socket patch server (a stale \ lock, or a patch server outside the allowlist — see \ @@ -1282,6 +1369,81 @@ async fn copy_tag(ctx: &DiscoverCtx<'_>, artifact_rel: &str, out: &mut Discovery } } +/// One `[patch.
]` entry that points at a Socket copy. +#[derive(Debug, Clone)] +struct WiredCopy { + table: String, + key: String, + path: String, + vref: VendorRef, +} + +/// Every Socket copy a file's `[patch]` tables point at. +fn socket_copies(doc: &DocumentMut) -> Vec { + patch_entries(doc) + .into_iter() + .filter_map(|e| { + let path = e.path?; + let vref = vendor_ref(path).filter(|v| v.eco == "cargo")?; + Some(WiredCopy { + table: e.source.to_string(), + key: e.key.to_string(), + path: path.to_string(), + vref, + }) + }) + .collect() +} + +/// Why cargo ignores a root-manifest `[patch]` entry, if it does: a +/// `[patch.""]` table replaces `[patch.crates-io]` +/// wholesale, and a project-config crates.io `[patch]` item with the same +/// key replaces the manifest's (unless it wires the same path, the +/// half-migrated shape attested through the config). Config entries are +/// never shadowed. +struct Shadow { + /// The manifest's `[patch.""]` table keys. + aliases: Vec, + /// The config's crates.io `[patch]` items: (file, key, path). + config_keys: Vec<(&'static str, String, Option)>, +} + +impl Shadow { + fn new(manifest: &DocumentMut, config: Option<&(&'static str, DocumentMut)>) -> Self { + let config_keys = config + .into_iter() + .flat_map(|(file, doc)| { + patch_entries(doc) + .into_iter() + .filter(|e| is_crates_io_source(e.source)) + .map(move |e| (*file, e.key.to_string(), e.path.map(same_path_key))) + }) + .collect(); + Shadow { + aliases: crates_io_url_alias_tables(manifest), + config_keys, + } + } + + fn why(&self, table: &str, key: &str, path: Option<&str>) -> Option { + if !is_crates_io_source(table) { + return None; + } + if let (true, Some(alias)) = (table == "crates-io", self.aliases.first()) { + return Some(format!( + "the `[patch.\"{alias}\"]` table replaces `[patch.crates-io]` in cargo" + )); + } + let own = path.map(same_path_key); + self.config_keys + .iter() + .find(|(_, k, p)| k == key && *p != own) + .map(|(file, k, _)| { + format!("{file} redefines `[patch]` key `{k}`, which replaces it in cargo") + }) + } +} + /// What a vendored copy is weighed against for copies its `[patch]` wiring /// does not redirect ([`patch_bypassed`]). struct Bypass<'a> { @@ -1289,30 +1451,37 @@ struct Bypass<'a> { /// Every declaration in the root and member manifests (`None`: no /// usable root manifest). decls: Option<&'a Decls>, - /// Every Socket copy the manifest and config `[patch]` tables point at, - /// with the `[patch.]` table pointing at it. - wired: &'a [(String, VendorRef)], + /// The `[patch]` entries pointing at a Socket copy that cargo honors + /// (a shadowed manifest entry redirects nothing). + wired: &'a [WiredCopy], +} + +/// Whether a `Cargo.lock` `source` is crates.io (its git or sparse index). +fn is_crates_io_lock_source(source: &str) -> bool { + is_crates_io_url_alias(source.trim().trim_start_matches("registry+")) + || index_key(source).as_deref() == Some("sparse+https://index.crates.io") } /// Mark the vendored ref `purl` (the copy `vref` of `name`@`version`) /// [`UnattestedKind::CargoPatchBypassed`] when the build may also compile -/// a copy of that version its wiring does not reach. A `[patch.]` -/// entry replaces the crate only in declarations from `` (v5 +/// a copy of that version its wiring does not reach. A `[patch.
]` +/// entry replaces the crate only in declarations from `
` (v5 /// `vendor` writes `[patch.crates-io]`), so either of these builds /// unpatched bytes: /// -/// * a live dependency declaration (the root's or a workspace member's) -/// from another registry, a `registry-index` or git, whose requirement -/// may select `version`, unless it is a registry or git dependency that a -/// `[patch]` table pointing at this copy redirects ([`Decl::patched_by`]): -/// cargo resolves it as a separate package, maybe at this very version; -/// * a `Cargo.lock` entry of `name` at exactly `version` with a source -/// (any registry or git; a Socket patch server's is hosted wiring, a ref -/// of its own): that copy is already locked beside the vendored one. +/// * a declaration (the root's or a workspace member's) that [`route`] +/// finds [`Route::Unpinned`] against the honored entries pointing at +/// this copy — the classifier hosted pins use, with this copy's tables +/// as the pins: a live dependency that may select `version` from a +/// source no such table redirects (another registry, a +/// `registry-index`, git, or crates.io when the copy is wired for +/// another source only). A path declaration (the user's own fork) is +/// left alone, as the lock's copy claim leaves its sourceless entry +/// alone; a `socket-patch-*` registry dependency is hosted wiring; +/// * a `Cargo.lock` entry of `name` at exactly `version` with a non-Socket +/// source: that copy is already locked beside the vendored one. /// -/// A path declaration (the user's own fork) is left alone, as the lock's -/// copy claim leaves its sourceless entry alone; a `socket-patch-*` -/// registry dependency is hosted wiring, weighed there. The ref stays +/// The detail ends with the remedy for its evidence. The ref stays /// (rollback, remove and the ledgers still see the wiring), but nothing /// attests it. fn patch_bypassed( @@ -1324,36 +1493,29 @@ fn patch_bypassed( purl: &str, out: &mut Discovery, ) { - let redirected = |d: &Decl| { + let pins: Vec<(&str, String)> = bypass + .wired + .iter() + .filter(|w| w.vref.uuid == vref.uuid && w.vref.leaf == vref.leaf) + .map(|w| (w.table.as_str(), vref.uuid.clone())) + .collect(); + let others = distinct( bypass - .wired + .lock + .packages() .iter() - .any(|(table, v)| v.uuid == vref.uuid && v.leaf == vref.leaf && d.patched_by(table)) - }; + .filter(|p| p.name == name && cargo_tag::strip_tag(&p.version) != version) + .map(|p| Some(cargo_tag::strip_tag(&p.version).to_string())), + ); let declared: Vec<&Decl> = bypass .decls .and_then(|d| d.get(name)) .into_iter() .flatten() - .filter(|d| d.live) - .filter(|d| match &d.source { - DeclSource::Registry => !d.socket_shaped() && !redirected(d), - DeclSource::Git(_) => !redirected(d), - DeclSource::RegistryIndex(_) => true, - DeclSource::Path | DeclSource::Patch(_) => false, - }) - .filter(|d| cargo_req_selects(d.version.as_deref(), version, &[]) != CargoReqMatch::NotOurs) + .filter(|d| d.source != DeclSource::Path) + .filter(|d| matches!(route(d, &pins, version, &others), Route::Unpinned(_))) .collect(); - let locked = bypass - .lock - .packages() - .iter() - .filter(|p| p.name == name && p.version == version) - .find_map(|p| { - p.source - .as_deref() - .filter(|s| source_uuid(ctx, s).is_none()) - }); + let locked = same_version_foreign_entry(ctx, bypass.lock, name, version); let redirects = format!( "which no [patch] entry pointing at {} redirects", vref.artifact_rel @@ -1365,13 +1527,7 @@ fn patch_bypassed( }); let declared = declared .iter() - .map(|d| { - let req = d - .version - .as_deref() - .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); - format!("{} declares {name} {req} from {}", d.file, d.origin()) - }) + .map(|d| d.describe(name)) .collect::>() .join(", and "); ( @@ -1379,17 +1535,38 @@ fn patch_bypassed( format!( "{declared}, {redirects}: cargo resolves that declaration as a separate \ package, so the crate declaring it builds an unpatched {name} that the \ - requirement allows to be {version}{evidence}" + requirement allows to be {version}{evidence}. Re-running `vendor` does \ + not change this: declare {name} from crates.io there (or point a \ + [patch] table for its source at the copy), or roll the patch back" + ), + ) + } + (None, Some(source)) => { + // A crates.io entry beside a copy `[patch.crates-io]` wires is + // stale (cargo would redirect it); any other is declared by + // some dependency, or left over from one that was removed. + let stale = is_crates_io_lock_source(source) + && pins.iter().any(|(table, _)| is_crates_io_source(table)); + let remedy = if stale { + format!( + "the entry is stale (`[patch.crates-io]` redirects crates.io), so \ + re-lock: re-run `socket-patch vendor`, or `cargo update -p {name}`" + ) + } else { + format!( + "`cargo tree -i {name}@{version}` names the dependency that declares it \ + (change that declaration or roll the patch back); if none does any \ + more, re-lock with `cargo update -p {name}`" + ) + }; + ( + CARGO_LOCK, + format!( + "{CARGO_LOCK} also locks {name} {version} from {source}, {redirects}: \ + cargo builds that copy unpatched beside the vendored one; {remedy}" ), ) } - (None, Some(source)) => ( - CARGO_LOCK, - format!( - "{CARGO_LOCK} also locks {name} {version} from {source}, {redirects}: cargo \ - builds that copy unpatched beside the vendored one" - ), - ), (None, None) => return, }; out.unattested( @@ -1401,14 +1578,15 @@ fn patch_bypassed( ); } -/// Vendored refs from one file's `[patch]` tables. `shadowed` names why -/// cargo ignores an entry (a higher-precedence item replaces it), if so. +/// Vendored refs from one file's `[patch]` tables. `shadow` (the root +/// manifest's) names why cargo ignores an entry (a higher-precedence item +/// replaces it), if so. async fn vendored_from_patches( ctx: &DiscoverCtx<'_>, file: &str, doc: &DocumentMut, lock: &Lock, - shadowed: &(dyn Fn(&CargoPatchEntry<'_>) -> Option + Sync), + shadow: Option<&Shadow>, bypass: &Bypass<'_>, out: &mut Discovery, ) { @@ -1420,7 +1598,7 @@ async fn vendored_from_patches( continue; // a user path, or the uuid-less `.socket/cargo-patches/` } let name = entry.name; - if let Some(why) = shadowed(&entry) { + if let Some(why) = shadow.and_then(|s| s.why(entry.source, entry.key, entry.path)) { out.diag( DIAG_REF_INVALID, file, @@ -2048,6 +2226,59 @@ mod tests { "{:#?}", out.diagnostics ); + // REVIEW: B's pin, which the lock does not reflect, is still + // diagnosed (it used to vanish once any Socket entry was locked). + assert!( + out.diagnostics + .iter() + .any(|d| d.code == DIAG_REF_UNATTRIBUTABLE + && d.file == std::path::Path::new("Cargo.toml") + && d.detail.contains(UUID_B) + && d.detail.contains("only from other Socket patches")), + "{:#?}", + out.diagnostics + ); + } + + /// REVIEW: a well-formed Socket registry that pins nothing — beside + /// `path` / `git` / `registry-index`, or on a `[workspace.dependencies]` + /// entry no manifest inherits — is diagnosed, since the file reads like + /// wiring. + #[tokio::test] + async fn an_ineffective_socket_registry_is_diagnosed() { + let reg = format!("registry = \"socket-patch-{UUID_A}\""); + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &format!("serde = {{ path = \"../serde\", version = \"1\", {reg} }}"), + &[], + ); + let out = run(&p).await; + assert!( + out.diagnostics.iter().any(|d| d.code == DIAG_REF_INVALID + && d.file == std::path::Path::new("crates/a/Cargo.toml") + && d.detail.contains("pins nothing there") + && d.detail.contains("a local path")), + "{:#?}", + out.diagnostics + ); + + let p = Project::new(); + p.write( + "Cargo.toml", + format!( + "[workspace]\nmembers = [\"crates/*\"]\n\n[workspace.dependencies]\n{}\n", + pinned("serde", "1", UUID_A) + ), + ); + member(&p, "a", "other = \"1\""); + let out = run(&p).await; + assert!( + out.diagnostics.iter().any(|d| d.code == DIAG_REF_INVALID + && d.file == std::path::Path::new("Cargo.toml") + && d.detail.contains("no manifest inherits")), + "{:#?}", + out.diagnostics + ); } /// A project-config definition of the pinned registry that indexes a @@ -3958,4 +4189,185 @@ mod tests { bypassed_mark(&out, "crates/a/Cargo.toml"); } } + + // ── review findings ────────────────────────────────────────────── + + /// The one vendored cfg-if ref of `out`: attested (no mark) or not. + fn assert_vendored_attested(out: &Discovery, attested: bool, what: &str) { + assert_refs( + out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + assert_eq!( + out.unattested.is_empty(), + attested, + "{what}: {:#?}", + out.unattested + ); + } + + /// REVIEW (false attestation): the project config redefines the + /// manifest's crates.io `[patch]` key with a git fork, so cargo ignores + /// the manifest's entry for the copy and builds the fork for crates.io + /// declarations. That shadowed entry used to count as redirecting them, + /// so the copy's other (honored) `[patch.my-corp]` entry was attested + /// beside the fork. + #[tokio::test] + async fn a_shadowed_manifest_entry_covers_no_declaration() { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let p = vendored_workspace( + "cfg-if = \"1\"", + &format!("\n[patch.my-corp]\ncfg-if = {{ path = \"{rel}\" }}\n"), + &[], + ); + p.write( + ".cargo/config.toml", + "[patch.crates-io]\ncfg-if-socket-aaaaaaaa = { package = \"cfg-if\", git = \ + \"https://example.com/fork/cfg-if\" }\n", + ); + let out = run(&p).await; + assert_vendored_attested(&out, false, "shadowed"); + let u = bypassed_mark(&out, "Cargo.toml"); + assert!( + u.detail + .contains("Cargo.toml declares cfg-if \"1\" from crates.io"), + "{}", + u.detail + ); + assert!( + diag_codes(&out).contains(&DIAG_REF_INVALID), + "{:#?}", + out.diagnostics + ); + } + + /// REVIEW: a `registry-index` declaration is redirected by a + /// `[patch.""]` table for the same index (host case, a + /// trailing `/` and the lock's `registry+` spelling aside; `sparse+` is + /// another source), and one naming the crates.io git index is + /// crates.io. The sparse crates.io index stays another source: whether + /// `[patch.crates-io]` reaches it is not what the files show, so it is + /// withheld rather than risked. + #[tokio::test] + async fn a_registry_index_patch_table_covers_that_index() { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let table = |key: &str| format!("\n[patch.\"{key}\"]\ncfg-if = {{ path = \"{rel}\" }}\n"); + let decl = + |index: &str| format!("cfg-if = {{ version = \"1\", registry-index = \"{index}\" }}"); + for (member, extra, attested) in [ + ( + decl("https://My-Corp.example/index/"), + table("https://my-corp.example/index"), + true, + ), + ( + decl("sparse+https://my-corp.example/index/"), + table("sparse+https://my-corp.example/index"), + true, + ), + ( + decl("https://github.com/rust-lang/crates.io-index"), + String::new(), + true, + ), + ( + decl("sparse+https://my-corp.example/index/"), + table("https://my-corp.example/index"), + false, + ), + ( + decl("https://other.example/index"), + table("https://my-corp.example/index"), + false, + ), + ( + decl("sparse+https://index.crates.io/"), + String::new(), + false, + ), + ] { + let out = run(&vendored_workspace(&member, &extra, &[])).await; + assert_vendored_attested(&out, attested, &member); + } + } + + /// REVIEW: the remedy follows the evidence — a declaration cannot be + /// fixed by re-vendoring; a crates.io lock entry beside a + /// `[patch.crates-io]` copy is stale (re-lock); another source's entry + /// is some dependency's (`cargo tree -i` finds it). + #[tokio::test] + async fn the_bypass_remedy_follows_its_evidence() { + let out = run(&vendored_workspace( + "cfg-if = { version = \"1\", registry = \"my-corp\" }", + "", + &[], + )) + .await; + let u = bypassed_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail + .contains("Re-running `vendor` does not change this"), + "{}", + u.detail + ); + + let out = run(&vendored_workspace( + "cfg-if = \"1\"", + "", + &[("1.0.4", CRATES_IO)], + )) + .await; + let u = bypassed_mark(&out, "Cargo.lock"); + assert!( + u.detail.contains("stale") + && u.detail.contains("cargo update -p cfg-if") + && !u.detail.contains("Re-running `vendor` does not"), + "{}", + u.detail + ); + + let out = run(&vendored_workspace( + "cfg-if = \"1\"", + "", + &[("1.0.4", MY_CORP)], + )) + .await; + let u = bypassed_mark(&out, "Cargo.lock"); + assert!( + u.detail.contains("cargo tree -i cfg-if@1.0.4") && !u.detail.contains("stale"), + "{}", + u.detail + ); + } + + /// REVIEW: git URLs compare as cargo's `CanonicalUrl` spells them — a + /// GitHub URL case-insensitively, any host's case aside, one trailing + /// `/` and `.git` dropped — so another path on a non-GitHub host is + /// another repository. + #[tokio::test] + async fn a_git_patch_table_matches_cargos_canonical_url() { + let rel = vendor_path(UUID_A, "cfg-if-1.0.4"); + let table = |key: &str| format!("\n[patch.\"{key}\"]\ncfg-if = {{ path = \"{rel}\" }}\n"); + let decl = |url: &str| format!("cfg-if = {{ git = \"{url}\" }}"); + for (member, extra, attested) in [ + ( + decl("https://GitHub.com/Foo/CFG-IF.git"), + table("https://github.com/foo/cfg-if"), + true, + ), + ( + decl("https://EXAMPLE.com/foo/cfg-if/"), + table("https://example.com/foo/cfg-if"), + true, + ), + ( + decl("https://example.com/Foo/cfg-if"), + table("https://example.com/foo/cfg-if"), + false, + ), + ] { + let out = run(&vendored_workspace(&member, &extra, &[])).await; + assert_vendored_attested(&out, attested, &member); + } + } } diff --git a/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-cargo.json b/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-cargo.json index 99be573f0..c14a317f3 100644 --- a/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-cargo.json +++ b/crates/socket-patch-core/tests/fixtures/vex-discover-golden/redirect-cargo.json @@ -634,7 +634,13 @@ "lockfile_basis_ok": true } ], - "diagnostics": [], + "diagnostics": [ + { + "code": "patched_ref_invalid", + "file": "Cargo.toml", + "detail": "Cargo.toml: serde's Socket patch registry pins nothing there: no manifest inherits that `[workspace.dependencies]` entry" + } + ], "recognized": [ { "uuid": "11111111-1111-1111-1111-111111111111", From 52230be5f463b3343c00b998461aaa4e0ec68aa9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:55:33 +0000 Subject: [PATCH 4/9] Let a shadowed [patch] Socket pin pin no hosted declaration FALSE ATTESTATION (found reviewing #2019; fixed here, where `Shadow` lives): a root `[patch.crates-io]` Socket registry pin cargo ignores (a `[patch.""]` table replaces it, or the config redefines the key) still counted as pinning every crates.io declaration of the crate, so a member's plain declaration was taken as routed to the patch and the hosted ref attested while cargo builds crates.io. Such an entry stays a declaration (Socket's wiring is present, the ref stays) but is not live, and `route` only weighs live `[patch]` pins, as the vendored bypass check already weighs only honored entries. Co-Authored-By: Claude Opus 5.5 --- .../src/vex/discover/cargo.rs | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 83e652565..1404a98dc 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -221,10 +221,14 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { for patch in patch_entries(doc) { if let Some(reg) = patch.registry { let name = patch.name.to_string(); - decls - .entry(name) - .or_default() - .push(Decl::patch(CARGO_TOML, patch.source, reg)); + // An entry cargo ignores (a crates.io URL table replaces + // `[patch.crates-io]`; the config redefines the key) is + // Socket's wiring still (the ref stays) but pins nothing. + let mut decl = Decl::patch(CARGO_TOML, patch.source, reg); + decl.live = shadow + .as_ref() + .is_none_or(|s| s.why(patch.source, patch.key, None).is_none()); + decls.entry(name).or_default().push(decl); } } decls @@ -995,7 +999,7 @@ fn beside_the_pin( let patch_pins: Vec<(&str, String)> = regs .iter() .filter_map(|d| match &d.source { - DeclSource::Patch(table) => Some((table.as_str(), d.socket_uuid()?)), + DeclSource::Patch(table) if d.live => Some((table.as_str(), d.socket_uuid()?)), _ => None, }) .collect(); @@ -2444,6 +2448,36 @@ mod tests { p } + /// Review on #2019 (round 2, fixed here where `Shadow` lives): a + /// `[patch.crates-io]` Socket pin that cargo ignores (a crates.io URL + /// table replaces it) pins no declaration: the member's crates.io + /// declaration is unpinned, so the ref is not attested. + #[tokio::test] + async fn a_shadowed_patch_pin_pins_no_declaration() { + let patch = format!( + "serde = \"1\"\n\n[patch.crates-io]\nserde = {{ registry = \"socket-patch-{UUID_A}\" }}\n" + ); + let p = mixed_workspace(&patch, "serde = \"1\"", &[]); + let out = run(&p).await; + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + let shadowed = format!( + "{patch}\n[patch.\"https://github.com/rust-lang/crates.io-index\"]\nother = {{ path = \"other\" }}\n" + ); + let p = mixed_workspace(&shadowed, "serde = \"1\"", &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + assert!( + out.unattested + .iter() + .any(|u| u.kind == UnattestedKind::CargoUnpinnedDeclaration), + "{:#?}", + out.unattested + ); + } + /// 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 { From a7611ef32633e17169a67cc6579797e6584f8d46 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 10:25:05 +0000 Subject: [PATCH 5/9] Shadow a manifest [patch] item by any config item for its source and key Cargo replaces a manifest [patch] item with a project-config item of the same source and key, whatever the source table and whether either wires a path. Shadow only looked at crates.io tables and treated two path-less items (a registry pin and a git fork) as the same wiring, so a config fork left the manifest's Socket pin live and its stale lock row could be attested. Only an identical path is now the same wiring, and named and URL tables are compared too (URLs as an index key). The ineffective-registry diagnostic also called a shadowed [patch] pin an uninherited [workspace.dependencies] template; it now says cargo ignores the [patch] entry. Addresses Bugbot 4501098254, 4501098256 and 4501098258. Co-Authored-By: Claude --- .../src/vex/discover/cargo.rs | 80 ++++++++++++++++--- 1 file changed, 67 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 1404a98dc..f94a11f17 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -1217,6 +1217,9 @@ fn unresolved_manifest_pins( "cargo builds it from {}, whatever `registry` says", d.origin() ) + } else if matches!(d.source, DeclSource::Patch(_)) { + "cargo ignores that `[patch]` entry (a crates.io URL table or the project config replaces it)" + .to_string() } else { "no manifest inherits that `[workspace.dependencies]` entry".to_string() }; @@ -1395,15 +1398,27 @@ fn socket_copies(doc: &DocumentMut) -> Vec { /// Why cargo ignores a root-manifest `[patch]` entry, if it does: a /// `[patch.""]` table replaces `[patch.crates-io]` -/// wholesale, and a project-config crates.io `[patch]` item with the same -/// key replaces the manifest's (unless it wires the same path, the +/// wholesale, and a project-config `[patch]` item for the same source and +/// key replaces the manifest's (unless both wire the same path, the /// half-migrated shape attested through the config). Config entries are /// never shadowed. struct Shadow { /// The manifest's `[patch.""]` table keys. aliases: Vec, - /// The config's crates.io `[patch]` items: (file, key, path). - config_keys: Vec<(&'static str, String, Option)>, + /// The config's `[patch]` items: (file, source table, key, path). + config_keys: Vec<(&'static str, String, String, Option)>, +} + +/// Whether two `[patch.]` table names patch the same source: +/// crates.io under any spelling, else the same name or URL. +fn same_patch_source(a: &str, b: &str) -> bool { + if is_crates_io_source(a) || is_crates_io_source(b) { + return is_crates_io_source(a) && is_crates_io_source(b); + } + // A URL table compares as an index (host case, a trailing `/`); a + // registry name verbatim. + let norm = |s: &str| index_key(s).unwrap_or_else(|| s.trim().to_string()); + norm(a) == norm(b) } impl Shadow { @@ -1411,10 +1426,14 @@ impl Shadow { let config_keys = config .into_iter() .flat_map(|(file, doc)| { - patch_entries(doc) - .into_iter() - .filter(|e| is_crates_io_source(e.source)) - .map(move |e| (*file, e.key.to_string(), e.path.map(same_path_key))) + patch_entries(doc).into_iter().map(move |e| { + ( + *file, + e.source.to_string(), + e.key.to_string(), + e.path.map(same_path_key), + ) + }) }) .collect(); Shadow { @@ -1424,19 +1443,20 @@ impl Shadow { } fn why(&self, table: &str, key: &str, path: Option<&str>) -> Option { - if !is_crates_io_source(table) { - return None; - } if let (true, Some(alias)) = (table == "crates-io", self.aliases.first()) { return Some(format!( "the `[patch.\"{alias}\"]` table replaces `[patch.crates-io]` in cargo" )); } + // Only an identical path is the same wiring: two path-less items (a + // registry pin and a git fork, say) are not. let own = path.map(same_path_key); self.config_keys .iter() - .find(|(_, k, p)| k == key && *p != own) - .map(|(file, k, _)| { + .find(|(_, src, k, p)| { + k == key && same_patch_source(src, table) && (own.is_none() || *p != own) + }) + .map(|(file, _, k, _)| { format!("{file} redefines `[patch]` key `{k}`, which replaces it in cargo") }) } @@ -4300,6 +4320,40 @@ mod tests { ); } + /// REVIEW (Bugbot on #2024): a config item for the same key replaces + /// the manifest's `[patch]` item in cargo whatever the source table and + /// whether or not either wires a path: a path-less config git fork + /// shadows a path-less manifest registry pin, and a `[patch.my-corp]` + /// config item shadows the manifest's `[patch.my-corp]` copy. + #[test] + fn config_items_shadow_pathless_and_named_source_entries() { + let manifest: toml_edit::DocumentMut = "[package]\nname = \"x\"\n".parse().unwrap(); + let config: toml_edit::DocumentMut = "[patch.crates-io]\n\ + cfg-if-socket-aaaaaaaa = { package = \"cfg-if\", git = \"https://example.com/f\" }\n\ + [patch.my-corp]\n\ + cfg-if = { git = \"https://example.com/f\" }\n\ + [patch.\"https://My-Corp.example/index/\"]\n\ + cfg-if = { path = \"../fork\" }\n" + .parse() + .unwrap(); + let cfg = (".cargo/config.toml", config); + let s = super::Shadow::new(&manifest, Some(&cfg)); + // A path-less registry pin under the same key: shadowed. + assert!(s.why("crates-io", "cfg-if-socket-aaaaaaaa", None).is_some()); + // The same key under another source: not. + assert!(s.why("other", "cfg-if-socket-aaaaaaaa", None).is_none()); + // A named non-crates.io table: shadowed. + assert!(s.why("my-corp", "cfg-if", Some("vendor/x")).is_some()); + // A URL table, compared as an index. + assert!(s + .why("https://my-corp.example/index", "cfg-if", Some("vendor/x")) + .is_some()); + // The half-migrated shape: the same path is the same wiring. + assert!(s + .why("https://my-corp.example/index", "cfg-if", Some("../fork")) + .is_none()); + } + /// REVIEW: a `registry-index` declaration is redirected by a /// `[patch.""]` table for the same index (host case, a /// trailing `/` and the lock's `registry+` spelling aside; `sparse+` is From 3d413af3f9c78367f7a957d5e5d444b303189956 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 7 Nov 2026 10:35:25 +0000 Subject: [PATCH 6/9] Compare [patch] URL tables the way cargo canonicalizes them Shadow compared a config's [patch.""] table with the manifest's through index_key, which keeps a trailing .git and a GitHub path's case. Cargo canonicalizes both, so a config item under a canonical-equivalent git URL replaced the manifest's Socket copy while discovery still counted it. Non-sparse URL tables now compare through git_key; sparse+ ones stay index keys (another source). Addresses Bugbot 4501313607. Co-Authored-By: Claude --- .../src/vex/discover/cargo.rs | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index f94a11f17..f3db4956e 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -1415,9 +1415,20 @@ fn same_patch_source(a: &str, b: &str) -> bool { if is_crates_io_source(a) || is_crates_io_source(b) { return is_crates_io_source(a) && is_crates_io_source(b); } - // A URL table compares as an index (host case, a trailing `/`); a - // registry name verbatim. - let norm = |s: &str| index_key(s).unwrap_or_else(|| s.trim().to_string()); + // A URL table compares as cargo's `CanonicalUrl` spells it (host case, + // a trailing `/` or `.git`, a GitHub path's case; `sparse+` kept as + // another source); a registry name verbatim. + let norm = |s: &str| { + let t = s.trim(); + let key = if t.starts_with("sparse+") { + index_key(t) + } else if t.contains("://") { + git_key(t.strip_prefix("registry+").unwrap_or(t)) + } else { + None + }; + key.unwrap_or_else(|| t.to_string()) + }; norm(a) == norm(b) } @@ -4348,6 +4359,23 @@ mod tests { assert!(s .why("https://my-corp.example/index", "cfg-if", Some("vendor/x")) .is_some()); + // A canonical-equivalent git table (Bugbot on a7611ef). + let config: toml_edit::DocumentMut = + "[patch.\"https://GitHub.com/Corp/Fork.git/\"]\ncfg-if = { git = \"https://example.com/f\" }\n" + .parse() + .unwrap(); + let cfg2 = (".cargo/config.toml", config); + let s2 = super::Shadow::new(&manifest, Some(&cfg2)); + assert!(s2 + .why("https://github.com/corp/fork", "cfg-if", Some("vendor/x")) + .is_some()); + assert!(s2 + .why( + "sparse+https://github.com/corp/fork", + "cfg-if", + Some("vendor/x") + ) + .is_none()); // The half-migrated shape: the same path is the same wiring. assert!(s .why("https://my-corp.example/index", "cfg-if", Some("../fork")) From 3695ddc6f80da135e84d891dde0748b12983b1a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 06:27:31 +0000 Subject: [PATCH 7/9] Withhold a vendored Cargo copy a stale pin to another patch bypasses patch_bypassed only withheld the vendored ref for declarations route() finds Unpinned. A member declaring the crate from another socket-patch registry routes to Pin(other) and was skipped. When the lock resolves the crate from that patch, its hosted ref sits beside the vendored one and the CLI's wiring_conflict gate refuses both. When the lock is stale and holds no entry from that patch, only a patched_ref_unattributable diagnostic was left, and patch A was attested even though the member re-resolves to patch B. Mark the vendored copy CargoPatchBypassed in that case instead. Co-Authored-By: Claude --- .../src/vex/discover/cargo.rs | 94 ++++++++++++++++++- 1 file changed, 93 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 3e927cd68..6c79a9934 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -1545,6 +1545,29 @@ fn patch_bypassed( .filter(|d| d.source != DeclSource::Path) .filter(|d| matches!(route(d, &pins, version, &others), Route::Unpinned(_))) .collect(); + // A declaration pinned to ANOTHER Socket patch builds that patch's + // copy, not this one. When the lock resolves the crate from that patch, + // its hosted ref stands beside this one and the CLI's conflict gate + // refuses both; when it does not (a stale lock), nothing else says so. + let locked_from: Vec = bypass + .lock + .packages() + .iter() + .filter(|p| p.name == name) + .filter_map(|p| p.source.as_deref().and_then(|s| source_uuid(ctx, s))) + .collect(); + let other_patch: Vec<(&Decl, String)> = bypass + .decls + .and_then(|d| d.get(name)) + .into_iter() + .flatten() + .filter_map(|d| match route(d, &pins, version, &others) { + Route::Pin(uuid) if uuid != vref.uuid && !locked_from.contains(&uuid) => { + Some((d, uuid)) + } + _ => None, + }) + .collect(); let locked = same_version_foreign_entry(ctx, bypass.lock, name, version); let redirects = format!( "which no [patch] entry pointing at {} redirects", @@ -1597,7 +1620,28 @@ fn patch_bypassed( ), ) } - (None, None) => return, + (None, None) => { + let Some((first, _)) = other_patch.first() else { + return; + }; + let declared = other_patch + .iter() + .map(|(d, uuid)| format!("{} (Socket patch {uuid})", d.describe(name))) + .collect::>() + .join(", and "); + ( + first.file.as_str(), + format!( + "{declared}, which {CARGO_LOCK} does not resolve {name} from (a stale \ + lock): cargo re-resolves that declaration from that patch's registry, so \ + the crate declaring it builds {name} with another patch than the copy at \ + {}, and the requirement allows it to be {version}. Keep one patch for \ + {name}: pin that declaration to this patch (or drop it), or roll this \ + patch back", + vref.artifact_rel + ), + ) + } }; out.unattested( purl, @@ -4534,4 +4578,52 @@ mod tests { assert_vendored_attested(&out, attested, &member); } } + /// REGRESSION (Bugbot on #2024): a member pinned to ANOTHER Socket + /// patch builds that patch's copy. When the lock resolves cfg-if from + /// it, its hosted ref stands beside the vendored one and the CLI's + /// conflict gate refuses both; when the lock does not (stale), the + /// vendored copy is marked bypassed rather than attested alone. + #[tokio::test] + async fn a_member_pinned_to_another_patch_bypasses_the_vendored_copy() { + let p = vendored_workspace(&pinned("cfg-if", "1", UUID_B), "", &[]); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored)], + ); + let u = bypassed_mark(&out, "crates/a/Cargo.toml"); + assert!( + u.detail.contains("crates/a/Cargo.toml declares cfg-if") + && u.detail.contains(UUID_B) + && u.detail.contains("stale"), + "{}", + u.detail + ); + + // Locked from that patch: the conflict pair, no bypass mark on A. + let b = index(UUID_B); + let p = vendored_workspace(&pinned("cfg-if", "1", UUID_B), "", &[("1.0.4", b.as_str())]); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:cargo/cfg-if@1.0.4", UUID_B, WiringMode::Hosted), + ("pkg:cargo/cfg-if@1.0.4", UUID_A, WiringMode::Vendored), + ], + ); + assert!( + out.unattested.iter().all(|u| u.uuid != UUID_A), + "{:#?}", + out.unattested + ); + + // A requirement that excludes the vendored version says nothing. + let p = vendored_workspace(&pinned("cfg-if", "0.1", UUID_B), "", &[]); + let out = run(&p).await; + assert!( + out.unattested.iter().all(|u| u.uuid != UUID_A), + "{:#?}", + out.unattested + ); + } } From bc39a271be43ab138c595e9983e12639a5c5edc3 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 9 Nov 2026 06:41:50 +0000 Subject: [PATCH 8/9] Pair the stale-pin bypass with an emitted ref, not any lock line The exemption for a member pinned to another Socket patch trusted any lock line from that patch. hosted_from_lock drops a line whose project registry definition disagrees, and a line at another version is no ref for this package, so wiring_conflicts never fired and the vendored copy stayed attested. Exempt only when the hosted pass emitted a ref for that patch at this very purl that is not itself withheld. Co-Authored-By: Claude --- .../src/vex/discover/cargo.rs | 55 +++++++++++++------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index 6c79a9934..63b9267f5 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -1546,25 +1546,28 @@ fn patch_bypassed( .filter(|d| matches!(route(d, &pins, version, &others), Route::Unpinned(_))) .collect(); // A declaration pinned to ANOTHER Socket patch builds that patch's - // copy, not this one. When the lock resolves the crate from that patch, - // its hosted ref stands beside this one and the CLI's conflict gate - // refuses both; when it does not (a stale lock), nothing else says so. - let locked_from: Vec = bypass - .lock - .packages() - .iter() - .filter(|p| p.name == name) - .filter_map(|p| p.source.as_deref().and_then(|s| source_uuid(ctx, s))) - .collect(); + // copy, not this one. Only a ref for that patch at this very package, + // emitted by the hosted pass (from the lock, or a conflict pair beside + // a lock pin) and not itself withheld, puts the pair before the CLI's + // conflict gate; otherwise (a stale lock, a lock line whose config + // index disagrees, another locked version) nothing else says so. + let gated = |uuid: &str| { + out.refs.iter().any(|r| { + r.uuid == uuid + && r.purl == purl + && !out + .unattested + .iter() + .any(|u| u.uuid == uuid && u.purl == purl && u.file == r.source_file) + }) + }; let other_patch: Vec<(&Decl, String)> = bypass .decls .and_then(|d| d.get(name)) .into_iter() .flatten() .filter_map(|d| match route(d, &pins, version, &others) { - Route::Pin(uuid) if uuid != vref.uuid && !locked_from.contains(&uuid) => { - Some((d, uuid)) - } + Route::Pin(uuid) if uuid != vref.uuid && !gated(&uuid) => Some((d, uuid)), _ => None, }) .collect(); @@ -1632,10 +1635,11 @@ fn patch_bypassed( ( first.file.as_str(), format!( - "{declared}, which {CARGO_LOCK} does not resolve {name} from (a stale \ - lock): cargo re-resolves that declaration from that patch's registry, so \ - the crate declaring it builds {name} with another patch than the copy at \ - {}, and the requirement allows it to be {version}. Keep one patch for \ + "{declared}, which no wiring of that patch for {name} {version} pairs \ + with this copy (a stale {CARGO_LOCK}, or a registry definition that \ + disagrees): cargo resolves that declaration from that patch's registry, \ + so the crate declaring it builds {name} with another patch than the copy \ + at {}, and the requirement allows it to be {version}. Keep one patch for \ {name}: pin that declaration to this patch (or drop it), or roll this \ patch back", vref.artifact_rel @@ -4595,7 +4599,7 @@ mod tests { assert!( u.detail.contains("crates/a/Cargo.toml declares cfg-if") && u.detail.contains(UUID_B) - && u.detail.contains("stale"), + && u.detail.contains("stale Cargo.lock"), "{}", u.detail ); @@ -4617,6 +4621,21 @@ mod tests { out.unattested ); + // Locked from that patch, but the project's registry definition + // indexes another patch: the hosted pass emits no ref for it, so + // nothing pairs with the vendored copy and it is withheld. + let p = vendored_workspace(&pinned("cfg-if", "1", UUID_B), "", &[("1.0.4", b.as_str())]); + p.write( + ".cargo/config.toml", + format!( + "[registries.socket-patch-{UUID_B}]\nindex = \"{}\"\n", + index(UUID_A) + ), + ); + let out = run(&p).await; + assert!(out.refs.iter().all(|r| r.uuid != UUID_B), "{:#?}", out.refs); + bypassed_mark(&out, "crates/a/Cargo.toml"); + // A requirement that excludes the vendored version says nothing. let p = vendored_workspace(&pinned("cfg-if", "0.1", UUID_B), "", &[]); let out = run(&p).await; From f87fc9f77d589490c276b3057cf80846f3c9e4cd Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Mon, 9 Nov 2026 16:01:34 -0500 Subject: [PATCH 9/9] Fix review findings on Cargo [patch] shadowing and bypass details Review on #2024 (/code-review high), round 4: - The "resolves it only from other Socket patches" diagnostic fired beside a same-version crates.io twin; it now needs a lock with no non-Socket entry of the crate. - Shadow resolves a [patch.] table through the config's [registries] index, so a config [patch.""] item shadows it. - An identical path-less Socket registry [patch] pin in the manifest and the config is the same wiring, not a shadow. - patched_by (registry-index) and same_patch_source share patch_source_key, with CanonicalUrl rules on both. - patch_bypassed's conflict-gate check matches the CLI's bypassed_ref (file-scoped marks only); it routes each declaration once; its detail also names other-patch declarations; a git lock entry a git [patch] table redirects gets the stale/re-lock remedy. - Reuse patch::redirect::is_crates_io_source for lock sources. - Move the manifests comment back above its vector. Co-Authored-By: Claude Opus 5.5 --- .../src/patch/redirect/mod.rs | 2 +- .../src/vex/discover/cargo.rs | 297 ++++++++++++++---- 2 files changed, 229 insertions(+), 70 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 92ed9d4c4..d2f21c1af 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -2538,7 +2538,7 @@ fn cargo_manifest_package_id( /// in either the git-index or the sparse spelling. Compared the way cargo's /// `CanonicalUrl` does: host casing, a trailing `/` and a `.git` suffix do /// not make a different source (and github paths are case-insensitive). -fn is_crates_io_source(source: &str) -> bool { +pub(crate) fn is_crates_io_source(source: &str) -> bool { let url = source.trim(); let url = url .strip_prefix("registry+") diff --git a/crates/socket-patch-core/src/vex/discover/cargo.rs b/crates/socket-patch-core/src/vex/discover/cargo.rs index e624ed5ef..564be0ec4 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -178,8 +178,6 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { let lock = load_lock(ctx, 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). // Every `[patch]` entry pointing at a Socket copy, the root manifest's // (with why cargo ignores it, if it does) and then the config's. let mut wired_all: Vec<(WiredCopy, Option)> = Vec::new(); @@ -187,7 +185,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if let Some(doc) = &root { let s = Shadow::new(doc, config.as_ref()); wired_all.extend(socket_copies(doc).into_iter().map(|w| { - let why = s.why(&w.table, &w.key, Some(&w.path)); + let why = s.why(&w.table, &w.key, Some(&w.path), None); (w, why) })); shadow = Some(s); @@ -195,6 +193,8 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if let Some((_, doc)) = &config { wired_all.extend(socket_copies(doc).into_iter().map(|w| (w, None))); } + // The root manifest, then every workspace member's — the manifests the + // hosted rewriter pins a crate in (each that declares it). let mut manifests: Vec<(String, DocumentMut)> = Vec::new(); if let Some(doc) = root { // Any Socket pin the root or the config carries, a `[patch]` @@ -227,7 +227,7 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { let mut decl = Decl::patch(CARGO_TOML, patch.source, reg); decl.live = shadow .as_ref() - .is_none_or(|s| s.why(patch.source, patch.key, None).is_none()); + .is_none_or(|s| s.why(patch.source, patch.key, None, Some(reg)).is_none()); decls.entry(name).or_default().push(decl); } } @@ -523,7 +523,8 @@ impl Decl { /// * a registry dependency of that registry (`crates-io`, or a /// crates.io URL alias, for one with no `registry` or /// `registry = "crates-io"`, which cargo reads as crates.io too); - /// * a `registry-index` dependency of that index URL ([`index_key`]); + /// * a `registry-index` dependency of that index URL + /// ([`patch_source_key`], the spelling `[patch]` tables compare in); /// one naming the crates.io git index is crates.io itself (the same /// cargo source), so `[patch.crates-io]` redirects it too; /// * a git dependency of that repository ([`git_key`], cargo's @@ -539,7 +540,7 @@ impl Decl { }, DeclSource::RegistryIndex(url) => { (is_crates_io_url_alias(url) && is_crates_io_source(table)) - || index_key(url).is_some_and(|k| index_key(table) == Some(k)) + || patch_source_key(url) == patch_source_key(table) } DeclSource::Git(url) => git_key(url).is_some_and(|k| git_key(table) == Some(k)), DeclSource::Path | DeclSource::Patch(_) => false, @@ -1293,6 +1294,15 @@ fn unresolved_manifest_pins( .filter(|p| p.name == *name) .filter_map(|p| p.source.as_deref().and_then(|s| source_uuid(ctx, s))) .collect(); + // Whether the lock also resolves the crate from a non-Socket source + // (or a path): then "only from other Socket patches" is not what it + // says (review on #2024). + let locked_elsewhere = lock.packages().iter().any(|p| { + p.name == *name + && p.source + .as_deref() + .is_none_or(|s| source_uuid(ctx, s).is_none()) + }); let carried = |uuid: &str| { locked_from.iter().any(|u| u == uuid) || out.refs.iter().any(|r| { @@ -1356,7 +1366,7 @@ fn unresolved_manifest_pins( } format!("there is no {CARGO_LOCK} to fix its version") } - Lock::Parsed { .. } if !locked_from.is_empty() => format!( + Lock::Parsed { .. } if !locked_from.is_empty() && !locked_elsewhere => format!( "{CARGO_LOCK} resolves it only from other Socket patches, at versions \ this pin's requirement excludes (a stale lock: the next build \ re-resolves this declaration)" @@ -1453,31 +1463,42 @@ fn socket_copies(doc: &DocumentMut) -> Vec { struct Shadow { /// The manifest's `[patch.""]` table keys. aliases: Vec, - /// The config's `[patch]` items: (file, source table, key, path). - config_keys: Vec<(&'static str, String, String, Option)>, + /// The config's `[patch]` items. + config_keys: Vec, + /// The config's `[registries.] index` values: a `[patch.]` + /// table patches the same source as `[patch.""]`. + registries: BTreeMap, +} + +/// One config `[patch]` item as [`Shadow`] compares it: (file, source +/// table, key, path, registry). +type ConfigPatchItem = (&'static str, String, String, Option, Option); + +/// A `[patch.]` table name, a `registry-index` or a lock `source` +/// as cargo's `CanonicalUrl` spells it, for comparison: a URL's host case, +/// a trailing `/` or `.git` and a GitHub path's case ignored, a lock's +/// `registry+` prefix dropped, `sparse+` kept (another source); a registry +/// name verbatim. +fn patch_source_key(source: &str) -> String { + let t = source.trim(); + let key = if t.starts_with("sparse+") { + index_key(t) + } else if t.contains("://") { + git_key(t.strip_prefix("registry+").unwrap_or(t)) + } else { + None + }; + key.unwrap_or_else(|| t.to_string()) } /// Whether two `[patch.]` table names patch the same source: -/// crates.io under any spelling, else the same name or URL. +/// crates.io under any spelling, else the same name or URL +/// ([`patch_source_key`]). fn same_patch_source(a: &str, b: &str) -> bool { if is_crates_io_source(a) || is_crates_io_source(b) { return is_crates_io_source(a) && is_crates_io_source(b); } - // A URL table compares as cargo's `CanonicalUrl` spells it (host case, - // a trailing `/` or `.git`, a GitHub path's case; `sparse+` kept as - // another source); a registry name verbatim. - let norm = |s: &str| { - let t = s.trim(); - let key = if t.starts_with("sparse+") { - index_key(t) - } else if t.contains("://") { - git_key(t.strip_prefix("registry+").unwrap_or(t)) - } else { - None - }; - key.unwrap_or_else(|| t.to_string()) - }; - norm(a) == norm(b) + patch_source_key(a) == patch_source_key(b) } impl Shadow { @@ -1491,31 +1512,67 @@ impl Shadow { e.source.to_string(), e.key.to_string(), e.path.map(same_path_key), + e.registry.map(|r| r.trim().to_string()), ) }) }) .collect(); + let registries = config + .and_then(|(_, doc)| doc.get("registries")) + .and_then(toml_edit::Item::as_table_like) + .into_iter() + .flat_map(|t| t.iter()) + .filter_map(|(name, item)| { + let index = item.as_table_like()?.get("index")?.as_str()?; + Some((name.to_string(), index.to_string())) + }) + .collect(); Shadow { aliases: crates_io_url_alias_tables(manifest), config_keys, + registries, } } - fn why(&self, table: &str, key: &str, path: Option<&str>) -> Option { + /// `table`, a registry name resolved to its configured index URL. + fn resolved<'s>(&'s self, table: &'s str) -> &'s str { + self.registries + .get(table.trim()) + .map_or(table, String::as_str) + } + + /// Why cargo ignores the manifest's `[patch.
]` item `key` + /// (wiring `path`, or Socket registry `registry`), if it does. + fn why( + &self, + table: &str, + key: &str, + path: Option<&str>, + registry: Option<&str>, + ) -> Option { if let (true, Some(alias)) = (table == "crates-io", self.aliases.first()) { return Some(format!( "the `[patch.\"{alias}\"]` table replaces `[patch.crates-io]` in cargo" )); } - // Only an identical path is the same wiring: two path-less items (a - // registry pin and a git fork, say) are not. + // Only identical wiring is the same: the same path, or the same + // registry for path-less items (review on #2024). Two path-less + // items otherwise (a registry pin and a git fork, say) are not. let own = path.map(same_path_key); + let own_reg = registry.map(str::trim); + let same_wiring = |p: &Option, r: &Option| match (&own, own_reg) { + (Some(_), _) => *p == own, + (None, Some(reg)) => p.is_none() && r.as_deref() == Some(reg), + (None, None) => false, + }; self.config_keys .iter() - .find(|(_, src, k, p)| { - k == key && same_patch_source(src, table) && (own.is_none() || *p != own) + .find(|(_, src, k, p, r)| { + k == key + && same_patch_source(self.resolved(src), self.resolved(table)) + && !same_wiring(p, r) }) - .map(|(file, _, k, _)| { + .map(|(file, _, k, _, _)| { format!("{file} redefines `[patch]` key `{k}`, which replaces it in cargo") }) } @@ -1533,12 +1590,6 @@ struct Bypass<'a> { wired: &'a [WiredCopy], } -/// Whether a `Cargo.lock` `source` is crates.io (its git or sparse index). -fn is_crates_io_lock_source(source: &str) -> bool { - is_crates_io_url_alias(source.trim().trim_start_matches("registry+")) - || index_key(source).as_deref() == Some("sparse+https://index.crates.io") -} - /// Mark the vendored ref `purl` (the copy `vref` of `name`@`version`) /// [`UnattestedKind::CargoPatchBypassed`] when the build may also compile /// a copy of that version its wiring does not reach. A `[patch.
]` @@ -1584,40 +1635,54 @@ fn patch_bypassed( .filter(|p| p.name == name && cargo_tag::strip_tag(&p.version) != version) .map(|p| Some(cargo_tag::strip_tag(&p.version).to_string())), ); - let declared: Vec<&Decl> = bypass - .decls - .and_then(|d| d.get(name)) - .into_iter() - .flatten() - .filter(|d| d.source != DeclSource::Path) - .filter(|d| matches!(route(d, &pins, version, &others), Route::Unpinned(_))) - .collect(); + let mut declared: Vec<&Decl> = Vec::new(); + let mut pinned_elsewhere: Vec<(&Decl, String)> = Vec::new(); + for d in bypass.decls.and_then(|d| d.get(name)).into_iter().flatten() { + match route(d, &pins, version, &others) { + Route::Unpinned(_) if d.source != DeclSource::Path => declared.push(d), + Route::Pin(uuid) if uuid != vref.uuid => pinned_elsewhere.push((d, uuid)), + _ => {} + } + } // A declaration pinned to ANOTHER Socket patch builds that patch's // copy, not this one. Only a ref for that patch at this very package, // emitted by the hosted pass (from the lock, or a conflict pair beside // a lock pin) and not itself withheld, puts the pair before the CLI's // conflict gate; otherwise (a stale lock, a lock line whose config // index disagrees, another locked version) nothing else says so. + // "Withheld" here is exactly the CLI's `bypassed_ref`: a file-scoped + // mark on the ref's own file (review on #2024); any other mark leaves + // the ref in the conflict gate. let gated = |uuid: &str| { out.refs.iter().any(|r| { r.uuid == uuid && r.purl == purl - && !out - .unattested - .iter() - .any(|u| u.uuid == uuid && u.purl == purl && u.file == r.source_file) + && !out.unattested.iter().any(|u| { + u.kind.file_scoped() + && u.uuid == uuid + && u.purl == purl + && u.file == r.source_file + }) }) }; - let other_patch: Vec<(&Decl, String)> = bypass - .decls - .and_then(|d| d.get(name)) + let other_patch: Vec<(&Decl, String)> = pinned_elsewhere .into_iter() - .flatten() - .filter_map(|d| match route(d, &pins, version, &others) { - Route::Pin(uuid) if uuid != vref.uuid && !gated(&uuid) => Some((d, uuid)), - _ => None, - }) + .filter(|(_, uuid)| !gated(uuid)) .collect(); + // Named in every detail, so one `vex` run shows each cause (review on + // #2024). + let also_other = if other_patch.is_empty() { + String::new() + } else { + format!( + ". Also: {}, which no wiring of that patch pairs with this copy", + other_patch + .iter() + .map(|(d, uuid)| format!("{} (Socket patch {uuid})", d.describe(name))) + .collect::>() + .join(", and ") + ) + }; let locked = same_version_foreign_entry(ctx, bypass.lock, name, version); let redirects = format!( "which no [patch] entry pointing at {} redirects", @@ -1640,7 +1705,8 @@ fn patch_bypassed( package, so the crate declaring it builds an unpatched {name} that the \ requirement allows to be {version}{evidence}. Re-running `vendor` does \ not change this: declare {name} from crates.io there (or point a \ - [patch] table for its source at the copy), or roll the patch back" + [patch] table for its source at the copy), or roll the patch \ + back{also_other}" ), ) } @@ -1648,8 +1714,16 @@ fn patch_bypassed( // A crates.io entry beside a copy `[patch.crates-io]` wires is // stale (cargo would redirect it); any other is declared by // some dependency, or left over from one that was removed. - let stale = is_crates_io_lock_source(source) - && pins.iter().any(|(table, _)| is_crates_io_source(table)); + // Likewise a git entry a `[patch.""]` table + // pointing at the copy redirects (review on #2024). + let stale = (crate::patch::redirect::is_crates_io_source(source) + && pins.iter().any(|(table, _)| is_crates_io_source(table))) + || source.trim().strip_prefix("git+").is_some_and(|repo| { + let repo = repo.split(['?', '#']).next().unwrap_or(repo); + pins.iter().any(|(table, _)| { + git_key(table).is_some() && git_key(table) == git_key(repo) + }) + }); let remedy = if stale { format!( "the entry is stale (`[patch.crates-io]` redirects crates.io), so \ @@ -1666,7 +1740,8 @@ fn patch_bypassed( CARGO_LOCK, format!( "{CARGO_LOCK} also locks {name} {version} from {source}, {redirects}: \ - cargo builds that copy unpatched beside the vendored one; {remedy}" + cargo builds that copy unpatched beside the vendored one; \ + {remedy}{also_other}" ), ) } @@ -1723,7 +1798,7 @@ async fn vendored_from_patches( continue; // a user path, or the uuid-less `.socket/cargo-patches/` } let name = entry.name; - if let Some(why) = shadow.and_then(|s| s.why(entry.source, entry.key, entry.path)) { + if let Some(why) = shadow.and_then(|s| s.why(entry.source, entry.key, entry.path, None)) { out.diag( DIAG_REF_INVALID, file, @@ -4474,6 +4549,29 @@ mod tests { ); } + /// Review on #2024 (round 3): a pin to another patch beside a lock that + /// also resolves the crate from crates.io is not described as "only + /// from other Socket patches". + #[tokio::test] + async fn another_patch_pin_beside_a_foreign_twin_is_not_called_socket_only() { + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "1", UUID_B), + &[("1.0.1", CRATES_IO)], + ); + let out = run(&p).await; + let diag = out + .diagnostics + .iter() + .find(|d| d.code == DIAG_REF_UNATTRIBUTABLE && d.detail.contains(UUID_B)) + .unwrap_or_else(|| panic!("{:#?}", out.diagnostics)); + assert!( + !diag.detail.contains("only from other Socket patches"), + "{}", + diag.detail + ); + } + /// REVIEW (Bugbot on #2024): a config item for the same key replaces /// the manifest's `[patch]` item in cargo whatever the source table and /// whether or not either wires a path: a path-less config git fork @@ -4493,14 +4591,33 @@ mod tests { let cfg = (".cargo/config.toml", config); let s = super::Shadow::new(&manifest, Some(&cfg)); // A path-less registry pin under the same key: shadowed. - assert!(s.why("crates-io", "cfg-if-socket-aaaaaaaa", None).is_some()); + assert!(s + .why( + "crates-io", + "cfg-if-socket-aaaaaaaa", + None, + Some("socket-patch-aaaaaaaa") + ) + .is_some()); // The same key under another source: not. - assert!(s.why("other", "cfg-if-socket-aaaaaaaa", None).is_none()); + assert!(s + .why( + "other", + "cfg-if-socket-aaaaaaaa", + None, + Some("socket-patch-aaaaaaaa") + ) + .is_none()); // A named non-crates.io table: shadowed. - assert!(s.why("my-corp", "cfg-if", Some("vendor/x")).is_some()); + assert!(s.why("my-corp", "cfg-if", Some("vendor/x"), None).is_some()); // A URL table, compared as an index. assert!(s - .why("https://my-corp.example/index", "cfg-if", Some("vendor/x")) + .why( + "https://my-corp.example/index", + "cfg-if", + Some("vendor/x"), + None + ) .is_some()); // A canonical-equivalent git table (Bugbot on a7611ef). let config: toml_edit::DocumentMut = @@ -4510,18 +4627,60 @@ mod tests { let cfg2 = (".cargo/config.toml", config); let s2 = super::Shadow::new(&manifest, Some(&cfg2)); assert!(s2 - .why("https://github.com/corp/fork", "cfg-if", Some("vendor/x")) + .why( + "https://github.com/corp/fork", + "cfg-if", + Some("vendor/x"), + None + ) .is_some()); assert!(s2 .why( "sparse+https://github.com/corp/fork", "cfg-if", - Some("vendor/x") + Some("vendor/x"), + None ) .is_none()); // The half-migrated shape: the same path is the same wiring. assert!(s - .why("https://my-corp.example/index", "cfg-if", Some("../fork")) + .why( + "https://my-corp.example/index", + "cfg-if", + Some("../fork"), + None + ) + .is_none()); + // Review on #2024: the identical path-less Socket registry pin in + // both files is the same wiring, not a shadow. + let config: toml_edit::DocumentMut = + "[patch.crates-io]\nserde = { registry = \"socket-patch-aaaaaaaa\" }\n" + .parse() + .unwrap(); + let cfg3 = (".cargo/config.toml", config); + let s3 = super::Shadow::new(&manifest, Some(&cfg3)); + assert!(s3 + .why("crates-io", "serde", None, Some("socket-patch-aaaaaaaa")) + .is_none()); + assert!(s3 + .why("crates-io", "serde", None, Some("socket-patch-bbbbbbbb")) + .is_some()); + // Review on #2024: a `[patch.""]` config table patches the + // same source as the manifest's `[patch.]` for a registry + // defined on that index. + let config: toml_edit::DocumentMut = "[registries.my-corp]\n\ + index = \"https://my-corp.example/index\"\n\ + [patch.\"https://my-corp.example/index\"]\n\ + cfg-if = { git = \"https://example.com/fork\" }\n" + .parse() + .unwrap(); + let cfg4 = (".cargo/config.toml", config); + let s4 = super::Shadow::new(&manifest, Some(&cfg4)); + assert!(s4 + .why("my-corp", "cfg-if", Some("vendor/x"), None) + .is_some()); + assert!(s4 + .why("other-corp", "cfg-if", Some("vendor/x"), None) .is_none()); }