From 684c5fb8d1e29ceaedb57bd320d199d0fc90e02a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 12:15:53 -0600 Subject: [PATCH] Withhold Cargo pins a member's other source or other patch bypasses Two gaps left after the mixed-member-pin fix: Vendored twin from another source. `vendor` wires a crate through the root's `[patch.crates-io]`, which cargo applies only to crates.io declarations. A workspace member (or the root) that declares the crate from another registry or from git, at a requirement the patched version satisfies, resolves its own unpatched copy, and the vendored ref stayed attested beside it because the copy claim only reads sourceless lock 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. A declaration whose source a `[patch.]` table pointing at the same copy names (e.g. `[patch.my-corp]`) is covered, and a path declaration (the user's own fork) keeps the existing "ignored" rule. The CLI omits the ref as `vex_cargo_patch_bypassed`; documented in CLI_CONTRACT.md. Split patches. When the root pins a crate to Socket patch A and a member pins it to patch B, only the Cargo.lock ref for A was emitted, so the CLI's wiring-conflict gate never saw B. Discovery now also emits a Cargo.toml ref for B (attributed to the member) when B's requirement accepts the locked version and the lock resolves the crate from no B entry, so the package is gated as `wiring_conflict`. A same-version B lock entry is already its own ref; B at another locked version is a separate, patched package and does not conflict. `DepEntry` / `Decl` now carry a `DepSource` (crates.io, registry, git, path) in place of the message-only origin string. 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 | 607 ++++++++++++++++-- .../socket-patch-core/src/vex/discover/mod.rs | 7 + 4 files changed, 630 insertions(+), 46 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 8a612fb66..30059bc6e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -547,7 +547,7 @@ Recognition rules that hold for every ecosystem: * **Patch hosts.** A hosted reference counts only on `https://patch.socket.dev` or the `--patch-server-url` / `SOCKET_PATCH_SERVER_URL` origin, with no userinfo. The uuid is the URL's LAST canonical-uuid path segment, because grant tokens may themselves be uuid-shaped. The Go module prefix is fixed. `socket-patch-` registry / repository / source names count only through a pin. For a URL on any other host, see **Patch hosts** above. * **Pins, not definitions.** A registry, index or source *definition* alone (cargo `[registries]`, nuget ``, pom ``, uv index tables, `.npmrc`) never makes a reference, because it survives a reverted pin. Sections the package manager ignores are not read: npm's v2 `dependencies` mirror, a `.cargo/config.toml` shadowed by `.cargo/config`. A Socket pin inside a maven `` is diagnosed, never a reference. * **Contested locks.** When one lock wires a package to a patch and another lock resolves the same `name@version` from a non-Socket source, the build's bytes depend on which package manager runs. The reference is then dropped with a `patched_ref_unattributable` diagnostic naming both files. This applies across npm / pnpm / yarn / bun and across uv / pylock / poetry / pdm / Pipfile.lock / requirements. PEP 723 script locks neither contest nor are contested. The root `requirements.txt` and its in-root `-r` includes count as one lock here: pip reads them as one requirement set, where a direct reference wins over a compatible `==` pin of the same version in another file of the set (#1086). A PyPI version matches under PEP 440 equality, so a requirements line pinned in any spelling that can only install the wired release (`==1.2.3.0`, `==v1.2.3`, `===1.2.3`) contests it like `==1.2.3` does; a range or wildcard (`~=1.2.3`, `==1.2.*`) does not (#1602). A **bundled** npm copy (`inBundle: true`, or v1 `bundled: true`, beneath a dependency that bundles it; the root project's own bundles install from the lock, #1618) of the same `name@version` contests the reference too, in the same lock, in the other npm lock of a shrinkwrap/package-lock pair, or in any other lock. npm unpacks it from the parent package's tarball, so no rewire reaches it and it stays unpatched. Bun and vlt unpack bundled copies the same way (#469, #471). pnpm unpacks bundled copies too, but its lock cannot tie one to a reference (see **Unattested references** below). For Bun, that is a `bun.lock` entry whose meta is `{ "bundled": true }`, or a `bun.lockb` record that a dependency edge with the `bundled` behavior bit reaches, including one Bun shares with a regular install. Such an entry is never a reference, and it contests the reference the same way. For vlt, the lock records no node for a bundled copy, so the copy is found in the installed store: a real package directory inside a store package's own `node_modules`. yarn classic's `yarn.lock` records nothing about bundled copies either (#758), so for a lock with a Socket wiring they are found in the installed `node_modules`: each package a parent's `bundleDependencies` / `bundledDependencies` names inside the parent's own `node_modules`, the dependencies of those beside them, and everything nested under them. Such a copy contests the reference in `yarn.lock` (the diagnostic names the parent's lock entry and the copy's directory); a lock-only `vex` with no `node_modules` can't see it. Hosted and vendored scans skip these copies with `redirect_bun_bundled_instance_skipped` / `redirect_vlt_bundled_instance_skipped` / `vendor_bundled_instance_skipped`, and a yarn classic hosted scan names one with `redirect_copy_unpatched`. When a bundled copy is the only instance, vendoring refuses with `vendor_lock_entry_not_rewritable`. A reference withheld only because such an unreachable copy installs beside it (a bundled npm / Bun / vlt copy, an npm copy beneath a `hasShrinkwrap` package (#753) or one npm installs from a git / url / `file:` spec, or a yarn classic git or `file:` directory block of the same `name@version`) is still the rewriter's own wiring of exactly one package version: it is never attested, but `rollback`, `remove`, `list` and the hosted → vendored takeover treat it as an ordinary hosted pin and restore its upstream registry entry (#828), rather than refusing it with `hosted_wiring_contested`. Another entry of the **same** npm, Bun or yarn lock that resolves the wired `name@version` from a non-Socket source (for example a workspace member added after the rewire, then `npm install` / `bun install` / `yarn install`; for yarn berry, a registry locator such as `left-pad@npm:1.3.0` beside the hosted `…::__archiveUrl=` one) contests the reference too (#588). The package manager installs both entries, and that copy stays unpatched. Re-running `scan` / `vendor` rewires every copy. -* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. Five cases besides Gradle's (`vex_gradle_lock_above_base`): **pnpm bundled copies** (`vex_pnpm_bundled_copy`): a `packages:` entry's `bundledDependencies:` names the copies pnpm unpacks from that package's own tarball, but pnpm never locks them, so the bundled version is not in the lock. A pnpm reference whose package name a `bundledDependencies` list in the same lock names is omitted whatever its version (a missed attestation when the bundled copy is another version, never a false one), and `bundledDependencies: true` (or a value that cannot be read) omits every reference of that lock. It reaches no other lock. **deno.lock** (`vex_deno_lock_copy`): `deno install` installs a `package.json` project's npm dependencies from `deno.lock` and never reads `package-lock.json` / `pnpm-lock.yaml` / `yarn.lock`, so an npm-family reference whose `name@version` the `deno.lock` npm section also locks is omitted (#406). Whether Deno or another package manager populates `node_modules` (`nodeModulesDir: "manual"` allows either) is not in the files, so this holds whatever `nodeModulesDir` says. **Maven reactor roots** (`vex_maven_reactor_root`): a hosted pin in a root `pom.xml` that declares `` / ``, or whose subprojects Maven 4 discovers (see the hosted Maven rewriter), is omitted, because a module's own literal `` overrides a root pin and module poms are not read (#261); hosted scan refuses such roots (`redirect_maven_multimodule_unsupported`), so this only covers pins an older release wrote. **Maven classifier copies** (`vex_maven_classifier_unpatched`): a `` dependency of a hosted-pinned GA is its own coordinate; the classifier-less pin does not retarget it and the grant serves only the main jar, so Maven fetches it from the next repository. When a live, non-profile classifier copy other than `sources` / `javadoc` (`tests`, a native or JDK-specific build) resolves any version other than the suffixed pin — its own literal ``, a `` entry with the same classifier, or no version in the root pom — the pin is omitted. `sources` / `javadoc` copies never run on a classpath and do not withhold the statement; hosted scan only warns about them (`redirect_maven_classifier_unsupported`) and skips the dep when an executable copy names the patched release. **Unpinned Cargo declarations** (`vex_cargo_unpinned_declaration`): cargo resolves each source on its own, so when one declaration of a crate pins it to `socket-patch-` but another — the root `Cargo.toml`'s or a workspace member's, read as hosted scan reads them — names no Socket registry (crates.io, another registry, a path or git) with a version requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. The `Cargo.lock` reference is omitted, naming the unpinned declaration's manifest, and the warning also names a same-version lock entry from another source when the lock already holds one. Hosted scan pins every declaration, so this is a partial revert or a hand edit; re-running `scan` pins a crates.io declaration again. A declaration whose requirement excludes the patched version (cargo resolves it to another version) does not withhold the statement. +* **Unattested references.** Some evidence shows a build may run a copy no wiring reaches, but cannot be tied to the reference's exact `name@version` or cannot say the build runs it. The reference then stays a reference: `list`, `rollback` and `remove` find it, and the ledgers' liveness gates (`vendor --check`, `scan`) keep treating the wiring as live, because re-running `scan` / `vendor` could never clear the evidence. Only `vex` omits it, as a run warning and as the `failed[].reason`. 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 — names no Socket registry (crates.io, another registry, a path or git) with a version requirement the locked patched version satisfies, that declaration builds a separate, unpatched copy, possibly of the very version the pin covers. The `Cargo.lock` reference is omitted, naming the unpinned declaration's manifest, and the warning also names a same-version lock entry from another source when the lock already holds one. Hosted scan pins every declaration, so this is a partial revert or a hand edit; re-running `scan` pins a crates.io declaration again. A declaration whose requirement excludes the patched version (cargo resolves it to another version) does not withhold the statement. **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 declaration of the crate (the root `Cargo.toml`'s or a workspace member's) names another registry or a git repository with a version requirement the patched version satisfies, and no `[patch]` table for that source points at the same copy; cargo builds that declaration's own, unpatched copy. 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. When the root pins a crate to one Socket patch and a workspace member pins it to another that `Cargo.lock` does not resolve, both wirings are reported, so `vex` omits the package as `wiring_conflict`. * **Shrinkwrap without a twin (#899).** npm >= 12 never reads `npm-shrinkwrap.json`: on a project whose only npm lock is the shrinkwrap it resolves the tree from the registry and writes a fresh `package-lock.json`, so a Socket reference there reaches npm <= 11 only. The reference is still discovered, so `list`, `rollback` and `remove` manage it, but `vex` omits the patch (`vex_npm_shrinkwrap_only`, as a run warning and as the `failed[].reason`). A `package-lock.json` beside it (wired the same way) makes it attestable again. Hosted and vendored runs still rewire the shrinkwrap and warn `redirect_npm_shrinkwrap_only` / `vendor_npm_shrinkwrap_only`. * **Lockless pins.** With no lock to name a version, a `Cargo.toml` pin (every declaration on `socket-patch-`, that registry defined on the patch host for the same uuid) or an exclusive nuget exact-id mapping is never a reference on its own, so v5.0 does not attest it (nor does `list` show it, or `rollback` / `remove` restore it — restore those files from version control). Only a pre-v5 redirect-ledger record naming a version the pin admits keeps it live. The same holds for a gem wired only in the `Gemfile` (the pre-bundler-2.6 mixed state, lock not converged). @@ -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) names no Socket registry and accepts the patched version, so cargo builds a separate unpatched copy for it; no statement while it does. `list`, `rollback` and `remove` still find the pin. | +| `vex_cargo_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 431dbd27d..34a32d7a1 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. @@ -278,6 +282,12 @@ fn unattested_note(kind: UnattestedKind) -> (&'static str, &'static str) { `socket-patch scan` to pin a crates.io declaration too; one from another \ registry, a path or git cannot be pinned, so roll the patch back)", ), + 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)", + ), } } @@ -1818,6 +1828,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 afcd7f444..5054855d3 100644 --- a/crates/socket-patch-core/src/vex/discover/cargo.rs +++ b/crates/socket-patch-core/src/vex/discover/cargo.rs @@ -48,7 +48,11 @@ //! entry) — so the ref is marked //! [`super::UnattestedKind::CargoUnpinnedDeclaration`], naming that //! declaration's file (the hosted rewriter pins every declaration, so this -//! is a partial revert or a hand edit); +//! is a partial revert or a hand edit); and when another declaration pins +//! a DIFFERENT `socket-patch-` whose requirement accepts the locked +//! version, and the lock resolves the crate from no `` entry, a +//! `Cargo.toml` ref for `` too (the root and a member split across +//! patches: the CLI gates the package as `wiring_conflict`); //! * declarations pin only OTHER `socket-patch-` registries → the lock //! ref AND a `Cargo.toml` ref for `` at the locked version (no pin): //! the files disagree and the CLI gates the package as `wiring_conflict`; @@ -118,6 +122,13 @@ //! 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 declaration (root or member) from another +//! registry or git that accepts the version and whose source no `[patch]` +//! table pointing at this copy names, or a same-version lock entry +//! from any source — stays a ref but is marked +//! [`super::UnattestedKind::CargoPatchBypassed`] ([`patch_bypassed`]). //! 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 @@ -211,6 +222,16 @@ pub(crate) async fn extract(ctx: &DiscoverCtx<'_>, out: &mut Discovery) { if let Some(decls) = &decls { unresolved_manifest_pins(ctx, &lock, decls, &manifests, &definitions, out); } + let wired: Vec<(String, VendorRef)> = manifest + .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 @@ -245,25 +266,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 - .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() } @@ -413,10 +431,10 @@ struct Decl { /// The `version` requirement (`None`: none written, or a `[patch]` /// registry pin). version: Option, - /// Where cargo resolves a dependency declaration from, for messages - /// ([`DepEntry::origin`]); `None` for a `[patch]` entry's registry pin, - /// which redirects declarations rather than adding one. - origin: Option, + /// Where cargo resolves a dependency declaration from; `None` for a + /// `[patch]` entry's registry pin, which redirects declarations rather + /// than adding one. + source: Option, } impl Decl { @@ -426,7 +444,7 @@ impl Decl { file: file.to_string(), registry: registry.map(str::to_string), version: None, - origin: None, + source: None, } } @@ -434,11 +452,29 @@ impl Decl { /// every Socket patch registry: no `registry` key (crates.io, or a /// path / git source), or a registry not named `socket-patch-*`. fn unpinned(&self) -> bool { - self.origin.is_some() - && self - .registry - .as_deref() - .is_none_or(|r| !r.trim().starts_with(PIN_NAME_PREFIX)) + self.source.is_some() && !self.socket_pinned() + } + + /// Whether the `registry` key names a `socket-patch-*` registry. + fn socket_pinned(&self) -> bool { + self.registry + .as_deref() + .is_some_and(|r| r.trim().starts_with(PIN_NAME_PREFIX)) + } + + /// "`` 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:?}")); + let from = self + .source + .as_ref() + .map(DepSource::describe) + .unwrap_or_default(); + format!("{} declares {name} {req} from {from}", self.file) } /// Whether this declaration's version requirement may select @@ -467,7 +503,7 @@ fn manifest_declarations(manifests: &[(String, DocumentMut)]) -> Decls { for dep in dependency_entries(doc) { let decl = Decl { file: file.clone(), - origin: Some(dep.origin()), + source: Some(dep.source), registry: dep.registry, version: dep.version, }; @@ -494,18 +530,62 @@ struct DepEntry { registry: Option, /// The `version` requirement (a bare string entry is one). version: Option, - /// The entry's `git` / `path` key, when it has one. - local: Option<&'static str>, + source: DepSource, +} + +/// Where cargo resolves a dependency declaration from. +#[derive(Debug, Clone, PartialEq, Eq)] +enum DepSource { + /// No `registry` key, or `registry = "crates-io"`. + CratesIo, + /// `registry = ""` (a `socket-patch-*` pin included). + Registry(String), + /// `git = ""` (empty when the value is not a string). + Git(String), + /// `path = "…"`. + Path, } -impl DepEntry { - /// Where cargo resolves this declaration from, for messages. - fn origin(&self) -> String { - match (self.local, &self.registry) { - (Some("git"), _) => "a git repository".to_string(), - (Some(_), _) => "a local path".to_string(), - (None, Some(reg)) => format!("registry `{reg}`"), - (None, None) => "crates.io".to_string(), +impl DepSource { + /// The source of a declaration entry with these `registry` / `git` / + /// `path` values (`git` and `path` override `registry`, as in cargo's + /// local builds). + fn of(registry: Option<&str>, git: Option>, path: bool) -> Self { + match (git, path, registry) { + (Some(url), _, _) => DepSource::Git(url.unwrap_or_default()), + (None, true, _) => DepSource::Path, + (None, false, Some(reg)) if reg.trim() != "crates-io" => { + DepSource::Registry(reg.to_string()) + } + (None, false, _) => DepSource::CratesIo, + } + } + + /// For messages. + fn describe(&self) -> String { + match self { + DepSource::CratesIo => "crates.io".to_string(), + DepSource::Registry(reg) => format!("registry `{reg}`"), + DepSource::Git(_) => "a git repository".to_string(), + DepSource::Path => "a local path".to_string(), + } + } + + /// Whether a `[patch.]` table replaces the crate in + /// declarations from this source: cargo keys it by `crates-io`, a + /// registry name, or a source URL. A git URL matches ignoring a + /// trailing `/` or `.git` (only ever a spelling cargo treats as the + /// same repository; any other difference reads as not covered). + 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 { + DepSource::CratesIo => is_crates_io_source(table), + DepSource::Registry(reg) => table == reg.trim(), + DepSource::Git(url) => !url.is_empty() && git_key(url) == git_key(table), + DepSource::Path => false, } } } @@ -537,18 +617,24 @@ fn dependency_entries(doc: &DocumentMut) -> Vec { name: key.to_string(), registry: None, version: Some(version.to_string()), - local: None, + source: DepSource::CratesIo, } } else if let Some(entry) = item.as_table_like() { if entry.get("workspace").and_then(Item::as_bool) == Some(true) { continue; } let field = |k: &str| entry.get(k).and_then(Item::as_str).map(str::to_string); + let registry = field("registry"); + let source = DepSource::of( + registry.as_deref(), + entry.contains_key("git").then(|| field("git")), + entry.contains_key("path"), + ); DepEntry { name: field("package").unwrap_or_else(|| key.to_string()), - registry: field("registry"), + registry, version: field("version"), - local: ["git", "path"].into_iter().find(|k| entry.contains_key(k)), + source, } } else { continue; @@ -635,6 +721,30 @@ fn hosted_from_lock( { unpinned_beside_pin(ctx, lock, pkg, &purl, &uuid, regs, out); out.push(lock_ref); + // Another declaration may pin a DIFFERENT Socket patch (a root + // pin and a member pin split across patches): cargo resolves it + // from that patch's registry, maybe at this very version, so + // emit that wiring too and let the CLI's conflict gate refuse + // to pick one. Not when its requirement excludes this version, + // or the lock already resolves it from its own patch (that + // entry is its own ref). + for (decl, other) in other_patch_pins(regs, &uuid) { + let locked = lock.packages().iter().any(|p| { + p.name == pkg.name + && p.source.as_deref().and_then(|s| source_uuid(ctx, s)) + == Some(other.clone()) + }); + if decl.accepts(&pkg.version) && !locked { + out.push(PatchedRef::hosted( + purl.clone(), + other, + &decl.file, + None, + None, + true, + )); + } + } continue; } let others: Vec<(&str, String)> = regs @@ -680,6 +790,17 @@ fn hosted_from_lock( } } +/// The declarations among `regs` pinned to a Socket patch other than +/// `uuid`, with that patch's uuid. +fn other_patch_pins<'a>(regs: &'a [Decl], uuid: &str) -> Vec<(&'a Decl, String)> { + regs.iter() + .filter_map(|d| { + let other = socket_patch_name_uuid(d.registry.as_deref()?, false)?; + (other != uuid).then_some((d, other)) + }) + .collect() +} + /// Mark the Socket-locked `pkg` [`UnattestedKind::CargoUnpinnedDeclaration`] /// when, beside a declaration pinned to its patch, another declaration of /// the crate (the root's or a workspace member's) is not pinned to any @@ -709,14 +830,7 @@ fn unpinned_beside_pin( }; let declared = unpinned .iter() - .map(|d| { - let req = d - .version - .as_deref() - .map_or_else(|| "with no version".to_string(), |v| format!("{v:?}")); - let from = d.origin.as_deref().unwrap_or_default(); - format!("{} declares {} {req} from {from}", d.file, pkg.name) - }) + .map(|d| d.describe(&pkg.name)) .collect::>() .join(", and "); // Direct evidence: the lock already holds that unpinned copy at the @@ -939,6 +1053,115 @@ 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 dependency declaration (the root's or a workspace member's) from +/// another registry or from git, whose requirement accepts `version` and +/// whose source no `[patch]` table pointing at this copy names: 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-*` +/// declaration 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 covered = |source: &DepSource| { + bypass.wired.iter().any(|(table, v)| { + v.uuid == vref.uuid && v.leaf == vref.leaf && source.patched_by(table) + }) + }; + let declared: Vec<&Decl> = bypass + .decls + .and_then(|d| d.get(name)) + .into_iter() + .flatten() + .filter(|d| match &d.source { + Some(s @ (DepSource::Registry(_) | DepSource::Git(_))) => { + !d.socket_pinned() && !covered(s) + } + _ => false, + }) + .filter(|d| d.accepts(version)) + .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| d.describe(name)) + .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( @@ -947,6 +1170,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) { @@ -1059,6 +1283,7 @@ async fn vendored_from_patches( continue; } } + patch_bypassed(ctx, bypass, &vref, name, version, &purl, out); out.push(PatchedRef::vendored(purl, &vref, file, None)); } } @@ -1892,6 +2117,102 @@ mod tests { assert!(!u.detail.contains("already locks"), "{}", u.detail); } + /// REGRESSION: the root pins serde to Socket patch A and a member pins + /// it to patch B, but the lock holds only A's entry. Some declaration + /// pinning A used to settle it, so B's pin went unread and A was + /// attested though the member resolves B. Both wirings are emitted now + /// (the member's attributed to its manifest), so the CLI gates the + /// package as a `wiring_conflict`, whichever side pins which patch. + #[tokio::test] + async fn split_patches_across_root_and_member_emit_both() { + for (root, member, file) in [ + (UUID_A, UUID_B, "crates/a/Cargo.toml"), + (UUID_B, UUID_A, "Cargo.toml"), + ] { + let p = mixed_workspace( + &pinned("serde", "1", root), + &pinned("serde", "1", member), + &[], + ); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted), + ("pkg:cargo/serde@1.0.1", UUID_B, WiringMode::Hosted), + ], + ); + let lock_ref = out.refs.iter().find(|r| r.uuid == UUID_A).unwrap(); + assert_eq!(lock_ref.source_file, std::path::PathBuf::from("Cargo.lock")); + let toml_ref = out.refs.iter().find(|r| r.uuid == UUID_B).unwrap(); + assert_eq!(toml_ref.source_file, std::path::PathBuf::from(file)); + assert!(!toml_ref.lockfile_basis_ok()); + assert!(out.unattested.is_empty(), "{:#?}", out.unattested); + } + } + + /// The other patch's pin does not contest the locked one when cargo + /// cannot resolve it to the locked version: its requirement excludes + /// that version, or the lock already resolves it from its own patch at + /// another version (each declaration builds its own patched copy). A + /// same-version entry for the other patch is its own lock ref, so the + /// conflict pair comes from the lock alone. + #[tokio::test] + async fn another_patch_pin_at_another_version_does_not_conflict() { + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "=1.0.2", UUID_B), + &[], + ); + let out = run(&p).await; + assert_refs( + &out, + &[("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted)], + ); + + let other = index(UUID_B); + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "1", UUID_B), + &[("1.0.2", other.as_str())], + ); + let out = run(&p).await; + let mut want = vec![ + ("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted), + ("pkg:cargo/serde@1.0.2", UUID_B, WiringMode::Hosted), + ]; + want.sort(); + assert_refs(&out, &want); + assert!( + out.refs + .iter() + .all(|r| r.source_file == std::path::Path::new("Cargo.lock")), + "{:#?}", + out.refs + ); + + let p = mixed_workspace( + &pinned("serde", "1", UUID_A), + &pinned("serde", "1", UUID_B), + &[("1.0.1", other.as_str())], + ); + let out = run(&p).await; + assert_refs( + &out, + &[ + ("pkg:cargo/serde@1.0.1", UUID_A, WiringMode::Hosted), + ("pkg:cargo/serde@1.0.1", UUID_B, WiringMode::Hosted), + ], + ); + assert!( + out.refs + .iter() + .all(|r| r.source_file == std::path::Path::new("Cargo.lock")), + "{:#?}", + out.refs + ); + } + /// A manifest under `.socket/` (a path dependency into Socket's own /// tree, as a vendored copy is) is not a declaration the hosted /// rewriter pins, so it never decides: the lock stays the only word. @@ -2788,4 +3109,200 @@ 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", + ), + ] { + 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 whose source a `[patch]` table for that source also + /// points at the same copy. 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\n\ + [patch.\"https://example.com/cfg-if\"]\ncfg-if = {{ path = \"{rel}\" }}\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 = { git = \"https://example.com/cfg-if.git\" }", + covered.as_str(), + ), + ("cfg-if = { path = \"../fork\" }", ""), + ] { + 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"); + } } diff --git a/crates/socket-patch-core/src/vex/discover/mod.rs b/crates/socket-patch-core/src/vex/discover/mod.rs index cd0a87698..2efe0eae4 100644 --- a/crates/socket-patch-core/src/vex/discover/mod.rs +++ b/crates/socket-patch-core/src/vex/discover/mod.rs @@ -612,6 +612,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,