From 088855efdc0e3cd62c70d7c13278dfb484cfa368 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 12:42:33 -0400 Subject: [PATCH 1/5] WIP: Fix cargo shared-cache patch orphaned on takeover/prune (#336, #1278) Co-Authored-By: Claude Opus 5.5 (1M context) From 3df92680bb9fac8c934866f4e708cbd236cead4e Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 13:15:31 -0400 Subject: [PATCH 2/5] 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) --- crates/socket-patch-cli/CLI_CONTRACT.md | 5 +- .../socket-patch-cli/src/commands/remove.rs | 3 +- .../socket-patch-cli/src/commands/rollback.rs | 29 ++- .../socket-patch-cli/src/commands/scan/gc.rs | 45 +++- .../src/ecosystem_dispatch.rs | 55 +++++ crates/socket-patch-cli/tests/e2e_cargo.rs | 192 +++++++++++++++++- .../tests/e2e_vendor_cargo_build.rs | 81 ++++++++ docs/ecosystems.md | 7 + 8 files changed, 408 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index bb4592f4b..d619507a3 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -144,7 +144,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (the per-patch `skipped`/`vendored` records in `apply.patches[]` are unchanged); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. -`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob and diff-archive files, and every legacy package archive, from `.socket/`. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `failedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview), plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (): .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so `revertableVendoredEntries` may over-promise what a wet run will actually reclaim. +`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob and diff-archive files, and every legacy package archive, from `.socket/`. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). A Cargo entry is also exempt while its agent-mode copy is still patched: the project crawl looks up only the crates `Cargo.lock` resolves, but a crate the lock bumped or dropped keeps its patched copy in the machine-wide `$CARGO_HOME/registry/src` cache (nothing deletes it), and the entry holds the only blobs that can restore that copy. The wet pass keeps it with a `cargo_cache_patch_kept` warning naming the `socket-patch rollback ` that restores the copy and drops the entry; the preview leaves it out of `prunableManifestEntries`. The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `failedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview), plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), `cargo_cache_patch_kept` (see above), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (): .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so `revertableVendoredEntries` may over-promise what a wet run will actually reclaim. `scan` queries the patch API in `--batch-size` chunks. Authenticated runs POST `/v0/orgs/{slug}/patches/batch`; token-less runs POST `{proxy}/patch/batch` on the public proxy and degrade to per-package `GET /patch/by-package/:purl` requests in two cases: the deployed proxy predates the batch endpoint (legacy proxies answer the POST with their `400 "Unsupported endpoint"` catch-all), or the all-or-nothing batch validation rejects the chunk (e.g. a crawled PURL type the server doesn't recognize, such as `pkg:jsr/…` — the per-package path tolerates those individually, preserving the pre-batch scan semantics). Rate limits and over-capacity 503s surface instead of silently degrading. @@ -939,7 +939,7 @@ worse, lets a warm cache silently serve unpatched bytes): A bare `rollback` (or a scoped one, for its scope) restores the SYSTEM to unpatched and cleans up the local state, in phases under one `apply.lock` acquisition: 1. **State discovery.** A missing manifest is no longer fatal when the vendor ledger or the lockfiles' hosted pins hold work (`rollback` runs manifest-less on hosted-only / vendored projects — every `scan`/`get --mode vendored` and v5 `scan --mode hosted` project is manifest-less). The **truly-empty** project — no manifest, no vendor ledger, no hosted pin — keeps the legacy "Manifest not found" exit 1 (JSON: the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape), with one v5.0 exception: when a pre-v5 `.socket/vendor/redirect-state.json` is the only thing left, nothing pins it any more, so a wet run deletes it and exits 0 (human `Removed the pre-v5 hosted ledger .socket/vendor/redirect-state.json: no lockfile pins a hosted patch.`, `Would remove …` on `--dry-run`, which deletes nothing; JSON `{status: "success", rolledBack: 0, alreadyOriginal: 0, failed: 0, dryRun, warnings, legacyRedirectLedgerRemoved}` — a minimal envelope without the keys below; a failed delete is the `legacy_redirect_ledger_kept` warning, still exit 0). A project whose lockfiles still reference `.socket/vendor/` artifacts but whose vendor ledger is missing errors asking for `.socket/vendor/state.json` to be restored from version control first (v5.0: `repair` no longer reconstructs the ledger). **Corrupt-ledger containment**: an unreadable vendor ledger fails ONLY the legs that need it — the vendored leg, manifest cleanup, and GC are skipped fail-closed (`vendor_state_unreadable` warning) while the agent and hosted legs still run; it drives `partial_failure` exit 1, and an emergency restore is never blocked by it. When the ONLY state on disk is an unreadable vendor ledger, the run fails closed naming the store. A pre-v5 redirect ledger is never read by rollback (v4's `redirect_state_unreadable` is no longer emitted). Under `--global`/`--global-prefix` the project's hosted pins and vendor ledger are not discovered at all, so the vendored and hosted legs below do not run (see "Global scope never touches the project's state"). -2. **Agent leg** — the existing in-place restore machinery, unchanged (v5.0 presentation: the human `No patches found in manifest` line prints only for an unscoped run with no work in ANY leg — a run whose work is all vendored/hosted stays quiet about the manifest): multi-copy restore, release-variant narrowing, the before-blob gate (+ on-demand download; a gate abort still exits 1 with per-package `missing_blob` failure results **and** skips manifest cleanup + GC entirely — nothing was restored, and the retry's revert data must survive), local-go redirect drop, and the `not_installed` exit-0 asymmetry verbatim. Vendor-owned purls are still excluded here (see the vendored-mode section) — they are handled by the next leg instead of being punted to other commands. +2. **Agent leg** — the existing in-place restore machinery, unchanged (v5.0 presentation: the human `No patches found in manifest` line prints only for an unscoped run with no work in ANY leg — a run whose work is all vendored/hosted stays quiet about the manifest): multi-copy restore, release-variant narrowing, the before-blob gate (+ on-demand download; a gate abort still exits 1 with per-package `missing_blob` failure results **and** skips manifest cleanup + GC entirely — nothing was restored, and the retry's revert data must survive), local-go redirect drop, and the `not_installed` exit-0 asymmetry verbatim. Vendor-owned purls are still excluded here (see the vendored-mode section) — they are handled by the next leg instead of being punted to other commands — with one exception: a vendored Cargo crate whose copy in the shared `$CARGO_HOME/registry/src` cache (or a `cargo vendor` dir) still carries an earlier agent-mode patch is restored here too (vendoring never touches that copy, and the manifest record holds the only blobs that can restore it); its record leaves the manifest only when both this leg and the vendored leg succeeded. `remove` restores it the same way. 3. **Vendored leg** — each in-scope ledger entry (embedded-record entries included) is reverted through the vendor backends: lockfile wiring restored, artifact dir deleted (and its emptied `.socket/vendor//` husk pruned, v5.0), ledger entry dropped + persisted per purl (crash-consistent, like `vendor --revert`). A **drift-keep** (the backend refused a drifted lock) keeps the entry, the artifact, AND the manifest record (`vendoredKept`, exit 1 — the system is still patched); a failure is recorded and other entries proceed. 4. **Hosted leg** — each in-scope hosted pin is restored to its default upstream registry entry; see "Hosted unwind coverage" below. After a hosted leg with no failure, a wet run deletes a pre-v5 `redirect-state.json` once no lockfile pins a hosted patch any more (a failed delete is the `legacy_redirect_ledger_kept` warning). 5. **Manifest cleanup** — entries are removed ONLY for in-scope purls whose legs fully succeeded, were not-installed, or were release-variant siblings narrowed away by an attempted variant that succeeded (half a variant group never lingers — `remove` parity); drift-kept and failed purls keep their records, and a failed variant holds its whole group. A manifest record that a live hosted pin has superseded (the lockfile wires the same package release to a different patch uuid, e.g. an agent → hosted migration after the patch was replaced) and whose installed copy holds neither side of the record is not restored in place and does not fail the run: the hosted leg's lock restore and the next install unwind it, and the record leaves the manifest with the `rollback_record_superseded` warning (`remove` does the same instead of aborting before its hosted leg). A copy still holding the record's patched bytes is restored as usual. No-op removals never rewrite the file. A failed write surfaces as `manifest_write_failed` (warning + `partial_failure` exit 1; GC still runs against the unchanged manifest). @@ -1269,6 +1269,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_revert_kept` | `skipped` + top-level error | remove (v5.0): the vendored revert drift-kept (`kept_artifact`), so the ledger entry AND the manifest entry were both kept. ANY drift-keep makes the run a `partialFailure` (exit 1) — part of the requested removal did not happen; when EVERY matching entry drift-kept, the top-level error carries this code (`summary.removed` stays 0; the identifier DID match, so never `not_found`). Remedy: re-run `scan --mode vendored` to normalize, then remove. Rollback's counterpart is the `vendoredKept: []` envelope array (also exit 1). | | `hosted_reverted` | `removed` | remove (v5.0): a hosted lockfile pin was restored to its upstream registry entry as part of removing the patch (`verified` on dry-run). Beside a manifest entry it bypasses `summary.removed` like `vendor_reverted`. | | `hosted_revert_failed` | top-level error | remove (v5.0): a matched hosted pin could not be restored to its upstream registry entry (`--offline`, a registry that does not answer, `bun.lockb`, a lock shape the restore refuses — see "Hosted unwind coverage"), or writing the restored files failed; the message names the `git checkout -- ` remedy. The manifest was not modified, exit 1. Rollback's counterpart is a `hosted.failed[]` entry (also `partial_failure` exit 1). v4's `hosted_revert_unsupported` is no longer emitted (every ecosystem has a restore). | +| `cargo_cache_patch_kept` | `scan --prune`/`--sync` `gc.warnings[]` (human: `GC: …`) | a Cargo manifest entry the crawl no longer reports (the lock bumped or dropped the crate) was NOT pruned because its copy in the shared `$CARGO_HOME/registry/src` cache still carries the agent-mode patch; the record holds the only blobs that can restore it. The detail names the purl and `socket-patch rollback `, which restores the copy and drops the entry. Never affects the exit. | | `reinstall_required` | rollback `warnings[]` | rollback (v5.0): vendored/hosted wiring was unwound, but installed trees keep their patched bytes until the next package-manager install — the stale-install advisory. When the run also emits a Bun advisory (`vendor_bun_reinstall_required` / `redirect_bun_reinstall_required`) the detail and the human note add " (Bun: a plain `bun install` keeps them; run `bun install --force`)". | | `hosted_state_not_preservable` | rollback / remove `warnings[]` | rollback `--preserve-state` (v5.0): hosted pins were restored to upstream anyway — the lockfile pins are hosted mode's only record, so there is no local state to preserve; re-run `scan --mode hosted` to re-wire. (`remove --preserve-state` reports the same code in its `warnings[]` — manifest-backed and hosted-only alike — and prints it as a `Note:` on stderr in human mode.) | | `out_of_scope_copies_restored` | rollback `warnings[]` | path-scoped rollback (v5.0): a selected patch had installed copies outside the given patterns; ALL copies were restored (patches are per-package). Informational — never flips the exit. | diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index 6e3591fad..1bf63d5f2 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -595,7 +595,8 @@ pub async fn run(args: RemoveArgs) -> i32 { // ── nested in-place rollback ──────────────────────────────────────── // Vendor-owned purls are excluded from the in-place restore (the - // vendored leg below reverts them); an unreadable ledger degrades to + // vendored leg below reverts them) unless their Cargo shared-cache copy + // still carries an agent-mode patch (#336); an unreadable ledger degrades to // "nothing vendored" here and fails closed at that leg. let vendored_keys: HashSet = vendor_state_result .as_ref() diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 02bcdf968..3bb74ca96 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1502,6 +1502,11 @@ pub async fn run(args: RollbackArgs) -> i32 { if vendored_excluded.contains(purl) { return vendored_reverted_ok(purl); } + // A vendored Cargo crate whose shared-cache copy the + // in-place leg restored too (#336) needs BOTH legs done. + if purl_keys_cover(&vendored_keys, purl) && !vendored_reverted_ok(purl) { + return false; + } succeeded_purls.contains(*purl) || not_installed.contains(purl) || superseded.contains(purl) @@ -1947,7 +1952,8 @@ pub async fn run(args: RollbackArgs) -> i32 { /// The in-place (agent) rollback engine over an already-loaded `manifest`. /// `vendored_keys` is the ledger's ownership set (see /// [`VendorState::purl_keys`]): vendor-owned purls are excluded from the -/// in-place restore. Both `run()` and `remove`'s delegation load each +/// in-place restore, except a vendored Cargo crate whose shared-cache copy +/// still carries an agent-mode patch (#336). Both `run()` and `remove`'s delegation load each /// store once under the lock and thread it in here. pub(crate) async fn rollback_patches_inner( common: &GlobalArgs, @@ -2018,9 +2024,28 @@ pub(crate) async fn rollback_patches_inner( // ledger-key / base-purl / qualifier-stripped triple; the caller // degrades unreadable state to "nothing vendored". let is_vendored = |p: &str| purl_keys_cover(vendored_keys, p); - let (vendored_targets, patches_to_rollback): (Vec<_>, Vec<_>) = patches_to_rollback + let (vendored_targets, mut patches_to_rollback): (Vec<_>, Vec<_>) = patches_to_rollback .into_iter() .partition(|p| is_vendored(&p.purl)); + // Except a vendored Cargo crate whose shared registry-cache copy still + // carries an earlier agent-mode patch (#336): vendoring never touched + // that copy, and the manifest record about to be dropped holds the only + // before-blobs that can restore it. Only a copy that is actually + // patched is restored — the vendored copy under `.socket/vendor/` is + // never a rollback location — so a plain vendored crate (cache copy + // absent or pristine) is skipped exactly as before. + let vendored_purls: Vec = vendored_targets.iter().map(|p| p.purl.clone()).collect(); + let cache_patched = crate::ecosystem_dispatch::cargo_copies_still_patched( + manifest, + &vendored_purls, + &common.crawler_options(), + &blobs_path, + ) + .await; + let (cache_targets, vendored_targets): (Vec<_>, Vec<_>) = vendored_targets + .into_iter() + .partition(|p| cache_patched.contains(&p.purl)); + patches_to_rollback.extend(cache_targets); let mut vendored_skipped: Vec = vendored_targets.into_iter().map(|p| p.purl).collect(); vendored_skipped.sort(); if patches_to_rollback.is_empty() { diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 38c102440..69eb9d625 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -235,7 +235,8 @@ pub(super) async fn run_apply_gc( Ok(Some(m)) => m, _ => return GcSummary::vendor_only(vendor_gc), }; - let prunable = detect_prunable(&manifest, scanned_purls, vendored); + let mut prunable = detect_prunable(&manifest, scanned_purls, vendored); + let kept = keep_patched_cargo_copies(common, &manifest, socket_dir, &mut prunable).await; for purl in &prunable { manifest.patches.remove(purl); } @@ -258,10 +259,47 @@ pub(super) async fn run_apply_gc( } let mut gc = run_gc(&manifest, prunable, socket_dir, /*dry_run=*/ false).await; gc.absorb_vendor_gc(vendor_gc); + gc.warnings.extend(kept); gc.warnings.extend(write_failure); gc } +/// Drop from `prunable` every Cargo entry whose agent-mode patch is still +/// on disk in a copy the crawl no longer reports (#1278): the project +/// crawl is scoped to the crates `Cargo.lock` resolves, but a crate bumped +/// or dropped from the lock keeps its patched copy in the machine-wide +/// `$CARGO_HOME/registry/src` cache, which nothing deletes. Its manifest +/// record holds the only before-blobs that can restore that copy, so it is +/// kept, with one `cargo_cache_patch_kept` warning per entry naming the +/// `rollback` that restores the copy and then drops the record. +async fn keep_patched_cargo_copies( + common: &GlobalArgs, + manifest: &PatchManifest, + socket_dir: &Path, + prunable: &mut Vec, +) -> Vec<(&'static str, String)> { + let kept = crate::ecosystem_dispatch::cargo_copies_still_patched( + manifest, + prunable.iter(), + &common.crawler_options(), + &socket_dir.join("blobs"), + ) + .await; + prunable.retain(|p| !kept.contains(p)); + kept.into_iter() + .map(|purl| { + ( + "cargo_cache_patch_kept", + format!( + "kept {purl}: the project no longer resolves it, but its copy in the \ + shared Cargo registry cache is still patched; run `socket-patch \ + rollback {purl}` to restore that copy and drop the entry" + ), + ) + }) + .collect() +} + /// The vendored-state half of the GC alone, for a `--prune` whose crawl /// found nothing (the manifest half is skipped there: pruning against an /// empty crawl would drop every entry). Reverting entries whose patch left @@ -334,7 +372,10 @@ async fn preview_apply_gc( } } } - let prunable = detect_prunable(&manifest, scanned_purls, vendored); + let mut prunable = detect_prunable(&manifest, scanned_purls, vendored); + // The wet pass keeps a Cargo entry whose shared-cache copy is still + // patched; so does the preview. + let _ = keep_patched_cargo_copies(common, &manifest, socket_dir, &mut prunable).await; // Likewise drop the prunable entries in memory before the sweep: the // cleanup helpers derive the referenced set from this manifest. for purl in &prunable { diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 5b08f8e1f..3a7b1a5a9 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -29,6 +29,61 @@ pub fn crawl_covers_purl(purl: &str) -> bool { Ecosystem::from_purl(purl).is_some() } +/// Of `purls` (manifest keys), the Cargo ones whose agent-mode in-place +/// patch is still on disk: a copy [`find_all_packages_for_rollback`] +/// locates (`$CARGO_HOME/registry/src`, or a `cargo vendor` dir) with at +/// least one patched file at its record's afterHash. Sorted. +/// +/// Cargo is the one ecosystem whose patched copy outlives the project's +/// use of it: the registry cache is shared machine-wide and nothing +/// deletes a crate from it when a project vendors the crate (#336) or +/// stops locking it (#1278). Its manifest record holds the only +/// before-blobs that can restore that copy, so callers about to drop the +/// record must first restore the copy (`rollback`) or keep the record +/// (`scan --prune`). Located the way rollback restores, not by the +/// lock-scoped crawl, so a crate the lock no longer resolves is found. +/// A vendored crate's committed copy under `.socket/vendor/` is never one +/// of these locations. +pub async fn cargo_copies_still_patched<'a>( + manifest: &socket_patch_core::manifest::schema::PatchManifest, + purls: impl IntoIterator, + options: &CrawlerOptions, + blobs_path: &std::path::Path, +) -> Vec { + use socket_patch_core::patch::rollback::{verify_file_rollback, VerifyRollbackStatus}; + let cargo: Vec = purls + .into_iter() + .filter(|p| Ecosystem::from_purl(p) == Some(Ecosystem::Cargo)) + .filter(|p| manifest.patches.contains_key(p.as_str())) + .cloned() + .collect(); + if cargo.is_empty() { + return Vec::new(); + } + let partitioned = HashMap::from([(Ecosystem::Cargo, cargo)]); + let found = find_all_packages_for_rollback(&partitioned, options, true).await; + let mut patched = Vec::new(); + for (purl, paths) in &found { + let Some(record) = manifest.patches.get(purl) else { + continue; + }; + 'copies: for path in paths { + for (file, info) in &record.files { + let v = verify_file_rollback(path, file, info, blobs_path).await; + if matches!( + v.status, + VerifyRollbackStatus::Ready | VerifyRollbackStatus::MissingBlob + ) { + patched.push(purl.clone()); + break 'copies; + } + } + } + } + patched.sort(); + patched +} + /// Partition PURLs by ecosystem, filtering by the `--ecosystems` flag if set. pub fn partition_purls( purls: &[String], diff --git a/crates/socket-patch-cli/tests/e2e_cargo.rs b/crates/socket-patch-cli/tests/e2e_cargo.rs index 34946dc53..7e0b10b08 100644 --- a/crates/socket-patch-cli/tests/e2e_cargo.rs +++ b/crates/socket-patch-cli/tests/e2e_cargo.rs @@ -15,9 +15,9 @@ #[path = "common/mod.rs"] mod common; -use common::binary; +use common::{binary, git_sha256}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::{Command, Output}; use wiremock::matchers::{method, path}; @@ -267,3 +267,191 @@ async fn scan_discovers_vendor_crates() { assert_proxy_served_scans(&server, 2).await; } + +/// Write `/-/` with a `Cargo.toml` and a +/// `src/lib.rs` holding `lib`. +fn fake_registry_crate(registry: &Path, name: &str, version: &str, lib: &[u8]) -> PathBuf { + let dir = registry.join(format!("{name}-{version}")); + std::fs::create_dir_all(dir.join("src")).unwrap(); + std::fs::write( + dir.join("Cargo.toml"), + format!("[package]\nname = \"{name}\"\nversion = \"{version}\"\n"), + ) + .unwrap(); + std::fs::write(dir.join("src/lib.rs"), lib).unwrap(); + dir +} + +/// One agent-mode manifest record patching `src/lib.rs` from `before` to +/// `after`, with both blobs staged under `.socket/blobs`. +fn manifest_record(socket: &Path, uuid: &str, before: &[u8], after: &[u8]) -> serde_json::Value { + let blobs = socket.join("blobs"); + std::fs::create_dir_all(&blobs).unwrap(); + std::fs::write(blobs.join(git_sha256(before)), before).unwrap(); + std::fs::write(blobs.join(git_sha256(after)), after).unwrap(); + serde_json::json!({ + "uuid": uuid, + "exportedAt": "2026-01-01T00:00:00Z", + "files": { "src/lib.rs": { + "beforeHash": git_sha256(before), + "afterHash": git_sha256(after), + }}, + "vulnerabilities": {}, + "description": "shared-cache fixture", + "license": "MIT", + "tier": "free", + }) +} + +/// #1278: the project crawl only looks up the crates `Cargo.lock` +/// resolves, so a patched crate the lock bumped away from is "not +/// crawled". `scan --sync` must not prune its manifest entry (and GC its +/// blobs) while its copy in the SHARED registry cache still carries the +/// agent-mode patch — that record is the only way to restore the copy, +/// which every other project on the machine builds. The entry is kept with +/// a `cargo_cache_patch_kept` warning; a dropped crate whose cache copy is +/// pristine is still pruned; and `rollback ` then restores the +/// unlocked copy and drops the entry. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sync_keeps_entry_whose_shared_cache_copy_is_still_patched() { + let server = start_proxy().await; + let proxy_url = server.uri(); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let cwd = root.to_str().unwrap(); + + std::fs::write( + root.join("Cargo.toml"), + "[package]\nname = \"myapp\"\nversion = \"0.1.0\"\n\n[dependencies]\nitoa = \"=1.0.17\"\n", + ) + .unwrap(); + // The lock after `cargo update` bumped itoa 1.0.11 -> 1.0.17 and + // dropped ryu. + std::fs::write( + root.join("Cargo.lock"), + "version = 3\n\n[[package]]\nname = \"itoa\"\nversion = \"1.0.17\"\n\ + source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\n\ + [[package]]\nname = \"myapp\"\nversion = \"0.1.0\"\ndependencies = [\n \"itoa\",\n]\n", + ) + .unwrap(); + + let registry = root.join(".cargo/registry/src/index.crates.io-test"); + let orig: &[u8] = b"pub fn fmt() {}\n"; + let patched: &[u8] = b"pub fn fmt() {}\npub fn socket_patched() {}\n"; + // Still patched in the shared cache, no longer locked. + let itoa_old = fake_registry_crate(®istry, "itoa", "1.0.11", patched); + fake_registry_crate(®istry, "itoa", "1.0.17", orig); + // No longer locked, cache copy pristine (rolled back or re-extracted). + fake_registry_crate(®istry, "ryu", "1.0.0", orig); + + let socket = root.join(".socket"); + let itoa = "pkg:cargo/itoa@1.0.11"; + let ryu = "pkg:cargo/ryu@1.0.0"; + let manifest = serde_json::json!({ "patches": { + itoa: manifest_record(&socket, "12780000-0000-4000-8000-000000000001", orig, patched), + ryu: manifest_record(&socket, "12780000-0000-4000-8000-000000000002", orig, patched), + }}); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_string_pretty(&manifest).unwrap(), + ) + .unwrap(); + let read_manifest = || -> serde_json::Value { + serde_json::from_str(&std::fs::read_to_string(socket.join("manifest.json")).unwrap()) + .unwrap() + }; + + // The preview prunes only the pristine one. + let out = run( + &[ + "scan", + "--json", + "--sync", + "--dry-run", + "--yes", + "--cwd", + cwd, + ], + root, + &proxy_url, + ) + .await; + let stdout = String::from_utf8_lossy(&out.stdout); + let json: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("scan --dry-run --sync JSON ({e}):\n{stdout}")); + assert_eq!( + json["gc"]["prunableManifestEntries"], + serde_json::json!([ryu]), + "{json:#}" + ); + + let out = run( + &["scan", "--json", "--sync", "--yes", "--cwd", cwd], + root, + &proxy_url, + ) + .await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "scan --sync must exit 0\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!( + json["gc"]["prunedManifestEntries"], + serde_json::json!([ryu]), + "only the pristine dropped crate is pruned: {json:#}" + ); + let warnings = json["gc"]["warnings"] + .as_array() + .cloned() + .unwrap_or_default(); + assert!( + warnings + .iter() + .any(|w| w["code"] == "cargo_cache_patch_kept" + && w["detail"].as_str().is_some_and(|d| d.contains(itoa))), + "the kept entry must be explained: {json:#}" + ); + let m = read_manifest(); + assert!( + m["patches"].get(itoa).is_some(), + "itoa entry dropped: {m:#}" + ); + assert!(m["patches"].get(ryu).is_none(), "ryu entry kept: {m:#}"); + assert!( + socket.join("blobs").join(git_sha256(patched)).exists(), + "a kept entry's afterHash blob was swept" + ); + assert_eq!(std::fs::read(itoa_old.join("src/lib.rs")).unwrap(), patched); + + // The prune sweep keeps afterHash blobs only (an online rollback + // downloads originals on demand); stage the original as `repair` would + // so the rollback below can run offline. + std::fs::write(socket.join("blobs").join(git_sha256(orig)), orig).unwrap(); + + // The kept record still restores the unlocked shared copy. + let out = run( + &["rollback", itoa, "--json", "--offline", "--cwd", cwd], + root, + &proxy_url, + ) + .await; + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "rollback must exit 0\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert_eq!( + std::fs::read(itoa_old.join("src/lib.rs")).unwrap(), + orig, + "rollback must restore the shared cache copy" + ); + let m = read_manifest(); + assert!( + m["patches"].get(itoa).is_none(), + "rolled-back entry kept: {m:#}" + ); +} diff --git a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs index 7102bd73e..629e1a3b3 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs @@ -943,6 +943,87 @@ fn cargo_vendor_reports_applied_event() { ); } +/// #336: an agent-mode apply patches the crate in the SHARED registry +/// cache; a later `vendor` moves the crate onto a committed copy but leaves +/// that cache edit behind. A bare `rollback` must restore the cache copy +/// too — not report success, drop the manifest entry and GC the only blobs +/// that could restore it while every other project on the machine keeps +/// building the patched crate. +#[test] +fn cargo_rollback_after_agent_to_vendored_restores_shared_cache() { + if !cargo_e2e_matrix::cargo_available("e2e_vendor_cargo_build (agent-then-vendor)") { + return; + } + let tmp = tempfile::tempdir().unwrap(); + let Some((proj, cargo_home, version, crate_dir)) = + stage_fixture(tmp.path(), "agent-then-vendor") + else { + return; + }; + let purl = format!("pkg:cargo/{DEP}@{version}"); + let lib = crate_dir.join("src/lib.rs"); + let orig = std::fs::read(&lib).unwrap(); + let patched: Vec = [orig.as_slice(), PATCH_SUFFIX.as_bytes()].concat(); + stage_patch(&proj, &purl, "src/lib.rs", &orig, &patched); + std::fs::write(proj.join(".socket/blobs").join(git_sha256(&orig)), &orig).unwrap(); + let cwd = proj.to_str().unwrap(); + + let (code, stdout, stderr) = run_socket( + &proj, + &["apply", "--json", "--offline", "--cwd", cwd], + &cargo_home, + ); + assert_eq!( + code, 0, + "apply failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + assert_eq!( + std::fs::read(&lib).unwrap(), + patched, + "apply must patch the cache copy" + ); + + let (code, stdout, stderr) = run_socket( + &proj, + &["vendor", "--json", "--offline", "--cwd", cwd], + &cargo_home, + ); + assert_eq!( + code, 0, + "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + + let (code, stdout, stderr) = run_socket( + &proj, + &["rollback", "--json", "--offline", "--cwd", cwd], + &cargo_home, + ); + assert_eq!( + code, 0, + "rollback failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = parse_json_envelope(&stdout); + assert_eq!(env["status"], "success", "{env}"); + assert_eq!( + std::fs::read(&lib).unwrap(), + orig, + "rollback must restore the shared registry-cache copy: {env}" + ); + let manifest: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(proj.join(".socket/manifest.json")).unwrap()) + .unwrap(); + assert!( + manifest["patches"].get(&purl).is_none(), + "a fully rolled-back entry leaves the manifest: {manifest}" + ); + let ledger = + std::fs::read_to_string(proj.join(".socket/vendor/state.json")).unwrap_or_default(); + assert!( + !ledger.contains(&purl), + "the vendored leg must have reverted the ledger entry: {ledger}" + ); +} + /// get-driven twin (v3.6): `get --mode vendored --vendor-source build` /// must reach the capstone's committed state through scan's vendored engine — /// no manifest (the ledger records the patch), the patched copy under `.socket/vendor/cargo//`, diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 759d7d182..0da4feff9 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -947,6 +947,13 @@ crate that means the **shared** `$CARGO_HOME/registry` cache: the patch affects project on the machine, and is silently reset by `cargo clean` or a cache prune. Use `--mode vendored` for a project-local, committable patch. +Nothing deletes a crate from the cache when the project stops using it, so the +manifest record stays the only way to restore that copy: `rollback` (and `remove`) +also restore it after the crate was moved to vendored mode, and `scan --prune` / +`--sync` keep the record of a crate the lock no longer resolves while its cache copy +is still patched (`cargo_cache_patch_kept`; `socket-patch rollback ` restores +the copy and drops the record). + ## Cargo: vendored wiring in Cargo.toml Vendored mode (v5+) wires a patched crate with a `[patch.crates-io]` path From 8ef69851f52d923f33022859c8a3b8af8213c209 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 14:00:48 -0400 Subject: [PATCH 3/5] 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) --- .../socket-patch-cli/src/commands/rollback.rs | 13 ++++ .../socket-patch-cli/src/commands/scan/gc.rs | 5 +- .../src/ecosystem_dispatch.rs | 57 +++++++++++++---- crates/socket-patch-cli/tests/e2e_cargo.rs | 64 +++++++++++++++++++ 4 files changed, 124 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 3bb74ca96..1062c4c8c 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2119,6 +2119,19 @@ pub(crate) async fn rollback_patches_inner( common.silent || common.json, ) .await; + // A vendored crate restored for its shared-cache copy (#336) is looked + // up the way that copy was found: a `cargo vendor` dir hides the + // registry cache from the crawl above. + let cache_in_scope: Vec = cache_patched + .iter() + .filter(|p| in_scope.contains(*p)) + .cloned() + .collect(); + if !cache_in_scope.is_empty() { + all_packages_multi.extend( + crate::ecosystem_dispatch::find_cargo_copies(cache_in_scope, &crawler_options).await, + ); + } // One restore per physical copy, as apply patches them (#633). distinct_npm_copies(&mut all_packages_multi).await; diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 69eb9d625..0ba02c8c3 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -292,8 +292,9 @@ async fn keep_patched_cargo_copies( "cargo_cache_patch_kept", format!( "kept {purl}: the project no longer resolves it, but its copy in the \ - shared Cargo registry cache is still patched; run `socket-patch \ - rollback {purl}` to restore that copy and drop the entry" + shared Cargo registry cache is still patched (or could not be \ + read to tell); run `socket-patch rollback {purl}` to restore that \ + copy and drop the entry" ), ) }) diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 3a7b1a5a9..78d1b7235 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -30,9 +30,9 @@ pub fn crawl_covers_purl(purl: &str) -> bool { } /// Of `purls` (manifest keys), the Cargo ones whose agent-mode in-place -/// patch is still on disk: a copy [`find_all_packages_for_rollback`] -/// locates (`$CARGO_HOME/registry/src`, or a `cargo vendor` dir) with at -/// least one patched file at its record's afterHash. Sorted. +/// patch may still be on disk: a copy [`find_cargo_copies`] locates with at +/// least one file at its record's afterHash, or one that cannot be read to +/// tell (an I/O error, an unsafe key: kept fail-closed). Sorted. /// /// Cargo is the one ecosystem whose patched copy outlives the project's /// use of it: the registry cache is shared machine-wide and nothing @@ -40,10 +40,8 @@ pub fn crawl_covers_purl(purl: &str) -> bool { /// stops locking it (#1278). Its manifest record holds the only /// before-blobs that can restore that copy, so callers about to drop the /// record must first restore the copy (`rollback`) or keep the record -/// (`scan --prune`). Located the way rollback restores, not by the -/// lock-scoped crawl, so a crate the lock no longer resolves is found. -/// A vendored crate's committed copy under `.socket/vendor/` is never one -/// of these locations. +/// (`scan --prune`). A vendored crate's committed copy under +/// `.socket/vendor/` is never one of these locations. pub async fn cargo_copies_still_patched<'a>( manifest: &socket_patch_core::manifest::schema::PatchManifest, purls: impl IntoIterator, @@ -60,8 +58,7 @@ pub async fn cargo_copies_still_patched<'a>( if cargo.is_empty() { return Vec::new(); } - let partitioned = HashMap::from([(Ecosystem::Cargo, cargo)]); - let found = find_all_packages_for_rollback(&partitioned, options, true).await; + let found = find_cargo_copies(cargo, options).await; let mut patched = Vec::new(); for (purl, paths) in &found { let Some(record) = manifest.patches.get(purl) else { @@ -70,10 +67,14 @@ pub async fn cargo_copies_still_patched<'a>( 'copies: for path in paths { for (file, info) in &record.files { let v = verify_file_rollback(path, file, info, blobs_path).await; - if matches!( - v.status, - VerifyRollbackStatus::Ready | VerifyRollbackStatus::MissingBlob - ) { + let still = match v.status { + VerifyRollbackStatus::Ready | VerifyRollbackStatus::MissingBlob => true, + VerifyRollbackStatus::NotFound => !v.is_absent(), + VerifyRollbackStatus::AlreadyOriginal | VerifyRollbackStatus::HashMismatch => { + false + } + }; + if still { patched.push(purl.clone()); break 'copies; } @@ -84,6 +85,36 @@ pub async fn cargo_copies_still_patched<'a>( patched } +/// Every copy of the Cargo `purls` that agent mode may have patched: the +/// copies [`find_all_packages_for_rollback`] locates, plus — for a local +/// project with a `cargo vendor` dir, whose crawl searches only that dir — +/// the shared `$CARGO_HOME/registry/src` copies an earlier apply (before +/// `cargo vendor`) may have patched. +pub async fn find_cargo_copies( + purls: Vec, + options: &CrawlerOptions, +) -> HashMap> { + let partitioned = HashMap::from([(Ecosystem::Cargo, purls)]); + let mut found = find_all_packages_for_rollback(&partitioned, options, true).await; + let local = !options.global && options.global_prefix.is_none(); + if local && options.cwd.join("vendor").is_dir() { + let registry = CrawlerOptions { + cwd: options.cwd.clone(), + global: true, + global_prefix: None, + }; + for (purl, paths) in find_all_packages_for_rollback(&partitioned, ®istry, true).await { + let copies = found.entry(purl).or_default(); + for path in paths { + if !copies.contains(&path) { + copies.push(path); + } + } + } + } + found +} + /// Partition PURLs by ecosystem, filtering by the `--ecosystems` flag if set. pub fn partition_purls( purls: &[String], diff --git a/crates/socket-patch-cli/tests/e2e_cargo.rs b/crates/socket-patch-cli/tests/e2e_cargo.rs index 7e0b10b08..941e861cb 100644 --- a/crates/socket-patch-cli/tests/e2e_cargo.rs +++ b/crates/socket-patch-cli/tests/e2e_cargo.rs @@ -455,3 +455,67 @@ async fn sync_keeps_entry_whose_shared_cache_copy_is_still_patched() { "rolled-back entry kept: {m:#}" ); } + +/// #1278 with a `cargo vendor` dir: the crawl then searches only +/// `vendor/`, but an apply from before `cargo vendor` patched the shared +/// registry copy, which must still keep its record. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sync_keeps_shared_cache_entry_behind_a_cargo_vendor_dir() { + let server = start_proxy().await; + let proxy_url = server.uri(); + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let cwd = root.to_str().unwrap(); + + std::fs::write( + root.join("Cargo.toml"), + "[package]\nname = \"myapp\"\nversion = \"0.1.0\"\n", + ) + .unwrap(); + let orig: &[u8] = b"pub fn fmt() {}\n"; + let patched: &[u8] = b"pub fn fmt() {}\npub fn socket_patched() {}\n"; + // The vendored (crawled) crate keeps the crawl non-empty. + let vendored = root.join("vendor/itoa"); + std::fs::create_dir_all(&vendored).unwrap(); + std::fs::write( + vendored.join("Cargo.toml"), + "[package]\nname = \"itoa\"\nversion = \"1.0.17\"\n", + ) + .unwrap(); + let registry = root.join(".cargo/registry/src/index.crates.io-test"); + let itoa_old = fake_registry_crate(®istry, "itoa", "1.0.11", patched); + + let socket = root.join(".socket"); + let itoa = "pkg:cargo/itoa@1.0.11"; + let manifest = serde_json::json!({ "patches": { + itoa: manifest_record(&socket, "12780000-0000-4000-8000-000000000003", orig, patched), + }}); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_string_pretty(&manifest).unwrap(), + ) + .unwrap(); + + let out = run( + &["scan", "--json", "--sync", "--yes", "--cwd", cwd], + root, + &proxy_url, + ) + .await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(out.status.success(), "scan --sync must exit 0:\n{stdout}"); + let json: serde_json::Value = serde_json::from_str(&stdout).unwrap(); + assert_eq!( + json["gc"]["prunedManifestEntries"], + serde_json::json!([]), + "{json:#}" + ); + let m: serde_json::Value = + serde_json::from_str(&std::fs::read_to_string(socket.join("manifest.json")).unwrap()) + .unwrap(); + assert!( + m["patches"].get(itoa).is_some(), + "itoa entry dropped: {m:#}" + ); + assert_eq!(std::fs::read(itoa_old.join("src/lib.rs")).unwrap(), patched); +} From fb9fb3a66e9657ffc6258d484e7a92d626871fcb Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 9 Oct 2026 14:36:16 -0400 Subject: [PATCH 4/5] 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) --- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-cli/src/commands/rollback.rs | 14 +---------- .../socket-patch-cli/src/commands/scan/gc.rs | 6 +++-- .../src/ecosystem_dispatch.rs | 24 ++++++++++++------- crates/socket-patch-cli/tests/e2e_cargo.rs | 23 ++++++++++++++++++ 5 files changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d619507a3..b6ac4ae0c 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1269,7 +1269,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_revert_kept` | `skipped` + top-level error | remove (v5.0): the vendored revert drift-kept (`kept_artifact`), so the ledger entry AND the manifest entry were both kept. ANY drift-keep makes the run a `partialFailure` (exit 1) — part of the requested removal did not happen; when EVERY matching entry drift-kept, the top-level error carries this code (`summary.removed` stays 0; the identifier DID match, so never `not_found`). Remedy: re-run `scan --mode vendored` to normalize, then remove. Rollback's counterpart is the `vendoredKept: []` envelope array (also exit 1). | | `hosted_reverted` | `removed` | remove (v5.0): a hosted lockfile pin was restored to its upstream registry entry as part of removing the patch (`verified` on dry-run). Beside a manifest entry it bypasses `summary.removed` like `vendor_reverted`. | | `hosted_revert_failed` | top-level error | remove (v5.0): a matched hosted pin could not be restored to its upstream registry entry (`--offline`, a registry that does not answer, `bun.lockb`, a lock shape the restore refuses — see "Hosted unwind coverage"), or writing the restored files failed; the message names the `git checkout -- ` remedy. The manifest was not modified, exit 1. Rollback's counterpart is a `hosted.failed[]` entry (also `partial_failure` exit 1). v4's `hosted_revert_unsupported` is no longer emitted (every ecosystem has a restore). | -| `cargo_cache_patch_kept` | `scan --prune`/`--sync` `gc.warnings[]` (human: `GC: …`) | a Cargo manifest entry the crawl no longer reports (the lock bumped or dropped the crate) was NOT pruned because its copy in the shared `$CARGO_HOME/registry/src` cache still carries the agent-mode patch; the record holds the only blobs that can restore it. The detail names the purl and `socket-patch rollback `, which restores the copy and drops the entry. Never affects the exit. | +| `cargo_cache_patch_kept` | `scan --prune`/`--sync` `gc.warnings[]` (human: `GC: …`) | a Cargo manifest entry the crawl no longer reports (the lock bumped or dropped the crate) was NOT pruned because its copy in the shared `$CARGO_HOME/registry/src` cache still carries the agent-mode patch; the record holds the only blobs that can restore it. The detail names the purl and `socket-patch rollback ` (with `--global` when a `cargo vendor` dir hides the registry cache from the project crawl), which restores the copy and drops the entry. Never affects the exit. | | `reinstall_required` | rollback `warnings[]` | rollback (v5.0): vendored/hosted wiring was unwound, but installed trees keep their patched bytes until the next package-manager install — the stale-install advisory. When the run also emits a Bun advisory (`vendor_bun_reinstall_required` / `redirect_bun_reinstall_required`) the detail and the human note add " (Bun: a plain `bun install` keeps them; run `bun install --force`)". | | `hosted_state_not_preservable` | rollback / remove `warnings[]` | rollback `--preserve-state` (v5.0): hosted pins were restored to upstream anyway — the lockfile pins are hosted mode's only record, so there is no local state to preserve; re-run `scan --mode hosted` to re-wire. (`remove --preserve-state` reports the same code in its `warnings[]` — manifest-backed and hosted-only alike — and prints it as a `Note:` on stderr in human mode.) | | `out_of_scope_copies_restored` | rollback `warnings[]` | path-scoped rollback (v5.0): a selected patch had installed copies outside the given patterns; ALL copies were restored (patches are per-package). Informational — never flips the exit. | diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 1062c4c8c..5731f87a3 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2040,6 +2040,7 @@ pub(crate) async fn rollback_patches_inner( &vendored_purls, &common.crawler_options(), &blobs_path, + false, ) .await; let (cache_targets, vendored_targets): (Vec<_>, Vec<_>) = vendored_targets @@ -2119,19 +2120,6 @@ pub(crate) async fn rollback_patches_inner( common.silent || common.json, ) .await; - // A vendored crate restored for its shared-cache copy (#336) is looked - // up the way that copy was found: a `cargo vendor` dir hides the - // registry cache from the crawl above. - let cache_in_scope: Vec = cache_patched - .iter() - .filter(|p| in_scope.contains(*p)) - .cloned() - .collect(); - if !cache_in_scope.is_empty() { - all_packages_multi.extend( - crate::ecosystem_dispatch::find_cargo_copies(cache_in_scope, &crawler_options).await, - ); - } // One restore per physical copy, as apply patches them (#633). distinct_npm_copies(&mut all_packages_multi).await; diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 0ba02c8c3..670f58e0a 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -283,6 +283,7 @@ async fn keep_patched_cargo_copies( prunable.iter(), &common.crawler_options(), &socket_dir.join("blobs"), + true, ) .await; prunable.retain(|p| !kept.contains(p)); @@ -293,8 +294,9 @@ async fn keep_patched_cargo_copies( format!( "kept {purl}: the project no longer resolves it, but its copy in the \ shared Cargo registry cache is still patched (or could not be \ - read to tell); run `socket-patch rollback {purl}` to restore that \ - copy and drop the entry" + read to tell); run `socket-patch rollback {purl}` (with `--global` \ + when a `cargo vendor` dir hides the registry cache) to restore \ + that copy and drop the entry" ), ) }) diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 78d1b7235..83fc61635 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -30,8 +30,8 @@ pub fn crawl_covers_purl(purl: &str) -> bool { } /// Of `purls` (manifest keys), the Cargo ones whose agent-mode in-place -/// patch may still be on disk: a copy [`find_cargo_copies`] locates with at -/// least one file at its record's afterHash, or one that cannot be read to +/// patch may still be on disk: a copy [`find_cargo_copies`] locates +/// (`shadowed_registry`: see there) with at least one file at its record's afterHash, or one that cannot be read to /// tell (an I/O error, an unsafe key: kept fail-closed). Sorted. /// /// Cargo is the one ecosystem whose patched copy outlives the project's @@ -47,6 +47,7 @@ pub async fn cargo_copies_still_patched<'a>( purls: impl IntoIterator, options: &CrawlerOptions, blobs_path: &std::path::Path, + shadowed_registry: bool, ) -> Vec { use socket_patch_core::patch::rollback::{verify_file_rollback, VerifyRollbackStatus}; let cargo: Vec = purls @@ -58,7 +59,7 @@ pub async fn cargo_copies_still_patched<'a>( if cargo.is_empty() { return Vec::new(); } - let found = find_cargo_copies(cargo, options).await; + let found = find_cargo_copies(cargo, options, shadowed_registry).await; let mut patched = Vec::new(); for (purl, paths) in &found { let Some(record) = manifest.patches.get(purl) else { @@ -85,19 +86,24 @@ pub async fn cargo_copies_still_patched<'a>( patched } -/// Every copy of the Cargo `purls` that agent mode may have patched: the -/// copies [`find_all_packages_for_rollback`] locates, plus — for a local -/// project with a `cargo vendor` dir, whose crawl searches only that dir — -/// the shared `$CARGO_HOME/registry/src` copies an earlier apply (before -/// `cargo vendor`) may have patched. +/// The copies of the Cargo `purls` [`find_all_packages_for_rollback`] +/// locates (the roots rollback restores), plus, with `shadowed_registry`, +/// the shared `$CARGO_HOME/registry/src` copies of a local Cargo project +/// with a `cargo vendor` dir, whose crawl searches only that dir — where +/// an apply from before `cargo vendor` may have left the patch. Callers +/// only READ the shadowed copies (the prune's keep decision): rollback +/// never writes a registry copy its own crawl would not reach. pub async fn find_cargo_copies( purls: Vec, options: &CrawlerOptions, + shadowed_registry: bool, ) -> HashMap> { let partitioned = HashMap::from([(Ecosystem::Cargo, purls)]); let mut found = find_all_packages_for_rollback(&partitioned, options, true).await; let local = !options.global && options.global_prefix.is_none(); - if local && options.cwd.join("vendor").is_dir() { + let cargo_project = + options.cwd.join("Cargo.toml").is_file() || options.cwd.join("Cargo.lock").is_file(); + if shadowed_registry && local && cargo_project && options.cwd.join("vendor").is_dir() { let registry = CrawlerOptions { cwd: options.cwd.clone(), global: true, diff --git a/crates/socket-patch-cli/tests/e2e_cargo.rs b/crates/socket-patch-cli/tests/e2e_cargo.rs index 941e861cb..0bd74b050 100644 --- a/crates/socket-patch-cli/tests/e2e_cargo.rs +++ b/crates/socket-patch-cli/tests/e2e_cargo.rs @@ -518,4 +518,27 @@ async fn sync_keeps_shared_cache_entry_behind_a_cargo_vendor_dir() { "itoa entry dropped: {m:#}" ); assert_eq!(std::fs::read(itoa_old.join("src/lib.rs")).unwrap(), patched); + + // The warning's remedy: `rollback --global` reaches the hidden copy. + std::fs::write(socket.join("blobs").join(git_sha256(orig)), orig).unwrap(); + let out = run( + &[ + "rollback", + itoa, + "--global", + "--json", + "--offline", + "--cwd", + cwd, + ], + root, + &proxy_url, + ) + .await; + let stdout = String::from_utf8_lossy(&out.stdout); + assert!( + out.status.success(), + "rollback --global must exit 0:\n{stdout}" + ); + assert_eq!(std::fs::read(itoa_old.join("src/lib.rs")).unwrap(), orig); } From 44e0232db0a35ac5c8e92e5a7d711a24f344d2b6 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Sat, 10 Oct 2026 10:16:17 -0400 Subject: [PATCH 5/5] 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) --- .../socket-patch-cli/src/commands/rollback.rs | 25 +++++++++++++++++-- .../src/ecosystem_dispatch.rs | 6 ++--- .../tests/e2e_vendor_cargo_build.rs | 15 +++++++++++ 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 240fd4f11..45be56c3e 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -2033,14 +2033,17 @@ pub(crate) async fn rollback_patches_inner( // before-blobs that can restore it. Only a copy that is actually // patched is restored — the vendored copy under `.socket/vendor/` is // never a rollback location — so a plain vendored crate (cache copy - // absent or pristine) is skipped exactly as before. + // absent or pristine) is skipped exactly as before. A project `vendor/` + // dir (`cargo vendor`) hides the registry cache from the crawl, so its + // copies are looked up there too: the record is dropped after this run, + // and an unrestored copy would be orphaned with no before-blobs. let vendored_purls: Vec = vendored_targets.iter().map(|p| p.purl.clone()).collect(); let cache_patched = crate::ecosystem_dispatch::cargo_copies_still_patched( manifest, &vendored_purls, &common.crawler_options(), &blobs_path, - false, + true, ) .await; let (cache_targets, vendored_targets): (Vec<_>, Vec<_>) = vendored_targets @@ -2120,6 +2123,24 @@ pub(crate) async fn rollback_patches_inner( common.silent || common.json, ) .await; + // The shared-cache copies of the vendored Cargo crates restored above + // (#336), including those a `cargo vendor` dir hides from the crawl. + let cache_purls: Vec = cache_patched + .into_iter() + .filter(|p| in_scope.contains(p)) + .collect(); + if !cache_purls.is_empty() { + let shadowed = + crate::ecosystem_dispatch::find_cargo_copies(cache_purls, &crawler_options, true).await; + for (purl, paths) in shadowed { + let copies = all_packages_multi.entry(purl).or_default(); + for path in paths { + if !copies.contains(&path) { + copies.push(path); + } + } + } + } // One restore per physical copy, as apply patches them (#633). distinct_npm_copies(&mut all_packages_multi).await; diff --git a/crates/socket-patch-cli/src/ecosystem_dispatch.rs b/crates/socket-patch-cli/src/ecosystem_dispatch.rs index 78228943a..a733b3179 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -90,9 +90,9 @@ pub async fn cargo_copies_still_patched<'a>( /// locates (the roots rollback restores), plus, with `shadowed_registry`, /// the shared `$CARGO_HOME/registry/src` copies of a local Cargo project /// with a `cargo vendor` dir, whose crawl searches only that dir — where -/// an apply from before `cargo vendor` may have left the patch. Callers -/// only READ the shadowed copies (the prune's keep decision): rollback -/// never writes a registry copy its own crawl would not reach. +/// an apply from before `cargo vendor` may have left the patch. The prune +/// only reads them (its keep decision); rollback restores them only for a +/// vendored crate whose record it is about to drop (#336). pub async fn find_cargo_copies( purls: Vec, options: &CrawlerOptions, diff --git a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs index 629e1a3b3..26a90d725 100644 --- a/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs +++ b/crates/socket-patch-cli/tests/e2e_vendor_cargo_build.rs @@ -951,6 +951,18 @@ fn cargo_vendor_reports_applied_event() { /// building the patched crate. #[test] fn cargo_rollback_after_agent_to_vendored_restores_shared_cache() { + agent_to_vendored_rollback_restores_shared_cache(false); +} + +/// #336 behind a `cargo vendor` dir: the project crawl then searches only +/// `vendor/`, which hides the registry cache. The rollback still drops the +/// record, so it must still find and restore the patched cache copy. +#[test] +fn cargo_rollback_after_agent_to_vendored_restores_cache_behind_cargo_vendor_dir() { + agent_to_vendored_rollback_restores_shared_cache(true); +} + +fn agent_to_vendored_rollback_restores_shared_cache(cargo_vendor_dir: bool) { if !cargo_e2e_matrix::cargo_available("e2e_vendor_cargo_build (agent-then-vendor)") { return; } @@ -992,6 +1004,9 @@ fn cargo_rollback_after_agent_to_vendored_restores_shared_cache() { code, 0, "vendor failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" ); + if cargo_vendor_dir { + std::fs::create_dir_all(proj.join("vendor")).unwrap(); + } let (code, stdout, stderr) = run_socket( &proj,