Fix cargo agent mode patching overridden crates (#506)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1491,7 +1491,7 @@ command's integration tests check its output.
 | Tag                       | Action(s)        | Context |
 |---------------------------|------------------|---------|
 | `already_patched`         | `skipped`        | apply: every file's hash already matches `afterHash`. |
-| `package_not_installed`   | `skipped`        | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). **Cargo is never lockfile-only (v5.0, #616)**: `cargo fetch` unpacks every `Cargo.lock` entry (target-gated ones included), so a locked crate missing from `$CARGO_HOME/registry/src` was simply not fetched yet (a cold CI cache, a pruned `registry/src`) and the next `cargo build` would compile it unpatched; it counts as an unresolved purl (an all-miss run exits 1, as in 4.x) and its detail, the human error and the human warning say to run `cargo fetch` first. |
+| `package_not_installed`   | `skipped`        | apply: manifest entry has no matching installed package. When the project's lockfiles resolve the purl (a platform-gated optional dependency, a devDependency under `--omit=dev`), the detail says it is lockfile-only and the entry never fails the run, even when no other patch matched; only an all-miss run with an unmatched purl that no lock resolves exits 1 (`partialFailure`). **Cargo is never lockfile-only (v5.0, #616)**: `cargo fetch` unpacks every `Cargo.lock` entry (target-gated ones included), so a locked crate missing from `$CARGO_HOME/registry/src` was simply not fetched yet (a cold CI cache, a pruned `registry/src`) and the next `cargo build` would compile it unpatched; it counts as an unresolved purl (an all-miss run exits 1, as in 4.x) and its detail, the human error and the human warning say to run `cargo fetch` first. A crate `Cargo.lock` takes from a path, git, `[patch]` or `[replace]` source is never looked up in the registry cache (#506): cargo does not build that copy, so the same detail also names that case. |
 | `apply_failed`            | `failed`         | apply: hash mismatch, write error, archive read error. |
 | `no_local_source` | `skipped`/`failed` | Agent patch application cannot obtain the required local or downloaded patch source. Vendored mode consumes complete server artifacts and no longer stages patch blobs. |
 | `offline_missing_sources` / `sources_download_failed` | apply run-level `warnings[]` | apply (additive): the patch sources were unavailable — `--offline` with no local source, or the download left a patch with no source — so nothing was attempted. The envelope keeps its pinned shape (`partialFailure`, empty `events[]`, zero summary, no top-level `error`); the warning is its machine-readable reason (the human path prints the staging `Error:` line on stderr instead, even under `--silent`). |

crates/socket-patch-cli/src/commands/apply.rs

@@ -3343,7 +3343,9 @@ fn lock_resolution_is_calm(purl: &str) -> bool {
 }
 
 /// The `package_not_installed` detail of an unmatched purl the lock does
-/// not calmly account for: Cargo's carries the `cargo fetch` remedy.
+/// not calmly account for: Cargo's carries the `cargo fetch` remedy, and
+/// says why a crate the lock takes from a path, git or `[patch]` source
+/// is never found (#506).
 fn not_installed_detail(purl: &str) -> &'static str {
     if Ecosystem::from_purl(purl) == Some(Ecosystem::Cargo) {
         CARGO_NOT_FETCHED_DETAIL
@@ -3355,7 +3357,8 @@ fn not_installed_detail(purl: &str) -> &'static str {
 /// [`not_installed_detail`] for a Cargo crate.
 const CARGO_NOT_FETCHED_DETAIL: &str =
     "No unpacked crate source matches this PURL; run `cargo fetch` first so the crate is in \
-     the Cargo registry cache, then re-run apply";
+     the Cargo registry cache, then re-run apply. A crate Cargo.lock takes from a path, git or \
+     [patch] source is never patched: cargo does not build its registry copy";
 
 /// The `unmatched` purls with no lock evidence (sorted input, sorted
 /// output): the ones that can still fail an all-miss run.
@@ -3431,7 +3434,8 @@ fn cargo_fetch_hint(unmatched: &[String]) -> Option<&'static str> {
         .any(|p| Ecosystem::from_purl(p) == Some(Ecosystem::Cargo))
         .then_some(
             "Cargo crates are patched in the registry cache: run `cargo fetch` first so every \
-             locked crate is unpacked there, then re-run apply.",
+             locked crate is unpacked there, then re-run apply. A crate Cargo.lock takes from a \
+             path, git or [patch] source is never patched there: cargo does not build that copy.",
         )
 }
 

crates/socket-patch-cli/src/ecosystem_dispatch.rs

@@ -621,15 +621,39 @@ pub async fn find_all_packages_for_purls(
     // first-wins via their own `merge_first_wins` wiring in
     // `dispatch_find`. An npm purl with no copy can still be reached
     // through a workspace member's alias link (#1494).
-    dispatch_find_with_npm_links(
+    let mut found = dispatch_find_with_npm_links(
         partitioned,
         options,
         silent,
         merge_variant_copies,
         None,
         NpmLinks::ForUnresolved,
     )
-    .await
+    .await;
+    retain_locked_cargo_copies(&mut found, options).await;
+    found
+}
+
+/// Drop the Cargo copies a local project's build never consumes: a
+/// registry-cache copy of a crate its `Cargo.lock` takes from a path, git
+/// or `[patch]` source instead (`CargoCrawler::locked_registry_crates`).
+/// The cache is shared machine-wide, so such a copy is often there, and
+/// patching or attesting it would vouch for code cargo does not build
+/// (#506). Only the commands that patch or attest call this: rollback and
+/// `scan --prune` still find the copy, since an earlier apply may have
+/// patched it (#1278).
+async fn retain_locked_cargo_copies(
+    copies: &mut HashMap<String, Vec<PathBuf>>,
+    options: &CrawlerOptions,
+) {
+    let is_cargo = |purl: &str| Ecosystem::from_purl(purl) == Some(Ecosystem::Cargo);
+    if !copies.keys().any(|purl| is_cargo(purl)) {
+        return;
+    }
+    let Some(locked) = CargoCrawler::new().locked_registry_crates(options).await else {
+        return;
+    };
+    copies.retain(|purl, _| !is_cargo(purl) || locked.contains_purl(purl));
 }
 
 /// Multi-copy variant of `find_packages_for_rollback` (qualified-aware
@@ -1075,6 +1099,9 @@ pub async fn find_manifest_package_copies_reusing(
         NpmLinks::ForUnresolved,
     )
     .await;
+    // Only the registry copies of crates the lock takes from a registry
+    // are ones the build consumes, as for `apply` (#506).
+    retain_locked_cargo_copies(&mut copies, &crawler_options).await;
     // `apply` also writes every store variant of each npm copy (a pnpm peer
     // suffix, a Deno `_N` copy index, a vlt peer extra), so "every copy"
     // includes them (#603).

crates/socket-patch-cli/tests/apply/cargo_lock_source.rs

@@ -0,0 +1,144 @@
+//! `apply` in a Cargo project whose `Cargo.lock` takes a crate from a path,
+//! git or `[patch]` source (#506).
+//!
+//! `$CARGO_HOME/registry/src` is shared machine-wide, so it can hold a
+//! crates.io copy of `cfg-if-1.0.4` that this project's build never reads:
+//! a `[patch.crates-io]` path override (in `Cargo.toml` or
+//! `.cargo/config.toml`) or a git dependency replaces it. Patching that
+//! copy in place reports success while cargo builds the user's unpatched
+//! source, so agent mode must leave it alone. A registry-sourced lock entry
+//! (the control) is still patched.
+
+use std::path::Path;
+
+use serde_json::{json, Value};
+
+use crate::cargo_lock_source_fixture::{
+    stage_registry_copy, write_project, Shape, AFTER, BEFORE, PURL,
+};
+use crate::common;
+
+use common::{git_sha256, parse_json_envelope, run_with_env};
+
+/// `.socket/manifest.json` with one agent-mode record for [`PURL`], and the
+/// after blob staged so apply runs offline.
+fn write_manifest(root: &Path) {
+    let socket = root.join(".socket");
+    std::fs::create_dir_all(socket.join("blobs")).unwrap();
+    std::fs::write(socket.join("blobs").join(git_sha256(AFTER)), AFTER).unwrap();
+    let manifest = json!({ "patches": { PURL: {
+        "uuid": "50650650-0000-4000-8000-000000000506",
+        "exportedAt": "2024-01-01T00:00:00Z",
+        "files": { "package/src/lib.rs": {
+            "beforeHash": git_sha256(BEFORE),
+            "afterHash": git_sha256(AFTER),
+        }},
+        "vulnerabilities": {},
+        "description": "cargo lock source fixture",
+        "license": "MIT",
+        "tier": "free"
+    }}});
+    std::fs::write(
+        socket.join("manifest.json"),
+        serde_json::to_vec_pretty(&manifest).unwrap(),
+    )
+    .unwrap();
+}
+
+/// Stage `shape`, run `apply --offline --json`, and return the exit code,
+/// the envelope and the registry copy's `src/lib.rs` afterwards.
+fn apply(shape: Shape, extra: &[&str]) -> (i32, Value, Vec<u8>) {
+    let tmp = tempfile::tempdir().unwrap();
+    let project = tmp.path().join("app");
+    std::fs::create_dir_all(&project).unwrap();
+    write_project(&project, shape);
+    write_manifest(&project);
+    let cargo_home = tmp.path().join("cargo-home");
+    let lib = stage_registry_copy(&cargo_home, BEFORE);
+
+    let mut argv = vec!["apply", "--offline", "--json"];
+    argv.extend_from_slice(extra);
+    let (code, stdout, stderr) = run_with_env(
+        &project,
+        &argv,
+        &[
+            ("SOCKET_TELEMETRY_DISABLED", "1"),
+            ("CARGO_HOME", cargo_home.to_str().unwrap()),
+        ],
+    );
+    let v = parse_json_envelope(&stdout);
+    assert!(!v.is_null(), "{shape:?}: no envelope\n{stdout}\n{stderr}");
+    (code, v, std::fs::read(&lib).unwrap())
+}
+
+/// Control: a crate the lock takes from crates.io is patched in the
+/// registry cache, so the fixture really reaches the agent apply path.
+#[test]
+fn registry_sourced_crate_is_patched_in_the_registry_cache() {
+    let (code, v, lib) = apply(Shape::Registry, &[]);
+    assert_eq!(code, 0, "{v}");
+    assert_eq!(lib, AFTER, "the registry copy is the build's: {v}");
+}
+
+/// #506: a crate the lock resolves to a path, git or `[patch]` source is
+/// never built from the registry cache, so apply must not patch that copy
+/// or report it applied.
+#[test]
+fn crate_overridden_away_from_the_registry_is_not_patched() {
+    for shape in [
+        Shape::ManifestPatch,
+        Shape::ConfigPatch,
+        Shape::GitDependency,
+        Shape::PathDependency,
+        Shape::Replace,
+    ] {
+        let (code, v, lib) = apply(shape, &[]);
+        assert_eq!(
+            lib, BEFORE,
+            "{shape:?}: the unused registry copy must stay untouched: {v}"
+        );
+        let ev = event(&v, shape);
+        assert_eq!(ev["action"], "skipped", "{shape:?}: {v}");
+        match shape {
+            // The user's own `[patch]` or `[replace]` entry: the consent
+            // gate's calm skip.
+            Shape::ManifestPatch | Shape::ConfigPatch | Shape::Replace => {
+                assert_eq!(code, 0, "{shape:?}: {v}");
+                assert_eq!(ev["errorCode"], "user_patch_conflict", "{shape:?}: {ev}");
+            }
+            // Nothing else protects a git or path dependency: the crate is not
+            // installed where agent mode patches, and the run says why.
+            _ => {
+                assert_eq!(code, 1, "{shape:?}: must not report success: {v}");
+                assert_eq!(ev["errorCode"], "package_not_installed", "{shape:?}: {ev}");
+                assert!(
+                    ev.to_string().contains("path, git or [patch] source"),
+                    "{shape:?}: the detail names the override case: {ev}"
+                );
+            }
+        }
+    }
+}
+
+/// #506: `--allow-patch-conflicts` lets apply try a crate the user's own
+/// `[patch]` entry replaces, but the registry copy is still not the one
+/// cargo builds, so it stays untouched and the run does not succeed.
+#[test]
+fn allowed_patch_conflict_still_leaves_the_unused_registry_copy() {
+    for shape in [Shape::ManifestPatch, Shape::ConfigPatch, Shape::Replace] {
+        let (code, v, lib) = apply(shape, &["--allow-patch-conflicts"]);
+        assert_eq!(lib, BEFORE, "{shape:?}: {v}");
+        assert_eq!(code, 1, "{shape:?}: must not report success: {v}");
+        let ev = event(&v, shape);
+        assert_eq!(ev["errorCode"], "package_not_installed", "{shape:?}: {ev}");
+    }
+}
+
+fn event(v: &Value, shape: Shape) -> &Value {
+    v["events"]
+        .as_array()
+        .expect("events array")
+        .iter()
+        .find(|e| e["purl"] == PURL)
+        .unwrap_or_else(|| panic!("{shape:?}: no event for {PURL}: {v}"))
+}

crates/socket-patch-cli/tests/apply/main.rs

@@ -4,6 +4,8 @@
 //!
 //! One test binary per command: each module was its own binary.
 
+#[path = "../common/cargo_lock_source.rs"]
+mod cargo_lock_source_fixture;
 #[path = "../common/mod.rs"]
 mod common;
 #[path = "../common/rollback_json.rs"]
@@ -20,6 +22,7 @@ mod apply_network;
 mod apply_render;
 mod apply_silent_staging_errors;
 mod bun_global_store;
+mod cargo_lock_source;
 mod check_verifies_installed_tree;
 mod cli_gem_variant_mismatch_policy;
 mod e2e_safety_advisories;

crates/socket-patch-cli/tests/common/cargo_lock_source.rs

@@ -0,0 +1,115 @@
+//! A Cargo project whose `Cargo.lock` takes `cfg-if 1.0.4` from crates.io,
+//! a `[patch.crates-io]` path override or a git dependency, beside a
+//! crates.io copy of the crate in `$CARGO_HOME/registry/src` (#506). Shared
+//! by the `apply` and `vex` suites; pull it in with
+//! `#[path = "common/cargo_lock_source.rs"] mod cargo_lock_source;`.
+
+#![allow(dead_code)]
+
+use std::path::{Path, PathBuf};
+
+pub const PURL: &str = "pkg:cargo/cfg-if@1.0.4";
+pub const BEFORE: &[u8] = b"pub fn upstream() -> u32 { 1 }\n";
+pub const AFTER: &[u8] = b"pub fn upstream() -> u32 { 1 }\npub fn socket_patched() {}\n";
+pub const CRATES_IO: &str = "registry+https://github.com/rust-lang/crates.io-index";
+pub const GIT_SOURCE: &str = "git+file:///srv/git/cfg-if#0123456789abcdef0123456789abcdef01234567";
+
+/// Where cargo builds `cfg-if` from in the project.
+#[derive(Clone, Copy, Debug)]
+pub enum Shape {
+    /// `cfg-if = "=1.0.4"` from crates.io: the registry copy is the build's.
+    Registry,
+    /// `[patch.crates-io] cfg-if = { path = "local/cfg-if" }` in `Cargo.toml`.
+    ManifestPatch,
+    /// The same `[patch.crates-io]` in `.cargo/config.toml`.
+    ConfigPatch,
+    /// `cfg-if = { git = "file:///…" }`, no `[patch]` at all.
+    GitDependency,
+    /// `cfg-if = { path = "local/cfg-if" }`, no `[patch]` at all.
+    PathDependency,
+    /// `[replace] "cfg-if:1.0.4" = { git = "file:///…" }`: the lock keeps
+    /// the crates.io entry and names the git copy in its `replace` key.
+    Replace,
+}
+
+/// A crates.io copy of `cfg-if-1.0.4` in `<cargo_home>/registry/src`,
+/// holding `lib`. Returns the copy's `src/lib.rs`.
+pub fn stage_registry_copy(cargo_home: &Path, lib: &[u8]) -> PathBuf {
+    let dir = cargo_home.join("registry/src/index.crates.io-1949cf8c6b5b557f/cfg-if-1.0.4");
+    std::fs::create_dir_all(dir.join("src")).unwrap();
+    std::fs::write(
+        dir.join("Cargo.toml"),
+        "[package]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nedition = \"2018\"\n",
+    )
+    .unwrap();
+    std::fs::write(dir.join("src/lib.rs"), lib).unwrap();
+    dir.join("src/lib.rs")
+}
+
+/// A binary crate depending on `cfg-if 1.0.4` the way `shape` says, with
+/// the `Cargo.lock` cargo writes for it.
+pub fn write_project(root: &Path, shape: Shape) {
+    let dep = match shape {
+        Shape::GitDependency => "cfg-if = { git = \"file:///srv/git/cfg-if\" }",
+        Shape::PathDependency => "cfg-if = { path = \"local/cfg-if\" }",
+        _ => "cfg-if = \"=1.0.4\"",
+    };
+    let mut manifest = format!(
+        "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \"2021\"\n\n\
+         [dependencies]\n{dep}\n"
+    );
+    let patch = "[patch.crates-io]\ncfg-if = { path = \"local/cfg-if\" }\n";
+    match shape {
+        Shape::ManifestPatch => manifest.push_str(&format!("\n{patch}")),
+        Shape::ConfigPatch => {
+            std::fs::create_dir_all(root.join(".cargo")).unwrap();
+            std::fs::write(root.join(".cargo/config.toml"), patch).unwrap();
+        }
+        Shape::Replace => manifest
+            .push_str("\n[replace]\n\"cfg-if:1.0.4\" = { git = \"file:///srv/git/cfg-if\" }\n"),
+        Shape::Registry | Shape::GitDependency | Shape::PathDependency => {}
+    }
+    if matches!(
+        shape,
+        Shape::ManifestPatch | Shape::ConfigPatch | Shape::PathDependency
+    ) {
+        let fork = root.join("local/cfg-if");
+        std::fs::create_dir_all(fork.join("src")).unwrap();
+        std::fs::write(
+            fork.join("Cargo.toml"),
+            "[package]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nedition = \"2018\"\n",
+        )
+        .unwrap();
+        std::fs::write(fork.join("src/lib.rs"), "pub fn user_fork() -> u32 { 7 }\n").unwrap();
+    }
+    std::fs::write(root.join("Cargo.toml"), manifest).unwrap();
+
+    let source = match shape {
+        Shape::Registry => format!(
+            "source = \"{CRATES_IO}\"\nchecksum = \
+             \"9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801\"\n"
+        ),
+        Shape::ManifestPatch | Shape::ConfigPatch | Shape::PathDependency => String::new(),
+        Shape::GitDependency => format!("source = \"{GIT_SOURCE}\"\n"),
+        // What cargo writes for a `[replace]`d crate: the original entry
+        // keeps its registry `source` and names the replacement, which is
+        // locked as a package of its own.
+        Shape::Replace => format!(
+            "source = \"{CRATES_IO}\"\nreplace = \"cfg-if 1.0.4 ({GIT_SOURCE})\"\n\n\
+             [[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\nsource = \"{GIT_SOURCE}\"\n"
+        ),
+    };
+    std::fs::write(
+        root.join("Cargo.lock"),
+        format!(
+            "# This file is automatically @generated by Cargo.\n\
+             # It is not intended for manual editing.\n\
+             version = 4\n\n\
+             [[package]]\nname = \"app\"\nversion = \"0.1.0\"\ndependencies = [\n \"cfg-if\",\n]\n\n\
+             [[package]]\nname = \"cfg-if\"\nversion = \"1.0.4\"\n{source}"
+        ),
+    )
+    .unwrap();
+    std::fs::create_dir_all(root.join("src")).unwrap();
+    std::fs::write(root.join("src/main.rs"), "fn main() {}\n").unwrap();
+}