Skip to content

Commit dcded7a

Browse files
Fix cargo shared-cache patch orphaned on takeover/prune (#336, #1278) (#1305)
* WIP: Fix cargo shared-cache patch orphaned on takeover/prune (#336, #1278) Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep cargo shared-cache patches restorable Agent mode patches a crate in the machine-wide $CARGO_HOME/registry/src cache, which nothing deletes when a project stops using the crate. Two flows dropped the only record that can restore that copy while leaving it patched for every project on the machine: - rollback/remove after an agent -> vendored takeover skipped the in-place restore for the now vendor-owned crate, reported success, dropped the manifest entry and GC'd its blobs (#336). A vendored cargo crate whose cache copy still holds the patch is now restored in place too, and its record only leaves the manifest when both the in-place and vendored legs succeeded. - scan --prune/--sync pruned the entry of a crate the lock bumped or dropped (the lock-scoped crawl no longer reports it) (#1278). The entry is now kept while its cache copy is still patched, with a cargo_cache_patch_kept warning naming the rollback that restores it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Find cargo cache copies behind a vendor dir Review follow-up. With a `cargo vendor` dir the cargo crawl searches only `vendor/`, so a registry-cache copy an earlier apply patched was never examined: prune could still drop its record and rollback could skip it. Copies are now also looked up in the registry cache there, and rollback restores the copy it found. A copy whose file cannot be read (I/O error, unsafe key) counts as possibly patched, so its record is kept rather than dropped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Never write registry copies behind vendor/ Security review follow-up. The registry-cache lookup behind a `cargo vendor` dir is now read-only: only the prune's keep decision uses it (so a record is never dropped while the hidden copy may be patched), and only for a real Cargo project (Cargo.toml or Cargo.lock), not any vendor/ dir. Rollback and remove restore only copies their own crawl reaches, as before. The cargo_cache_patch_kept warning names `rollback --global` for the hidden-copy case. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Restore a vendored crate's cache copy behind a cargo vendor dir A project vendor/ dir hides the registry cache from the crawl, so rollback skipped a still-patched $CARGO_HOME copy of a vendored crate, let the vendored leg succeed and dropped the manifest record: the #336 orphan again. Look up the shadowed copies for those crates and restore them before the record goes; scan --prune already keeps such a record. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent d5d7a53 commit dcded7a

8 files changed

Lines changed: 572 additions & 9 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 3 additions & 2 deletions
Large diffs are not rendered by default.

‎crates/socket-patch-cli/src/commands/remove.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,8 @@ pub async fn run(args: RemoveArgs) -> i32 {
599599

600600
// ── nested in-place rollback ────────────────────────────────────────
601601
// Vendor-owned purls are excluded from the in-place restore (the
602-
// vendored leg below reverts them); an unreadable ledger degrades to
602+
// vendored leg below reverts them) unless their Cargo shared-cache copy
603+
// still carries an agent-mode patch (#336); an unreadable ledger degrades to
603604
// "nothing vendored" here and fails closed at that leg.
604605
let vendored_keys: HashSet<PurlKey> = vendor_state_result
605606
.as_ref()

‎crates/socket-patch-cli/src/commands/rollback.rs‎

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1528,6 +1528,11 @@ pub async fn run(args: RollbackArgs) -> i32 {
15281528
if vendored_excluded.contains(purl) {
15291529
return vendored_reverted_ok(purl);
15301530
}
1531+
// A vendored Cargo crate whose shared-cache copy the
1532+
// in-place leg restored too (#336) needs BOTH legs done.
1533+
if purl_keys_cover(&vendored_keys, purl) && !vendored_reverted_ok(purl) {
1534+
return false;
1535+
}
15311536
succeeded_purls.contains(*purl)
15321537
|| not_installed.contains(purl)
15331538
|| superseded.contains(purl)
@@ -1981,7 +1986,8 @@ pub async fn run(args: RollbackArgs) -> i32 {
19811986
/// The in-place (agent) rollback engine over an already-loaded `manifest`.
19821987
/// `vendored_keys` is the ledger's ownership set (see
19831988
/// [`VendorState::purl_keys`]): vendor-owned purls are excluded from the
1984-
/// in-place restore. Both `run()` and `remove`'s delegation load each
1989+
/// in-place restore, except a vendored Cargo crate whose shared-cache copy
1990+
/// still carries an agent-mode patch (#336). Both `run()` and `remove`'s delegation load each
19851991
/// store once under the lock and thread it in here.
19861992
pub(crate) async fn rollback_patches_inner(
19871993
common: &GlobalArgs,
@@ -2052,9 +2058,32 @@ pub(crate) async fn rollback_patches_inner(
20522058
// ledger-key / base-purl / qualifier-stripped triple; the caller
20532059
// degrades unreadable state to "nothing vendored".
20542060
let is_vendored = |p: &str| purl_keys_cover(vendored_keys, p);
2055-
let (vendored_targets, patches_to_rollback): (Vec<_>, Vec<_>) = patches_to_rollback
2061+
let (vendored_targets, mut patches_to_rollback): (Vec<_>, Vec<_>) = patches_to_rollback
20562062
.into_iter()
20572063
.partition(|p| is_vendored(&p.purl));
2064+
// Except a vendored Cargo crate whose shared registry-cache copy still
2065+
// carries an earlier agent-mode patch (#336): vendoring never touched
2066+
// that copy, and the manifest record about to be dropped holds the only
2067+
// before-blobs that can restore it. Only a copy that is actually
2068+
// patched is restored — the vendored copy under `.socket/vendor/` is
2069+
// never a rollback location — so a plain vendored crate (cache copy
2070+
// absent or pristine) is skipped exactly as before. A project `vendor/`
2071+
// dir (`cargo vendor`) hides the registry cache from the crawl, so its
2072+
// copies are looked up there too: the record is dropped after this run,
2073+
// and an unrestored copy would be orphaned with no before-blobs.
2074+
let vendored_purls: Vec<String> = vendored_targets.iter().map(|p| p.purl.clone()).collect();
2075+
let cache_patched = crate::ecosystem_dispatch::cargo_copies_still_patched(
2076+
manifest,
2077+
&vendored_purls,
2078+
&common.crawler_options(),
2079+
&blobs_path,
2080+
true,
2081+
)
2082+
.await;
2083+
let (cache_targets, vendored_targets): (Vec<_>, Vec<_>) = vendored_targets
2084+
.into_iter()
2085+
.partition(|p| cache_patched.contains(&p.purl));
2086+
patches_to_rollback.extend(cache_targets);
20582087
let mut vendored_skipped: Vec<String> = vendored_targets.into_iter().map(|p| p.purl).collect();
20592088
vendored_skipped.sort();
20602089
if patches_to_rollback.is_empty() {
@@ -2128,6 +2157,24 @@ pub(crate) async fn rollback_patches_inner(
21282157
common.silent || common.json,
21292158
)
21302159
.await;
2160+
// The shared-cache copies of the vendored Cargo crates restored above
2161+
// (#336), including those a `cargo vendor` dir hides from the crawl.
2162+
let cache_purls: Vec<String> = cache_patched
2163+
.into_iter()
2164+
.filter(|p| in_scope.contains(p))
2165+
.collect();
2166+
if !cache_purls.is_empty() {
2167+
let shadowed =
2168+
crate::ecosystem_dispatch::find_cargo_copies(cache_purls, &crawler_options, true).await;
2169+
for (purl, paths) in shadowed {
2170+
let copies = all_packages_multi.entry(purl).or_default();
2171+
for path in paths {
2172+
if !copies.contains(&path) {
2173+
copies.push(path);
2174+
}
2175+
}
2176+
}
2177+
}
21312178
// One restore per physical copy, as apply patches them (#633).
21322179
distinct_npm_copies(&mut all_packages_multi).await;
21332180

‎crates/socket-patch-cli/src/commands/scan/gc.rs‎

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,8 @@ pub(super) async fn run_apply_gc(
235235
Ok(Some(m)) => m,
236236
_ => return GcSummary::vendor_only(vendor_gc),
237237
};
238-
let prunable = detect_prunable(&manifest, scanned_purls, vendored);
238+
let mut prunable = detect_prunable(&manifest, scanned_purls, vendored);
239+
let kept = keep_patched_cargo_copies(common, &manifest, socket_dir, &mut prunable).await;
239240
for purl in &prunable {
240241
manifest.patches.remove(purl);
241242
}
@@ -258,10 +259,50 @@ pub(super) async fn run_apply_gc(
258259
}
259260
let mut gc = run_gc(&manifest, prunable, socket_dir, /*dry_run=*/ false).await;
260261
gc.absorb_vendor_gc(vendor_gc);
262+
gc.warnings.extend(kept);
261263
gc.warnings.extend(write_failure);
262264
gc
263265
}
264266

267+
/// Drop from `prunable` every Cargo entry whose agent-mode patch is still
268+
/// on disk in a copy the crawl no longer reports (#1278): the project
269+
/// crawl is scoped to the crates `Cargo.lock` resolves, but a crate bumped
270+
/// or dropped from the lock keeps its patched copy in the machine-wide
271+
/// `$CARGO_HOME/registry/src` cache, which nothing deletes. Its manifest
272+
/// record holds the only before-blobs that can restore that copy, so it is
273+
/// kept, with one `cargo_cache_patch_kept` warning per entry naming the
274+
/// `rollback` that restores the copy and then drops the record.
275+
async fn keep_patched_cargo_copies(
276+
common: &GlobalArgs,
277+
manifest: &PatchManifest,
278+
socket_dir: &Path,
279+
prunable: &mut Vec<String>,
280+
) -> Vec<(&'static str, String)> {
281+
let kept = crate::ecosystem_dispatch::cargo_copies_still_patched(
282+
manifest,
283+
prunable.iter(),
284+
&common.crawler_options(),
285+
&socket_dir.join("blobs"),
286+
true,
287+
)
288+
.await;
289+
prunable.retain(|p| !kept.contains(p));
290+
kept.into_iter()
291+
.map(|purl| {
292+
(
293+
"cargo_cache_patch_kept",
294+
format!(
295+
"kept {purl}: the project no longer resolves it, but its copy in the \
296+
shared Cargo registry cache is still patched (or could not be \
297+
read to tell); run `socket-patch rollback {purl}` (with `--global` \
298+
when a `cargo vendor` dir hides the registry cache) to restore \
299+
that copy and drop the entry"
300+
),
301+
)
302+
})
303+
.collect()
304+
}
305+
265306
/// The vendored-state half of the GC alone, for a `--prune` whose crawl
266307
/// found nothing (the manifest half is skipped there: pruning against an
267308
/// empty crawl would drop every entry). Reverting entries whose patch left
@@ -334,7 +375,10 @@ async fn preview_apply_gc(
334375
}
335376
}
336377
}
337-
let prunable = detect_prunable(&manifest, scanned_purls, vendored);
378+
let mut prunable = detect_prunable(&manifest, scanned_purls, vendored);
379+
// The wet pass keeps a Cargo entry whose shared-cache copy is still
380+
// patched; so does the preview.
381+
let _ = keep_patched_cargo_copies(common, &manifest, socket_dir, &mut prunable).await;
338382
// Likewise drop the prunable entries in memory before the sweep: the
339383
// cleanup helpers derive the referenced set from this manifest.
340384
for purl in &prunable {

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

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,98 @@ pub fn crawl_covers_purl(purl: &str) -> bool {
2929
Ecosystem::from_purl(purl).is_some()
3030
}
3131

32+
/// Of `purls` (manifest keys), the Cargo ones whose agent-mode in-place
33+
/// patch may still be on disk: a copy [`find_cargo_copies`] locates
34+
/// (`shadowed_registry`: see there) with at least one file at its record's afterHash, or one that cannot be read to
35+
/// tell (an I/O error, an unsafe key: kept fail-closed). Sorted.
36+
///
37+
/// Cargo is the one ecosystem whose patched copy outlives the project's
38+
/// use of it: the registry cache is shared machine-wide and nothing
39+
/// deletes a crate from it when a project vendors the crate (#336) or
40+
/// stops locking it (#1278). Its manifest record holds the only
41+
/// before-blobs that can restore that copy, so callers about to drop the
42+
/// record must first restore the copy (`rollback`) or keep the record
43+
/// (`scan --prune`). A vendored crate's committed copy under
44+
/// `.socket/vendor/` is never one of these locations.
45+
pub async fn cargo_copies_still_patched<'a>(
46+
manifest: &socket_patch_core::manifest::schema::PatchManifest,
47+
purls: impl IntoIterator<Item = &'a String>,
48+
options: &CrawlerOptions,
49+
blobs_path: &std::path::Path,
50+
shadowed_registry: bool,
51+
) -> Vec<String> {
52+
use socket_patch_core::patch::rollback::{verify_file_rollback, VerifyRollbackStatus};
53+
let cargo: Vec<String> = purls
54+
.into_iter()
55+
.filter(|p| Ecosystem::from_purl(p) == Some(Ecosystem::Cargo))
56+
.filter(|p| manifest.patches.contains_key(p.as_str()))
57+
.cloned()
58+
.collect();
59+
if cargo.is_empty() {
60+
return Vec::new();
61+
}
62+
let found = find_cargo_copies(cargo, options, shadowed_registry).await;
63+
let mut patched = Vec::new();
64+
for (purl, paths) in &found {
65+
let Some(record) = manifest.patches.get(purl) else {
66+
continue;
67+
};
68+
'copies: for path in paths {
69+
for (file, info) in &record.files {
70+
let v = verify_file_rollback(path, file, info, blobs_path).await;
71+
let still = match v.status {
72+
VerifyRollbackStatus::Ready | VerifyRollbackStatus::MissingBlob => true,
73+
VerifyRollbackStatus::NotFound => !v.is_absent(),
74+
VerifyRollbackStatus::AlreadyOriginal | VerifyRollbackStatus::HashMismatch => {
75+
false
76+
}
77+
};
78+
if still {
79+
patched.push(purl.clone());
80+
break 'copies;
81+
}
82+
}
83+
}
84+
}
85+
patched.sort();
86+
patched
87+
}
88+
89+
/// The copies of the Cargo `purls` [`find_all_packages_for_rollback`]
90+
/// locates (the roots rollback restores), plus, with `shadowed_registry`,
91+
/// the shared `$CARGO_HOME/registry/src` copies of a local Cargo project
92+
/// with a `cargo vendor` dir, whose crawl searches only that dir — where
93+
/// an apply from before `cargo vendor` may have left the patch. The prune
94+
/// only reads them (its keep decision); rollback restores them only for a
95+
/// vendored crate whose record it is about to drop (#336).
96+
pub async fn find_cargo_copies(
97+
purls: Vec<String>,
98+
options: &CrawlerOptions,
99+
shadowed_registry: bool,
100+
) -> HashMap<String, Vec<PathBuf>> {
101+
let partitioned = HashMap::from([(Ecosystem::Cargo, purls)]);
102+
let mut found = find_all_packages_for_rollback(&partitioned, options, true).await;
103+
let local = !options.global && options.global_prefix.is_none();
104+
let cargo_project =
105+
options.cwd.join("Cargo.toml").is_file() || options.cwd.join("Cargo.lock").is_file();
106+
if shadowed_registry && local && cargo_project && options.cwd.join("vendor").is_dir() {
107+
let registry = CrawlerOptions {
108+
cwd: options.cwd.clone(),
109+
global: true,
110+
global_prefix: None,
111+
};
112+
for (purl, paths) in find_all_packages_for_rollback(&partitioned, &registry, true).await {
113+
let copies = found.entry(purl).or_default();
114+
for path in paths {
115+
if !copies.contains(&path) {
116+
copies.push(path);
117+
}
118+
}
119+
}
120+
}
121+
found
122+
}
123+
32124
/// Partition PURLs by ecosystem, filtering by the `--ecosystems` flag if set.
33125
pub fn partition_purls(
34126
purls: &[String],

0 commit comments

Comments
 (0)