Skip to content

Commit 73f4a18

Browse files
committed
Keep other registries' crates out of cargo apply
A pkg:cargo PURL names a crates.io crate, but a private registry or the hosted Socket registry can unpack a different crate with the same name and version into its own registry/src dir. Patch every crates.io copy only, and fall back to the single first-found copy as before when no crates.io copy exists. The regression test now also shows apply --check passing and vex attesting once every copy is patched, so its failure checks can't pass for an unrelated reason. Assisted-by: Claude Code:claude-opus-5-5
1 parent aafa230 commit 73f4a18

2 files changed

Lines changed: 134 additions & 38 deletions

File tree

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

Lines changed: 74 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,8 @@ pub fn partition_purls(
142142

143143
/// Standard scan-one-ecosystem pattern: discover source paths, run
144144
/// `find_by_purls` on each, and merge results into `$out` keyed by PURL
145-
/// (first wins). Used by every ecosystem except pypi (which dedups
146-
/// PURLs and, on rollback, remaps base PURLs back to qualified ones).
145+
/// with the ecosystem's `$on_match` merge (first-wins, or every copy for
146+
/// npm, cargo and the release-variant ecosystems).
147147
///
148148
/// `$using_label` is the noun in "Using <X> at: <path>" for global
149149
/// scans; pass `""` to suppress that line. The banner is progress chrome
@@ -209,7 +209,8 @@ macro_rules! scan_ecosystem {
209209
}};
210210
}
211211

