diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 10a71045f..3f4697b35 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -187,7 +187,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. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; 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 files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. 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`, `vendor_pypi_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 files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. 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`, `vendor_pypi_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. @@ -1004,7 +1004,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). @@ -1333,6 +1333,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. A PyPI revert that restored its recorded wiring but kept the wheel because another project file still installs from it (`vendor_revert_residual_reference`, e.g. a `pipenv requirements` / `uv export` / `poetry export` requirements file) is a keep too, but not a drift (#1184): its skip reason, the per-entry warning, the top-level message and rollback's `vendoredKept[].reason` say that a project file still installs from the vendored artifact, and the remedy is to point the file named by `vendor_revert_residual_reference` back at the registry release (or re-export it from the restored lock), then remove (or roll back) again. Its `vendor_artifact_kept` advisory says the same, and a hosted takeover refused for it (`redirect_vendored_revert_failed`) names the file instead of a wiring edit. 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 ` (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`)". When it emits a PyPI advisory (`vendor_pypi_reinstall_required` / `redirect_pypi_reinstall_required`) they add " (PDM, uv and Pipenv keep a same-version install through a plain sync: run the reinstall the pypi_reinstall_required warning names)". | | `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 adc434144..c8729a408 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -599,7 +599,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 8eb7195b5..77859bd16 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1528,6 +1528,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) @@ -1981,7 +1986,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, @@ -2052,9 +2058,32 @@ 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. 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, + true, + ) + .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() { @@ -2128,6 +2157,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/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index fdf71eea0..1d78d1846 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,50 @@ 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"), + true, + ) + .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 (or could not be \ + 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" + ), + ) + }) + .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 +375,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 2d86b5dc2..a733b3179 100644 --- a/crates/socket-patch-cli/src/ecosystem_dispatch.rs +++ b/crates/socket-patch-cli/src/ecosystem_dispatch.rs @@ -29,6 +29,98 @@ 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 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 +/// 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`). 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, + shadowed_registry: bool, +) -> 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 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 { + continue; + }; + 'copies: for path in paths { + for (file, info) in &record.files { + let v = verify_file_rollback(path, file, info, blobs_path).await; + 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; + } + } + } + } + patched.sort(); + 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. 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, + 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(); + 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, + 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 34946dc53..0bd74b050 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,278 @@ 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:#}" + ); +} + +/// #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); + + // 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); +} 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..26a90d725 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,102 @@ 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() { + 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; + } + 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}" + ); + if cargo_vendor_dir { + std::fs::create_dir_all(proj.join("vendor")).unwrap(); + } + + 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 f0d064b3c..396da17c5 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -1067,6 +1067,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 never re-checks a registry or `cargo vendor` crate's source files: it reuses the crate's compiled artifacts while the package id is unchanged. So after `apply` or `rollback` changes a crate's bytes, socket-patch deletes that crate's fingerprints