212-
/// Signature shared by `merge_first_wins` and `merge_qualified`.
212+
/// Signature shared by the `dispatch_find` merges (`merge_first_wins`,
213+
/// `merge_cargo_copies`, `merge_variant_copies`, `merge_qualified`).
213214
/// `dispatch_find` swaps between them so the rollback path can fan one
214215
/// crawler result back out to every caller-supplied qualified PURL. The
215216
/// output map holds a `Vec` of paths per PURL: most ecosystems install one
@@ -237,9 +238,9 @@ fn push_path(out: &mut HashMap<String, Vec<PathBuf>>, purl: String, path: PathBu
237238
/// would re-apply to what is effectively the same package — a scope-expanding
238239
/// behavior change that is out of scope for the npm multi-copy fix and would,
239240
/// for a shared global cache, mutate state other projects rely on. Fanning out
240-
/// to genuinely-distinct installs is npm-only (`merge_npm_copies`); if a
241-
/// per-root fan-out is ever wanted for these ecosystems it must be an explicit,
242-
/// separately-tested decision.
241+
/// to genuinely-distinct installs is an explicit, separately-tested decision
242+
/// per ecosystem: npm (`merge_npm_copies`) and cargo's registry index dirs
243+
/// (`merge_cargo_copies`).
243244
fn merge_first_wins(
244245
out: &mut HashMap<String, Vec<PathBuf>>,
245246
_purls: &[String],
@@ -277,26 +278,51 @@ fn merge_variant_copies(
277278
}
278279
}
279280

280-
/// Cargo merge: keep EVERY distinct copy, one per source root. Each root
281-
/// of the shared registry cache is one registry index dir
281+
/// Cargo merge: keep EVERY crates.io copy, one per registry index dir.
282+
/// The shared registry cache extracts a crate once per index dir
282283
/// (`index.crates.io-6f17d22bba15001f` for cargo 1.70–1.84,
283284
/// `index.crates.io-1949cf8c6b5b557f` for cargo ≥ 1.85, the pre-1.70 git
284-
/// index), and each holds its own extracted copy that a different cargo
285-
/// builds from. First-wins patched, checked and restored only the copy in
286-
/// the first dir in listing order, so a project on the other cargo kept
287-
/// building the unpatched crate while `apply` reported success and `vex`
288-
/// attested it (#339). A `cargo vendor` tree or `--global-prefix` is a
289-
/// single root, so it still yields one copy.
285+
/// index), and each cargo builds from its own. First-wins patched, checked
286+
/// and restored only the copy in the first dir in listing order, so a
287+
/// project on the other cargo kept building the unpatched crate while
288+
/// `apply` reported success and `vex` attested it (#339).
289+
///
290+
/// A `pkg:cargo` PURL names a crates.io crate, so a same-`name@version`
291+
/// copy in another registry's dir (a private registry, a hosted Socket
292+
/// patch registry) is a different crate: it is kept only when no crates.io
293+
/// copy exists, as the single first-found copy it always was. A
294+
/// `cargo vendor` tree or `--global-prefix` is one root, so one copy.
290295
fn merge_cargo_copies(
291296
out: &mut HashMap<String, Vec<PathBuf>>,
292297
_purls: &[String],
293298
packages: HashMap<String, CrawledPackage>,
294299
) {
295300
for (purl, pkg) in packages {
296-
push_path(out, purl, pkg.path);
301+
let paths = out.entry(purl).or_default();
302+
if paths.is_empty() {
303+
paths.push(pkg.path);
304+
} else if is_crates_io_copy(&pkg.path) && !paths.contains(&pkg.path) {
305+
if paths.iter().all(|p| is_crates_io_copy(p)) {
306+
paths.push(pkg.path);
307+
} else {
308+
// The first-found copy belongs to another registry.
309+
*paths = vec![pkg.path];
310+
}
311+
}
297312
}
298313
}
299314

315+
/// Whether `crate_dir` sits in a crates.io index dir of the registry
316+
/// cache: `index.crates.io-<hash>` (sparse, cargo ≥ 1.70) or the git
317+
/// index's `github.com-1ecc6299db9ec823` (cargo < 1.70).
318+
fn is_crates_io_copy(crate_dir: &std::path::Path) -> bool {
319+
crate_dir
320+
.parent()
321+
.and_then(|p| p.file_name())
322+
.and_then(|n| n.to_str())
323+
.is_some_and(|n| n.starts_with("index.crates.io-") || n == "github.com-1ecc6299db9ec823")
324+
}
325+
300326
/// npm merge: the npm crawler returns EVERY physical copy of each PURL
301327
/// (nested duplicates, diamonds, `file:` dups), so fold every path in.
302328
/// A one-path `HashMap<String, PathBuf>` would silently drop every copy
@@ -392,8 +418,8 @@ fn passthrough_purls(purls: &[String]) -> Vec<String> {
392418
/// into one `purl -> path` map.
393419
///
394420
/// `variant_merge` lets the rollback variant fan a single crawler result
395-
/// out to every caller-supplied qualified PURL; everything else just
396-
/// inserts the crawler-returned PURL with first-wins semantics. It is
421+
/// out to every caller-supplied qualified PURL; every other ecosystem
422+
/// inserts the crawler-returned PURL with its own fixed merge. It is
397423
/// applied to the release-variant ecosystems (PyPI / RubyGems / Maven),
398424
/// which are also queried with deduped base PURLs.
399425
///
@@ -1194,7 +1220,7 @@ mod tests {
11941220
// keeps ONLY the first — matching the historical first-wins contract.
11951221
// Fanning out to every root re-patches what is effectively one install
11961222
// (the docker_e2e_nuget `already_patched` regression); genuine
1197-
// multi-copy fan-out is npm-only via `merge_npm_copies`.
1223+
// multi-copy fan-out is opt-in per ecosystem (npm, cargo).
11981224
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
11991225
merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/first")]));
12001226
merge_first_wins(&mut out, &[], packages(&[("pkg:cargo/foo@1.0", "/second")]));
@@ -1307,28 +1333,53 @@ mod tests {
13071333

13081334
// ---- merge_cargo_copies ------------------------------------------------
13091335

1336+
#[test]
1337+
fn merge_cargo_copies_prefers_crates_io_over_another_registry() {
1338+
// A same-name@version crate from a private registry is another
1339+
// crate: never patched beside crates.io copies, whichever order the
1340+
// index dirs list in, but still the copy when it is the only one.
1341+
let purl = "pkg:cargo/foo@1.0.0";
1342+
let corp = "/src/corp.example-0123456789abcdef/foo-1.0.0";
1343+
let new = "/src/index.crates.io-1949cf8c6b5b557f/foo-1.0.0";
1344+
let git = "/src/github.com-1ecc6299db9ec823/foo-1.0.0";
1345+
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
1346+
for path in [corp, new, corp, git] {
1347+
merge_cargo_copies(&mut out, &[], packages(&[(purl, path)]));
1348+
}
1349+
assert_eq!(
1350+
out.get(purl),
1351+
Some(&vec![PathBuf::from(new), PathBuf::from(git)])
1352+
);
1353+
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
1354+
merge_cargo_copies(&mut out, &[], packages(&[(purl, corp)]));
1355+
assert_eq!(out.get(purl), Some(&vec![PathBuf::from(corp)]));
1356+
}
1357+
13101358
#[test]
13111359
fn merge_cargo_copies_keeps_every_registry_index_copy() {
13121360
// #339: the macro calls on_match once per registry index dir; each
13131361
// dir's copy is built by a different cargo, so all are carried in
13141362
// discovery order, and a path re-observed is recorded once.
13151363
let mut out: HashMap<String, Vec<PathBuf>> = HashMap::new();
13161364
for path in [
1317-
"/src/index-1949cf/cfg-if-1.0.0",
1318-
"/src/index-6f17d2/cfg-if-1.0.0",
1365+
"/src/index.crates.io-1949cf8c6b5b557f/cfg-if-1.0.0",
1366+
"/src/index.crates.io-6f17d22bba15001f/cfg-if-1.0.0",
13191367
] {
13201368
merge_cargo_copies(&mut out, &[], packages(&[("pkg:cargo/cfg-if@1.0.0", path)]));
13211369
}
13221370
merge_cargo_copies(
13231371
&mut out,
13241372
&[],
1325-
packages(&[("pkg:cargo/cfg-if@1.0.0", "/src/index-6f17d2/cfg-if-1.0.0")]),
1373+
packages(&[(
1374+
"pkg:cargo/cfg-if@1.0.0",
1375+
"/src/index.crates.io-6f17d22bba15001f/cfg-if-1.0.0",
1376+
)]),
13261377
);
13271378
assert_eq!(
13281379
out.get("pkg:cargo/cfg-if@1.0.0"),
13291380
Some(&vec![
1330-
PathBuf::from("/src/index-1949cf/cfg-if-1.0.0"),
1331-
PathBuf::from("/src/index-6f17d2/cfg-if-1.0.0"),
1381+
PathBuf::from("/src/index.crates.io-1949cf8c6b5b557f/cfg-if-1.0.0"),
1382+
PathBuf::from("/src/index.crates.io-6f17d22bba15001f/cfg-if-1.0.0"),
13321383
])
13331384
);
13341385
}

‎crates/socket-patch-cli/tests/e2e_cargo.rs‎

Lines changed: 60 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -599,13 +599,32 @@ async fn agent_mode_covers_every_registry_index_copy() {
599599
orig,
600600
),
601601
];
602+
// A private registry's crate of the same name@version is another
603+
// crate: agent mode must leave it alone.
604+
let corp = fake_registry_crate(
605+
&src.join("corp.example-0123456789abcdef"),
606+
"cfg-if",
607+
"1.0.0",
608+
b"pub fn corp() {}\n",
609+
);
602610
let lib = |copy: &PathBuf| std::fs::read(copy.join("src/lib.rs")).unwrap();
603611

604612
let socket = root.join(".socket");
605613
let purl = "pkg:cargo/cfg-if@1.0.0";
606-
let manifest = serde_json::json!({ "patches": {
607-
purl: manifest_record(&socket, "33900000-0000-4000-8000-000000000001", orig, patched),
614+
// vex attests only patches that carry vulnerability metadata.
615+
let mut record = manifest_record(
616+
&socket,
617+
"33900000-0000-4000-8000-000000000001",
618+
orig,
619+
patched,
620+
);
621+
record["vulnerabilities"] = serde_json::json!({ "GHSA-xxxx-xxxx-xxxx": {
622+
"cves": ["CVE-2024-0001"],
623+
"summary": "s",
624+
"severity": "high",
625+
"description": "d",
608626
}});
627+
let manifest = serde_json::json!({ "patches": { purl: record } });
609628
std::fs::write(
610629
socket.join("manifest.json"),
611630
serde_json::to_string_pretty(&manifest).unwrap(),
@@ -633,6 +652,39 @@ async fn agent_mode_covers_every_registry_index_copy() {
633652
copy.display()
634653
);
635654
}
655+
assert_eq!(
656+
lib(&corp),
657+
b"pub fn corp() {}\n",
658+
"apply patched another registry's crate"
659+
);
660+
661+
// Baselines: with every copy patched the check passes and vex attests.
662+
let out = run(
663+
&["apply", "--check", "--json", "--offline", "--cwd", cwd],
664+
root,
665+
&proxy_url,
666+
)
667+
.await;
668+
let stdout = String::from_utf8_lossy(&out.stdout);
669+
assert!(
670+
out.status.success(),
671+
"apply --check must pass with every copy patched\nstdout:\n{stdout}"
672+
);
673+
let vex_args = [
674+
"vex",
675+
"--offline",
676+
"--product",
677+
"pkg:cargo/myapp@0.1.0",
678+
"--cwd",
679+
cwd,
680+
];
681+
let out = run(&vex_args, root, &proxy_url).await;
682+
let stdout = String::from_utf8_lossy(&out.stdout);
683+
let stderr = String::from_utf8_lossy(&out.stderr);
684+
assert!(
685+
out.status.success() && stdout.contains("not_affected"),
686+
"vex must attest with every copy patched\nstdout:\n{stdout}\nstderr:\n{stderr}"
687+
);
636688

637689
// A copy re-extracted pristine (another cargo fetched it again) must
638690
// fail the CI gate instead of reading as in sync.
@@ -651,19 +703,7 @@ async fn agent_mode_covers_every_registry_index_copy() {
651703
);
652704

653705
// ... and must keep vex from attesting the patch.
654-
let out = run(
655-
&[
656-
"vex",
657-
"--offline",
658-
"--product",
659-
"pkg:cargo/myapp@0.1.0",
660-
"--cwd",
661-
cwd,
662-
],
663-
root,
664-
&proxy_url,
665-
)
666-
.await;
706+
let out = run(&vex_args, root, &proxy_url).await;
667707
let stdout = String::from_utf8_lossy(&out.stdout);
668708
let stderr = String::from_utf8_lossy(&out.stderr);
669709
assert!(
@@ -702,4 +742,9 @@ async fn agent_mode_covers_every_registry_index_copy() {
702742
copy.display()
703743
);
704744
}
745+
assert_eq!(
746+
lib(&corp),
747+
b"pub fn corp() {}\n",
748+
"rollback touched another registry's crate"
749+
);
705750
}

0 commit comments

Comments
 (0)